💬 hebasto commented on pull request "test: Remove unused signed-integer-overflow suppression":
(https://github.com/bitcoin/bitcoin/pull/27498#discussion_r1172435340)
> was fixed in gcc-11 and later.
Mind pointing at the actual fix if it is known?
(https://github.com/bitcoin/bitcoin/pull/27498#discussion_r1172435340)
> was fixed in gcc-11 and later.
Mind pointing at the actual fix if it is known?
💬 MarcoFalke commented on pull request "test: Remove unused signed-integer-overflow suppression":
(https://github.com/bitcoin/bitcoin/pull/27498#discussion_r1172445226)
I don't know for sure, but limiting the amounts or the previous fix to `PrioritiseTransaction` may have fixed it?
In any case, I think it is good to remove it, even if the bug still exists. Otherwise it is harder to find and fix the bug.
(https://github.com/bitcoin/bitcoin/pull/27498#discussion_r1172445226)
I don't know for sure, but limiting the amounts or the previous fix to `PrioritiseTransaction` may have fixed it?
In any case, I think it is good to remove it, even if the bug still exists. Otherwise it is harder to find and fix the bug.
📝 dergoegge opened a pull request: "net processing, refactor: Decouple PeerManager from gArgs"
(https://github.com/bitcoin/bitcoin/pull/27499)
This PR decouples `PeerManager` from our global args manager by introducing `PeerManager::Options`.
(https://github.com/bitcoin/bitcoin/pull/27499)
This PR decouples `PeerManager` from our global args manager by introducing `PeerManager::Options`.
💬 michaelfolkson commented on issue "Document CoreDev organization":
(https://github.com/bitcoin/bitcoin/issues/27497#issuecomment-1516269521)
Agreed. As someone who has been invited and then abruptly disinvited from these without an explanation of why I do think the secrecy around these is convenient for those who want to push certain agendas or push certain individuals away from contributing to the project. But I think it needs to be a separate repo for organizing and discussing events rather than within this repo. There has to be a cut off for invites and directing that discussion to this repo doesn't seem ideal to me.
(https://github.com/bitcoin/bitcoin/issues/27497#issuecomment-1516269521)
Agreed. As someone who has been invited and then abruptly disinvited from these without an explanation of why I do think the secrecy around these is convenient for those who want to push certain agendas or push certain individuals away from contributing to the project. But I think it needs to be a separate repo for organizing and discussing events rather than within this repo. There has to be a cut off for invites and directing that discussion to this repo doesn't seem ideal to me.
👍 hebasto approved a pull request: "test: Remove unused signed-integer-overflow suppression"
(https://github.com/bitcoin/bitcoin/pull/27498#pullrequestreview-1393937727)
ACK 9999813e91b94e78ed65130478d9dcb6d787514f, I have reviewed the code and it looks OK, I agree it can be merged.
(https://github.com/bitcoin/bitcoin/pull/27498#pullrequestreview-1393937727)
ACK 9999813e91b94e78ed65130478d9dcb6d787514f, I have reviewed the code and it looks OK, I agree it can be merged.
💬 jonatack commented on pull request "p2p: update hardcoded mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1516301818)
> Needs rebase
Done.
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1516301818)
> Needs rebase
Done.
💬 theuni commented on pull request "depends: reuse _config_opts for CMake options":
(https://github.com/bitcoin/bitcoin/pull/27496#issuecomment-1516343383)
> > Building them with CMake will allow us to drop several deps that we currently have (autoconf, automake, pkg-config, etc) which would be unfortunate to carry over after the switch-over.
>
> I know it's not relevant for this PR, but is the plan to upstream CMake support everywhere it's missing (sqlite, all the qt deps etc), and maintain it ourselves where it's not upstreamable (i.e bdb), otherwise we still can't drop any deps.
Technically there's no need to switch all projects to CMake,
...
(https://github.com/bitcoin/bitcoin/pull/27496#issuecomment-1516343383)
> > Building them with CMake will allow us to drop several deps that we currently have (autoconf, automake, pkg-config, etc) which would be unfortunate to carry over after the switch-over.
>
> I know it's not relevant for this PR, but is the plan to upstream CMake support everywhere it's missing (sqlite, all the qt deps etc), and maintain it ourselves where it's not upstreamable (i.e bdb), otherwise we still can't drop any deps.
Technically there's no need to switch all projects to CMake,
...
💬 fanquake commented on issue "Document CoreDev organization":
(https://github.com/bitcoin/bitcoin/issues/27497#issuecomment-1516364064)
Going to close this, as I don't think it's the right place for this discussion, it's not really clear what the outcome should be.
(https://github.com/bitcoin/bitcoin/issues/27497#issuecomment-1516364064)
Going to close this, as I don't think it's the right place for this discussion, it's not really clear what the outcome should be.
✅ fanquake closed an issue: "Document CoreDev organization"
(https://github.com/bitcoin/bitcoin/issues/27497)
(https://github.com/bitcoin/bitcoin/issues/27497)
💬 fanquake commented on pull request "depends: reuse _config_opts for CMake options":
(https://github.com/bitcoin/bitcoin/pull/27496#issuecomment-1516365687)
Guix Build:
```bash
45d96de0248ec8b604cc5c980f1abbc780862d497f195a795c6088087d646c73 guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/SHA256SUMS.part
ebdd087cbb45f1fa337b717c0d7bd53c8b7d0bc78d4e9693de2430a1ab9e03d9 guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/bitcoin-63c0c4ff10b2-aarch64-linux-gnu-debug.tar.gz
e09206e6a70068036ea52731e0f8122665525a132e85f2798662ff9d7644dd8b guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/bitcoin-63c0c4ff10b2-aarch64-linux-gnu.tar.gz
20e71234d2e90386
...
(https://github.com/bitcoin/bitcoin/pull/27496#issuecomment-1516365687)
Guix Build:
```bash
45d96de0248ec8b604cc5c980f1abbc780862d497f195a795c6088087d646c73 guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/SHA256SUMS.part
ebdd087cbb45f1fa337b717c0d7bd53c8b7d0bc78d4e9693de2430a1ab9e03d9 guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/bitcoin-63c0c4ff10b2-aarch64-linux-gnu-debug.tar.gz
e09206e6a70068036ea52731e0f8122665525a132e85f2798662ff9d7644dd8b guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/bitcoin-63c0c4ff10b2-aarch64-linux-gnu.tar.gz
20e71234d2e90386
...
💬 MarcoFalke commented on pull request "test: Remove unused sanitizer suppressions":
(https://github.com/bitcoin/bitcoin/pull/27498#issuecomment-1516366010)
Force pushed to remove another unused one
(https://github.com/bitcoin/bitcoin/pull/27498#issuecomment-1516366010)
Force pushed to remove another unused one
💬 furszy commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1172623190)
> CBlock already has a GetBlockTime(), so I don't think it's necessary to include the time as a separate field here.
Yeah, that works here. The new field was because I initially implemented this on the filters-only sync branch. Where I have no blocks, only headers and filters.
> However I'm not sure that we should be using the block's timestamp for this time rather than MTP. With the block timestamp, we could ignore a more recent block that has a timestamp older than its parent which we wo
...
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1172623190)
> CBlock already has a GetBlockTime(), so I don't think it's necessary to include the time as a separate field here.
Yeah, that works here. The new field was because I initially implemented this on the filters-only sync branch. Where I have no blocks, only headers and filters.
> However I'm not sure that we should be using the block's timestamp for this time rather than MTP. With the block timestamp, we could ignore a more recent block that has a timestamp older than its parent which we wo
...
💬 furszy commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1172638173)
pushed thanks
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1172638173)
pushed thanks
💬 MarcoFalke commented on pull request "test: Remove unused sanitizer suppressions":
(https://github.com/bitcoin/bitcoin/pull/27498#issuecomment-1516416505)
I am thinking about optimistically adding a commit to remove the epoll_ctl one as well. If it still reproduces, it can be re-added?
(https://github.com/bitcoin/bitcoin/pull/27498#issuecomment-1516416505)
I am thinking about optimistically adding a commit to remove the epoll_ctl one as well. If it still reproduces, it can be re-added?
💬 fanquake commented on pull request "test: Remove unused sanitizer suppressions":
(https://github.com/bitcoin/bitcoin/pull/27498#issuecomment-1516418849)
Might as well have a go
(https://github.com/bitcoin/bitcoin/pull/27498#issuecomment-1516418849)
Might as well have a go
📝 glozow opened a pull request: "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0"
(https://github.com/bitcoin/bitcoin/pull/27501)
Add an RPC to get prioritised transactions (also tells you whether the tx is in mempool or not), helping users clean up `mapDeltas` manually. When `CTxMemPool::PrioritiseTransaction` sets a delta to 0, remove the entry from `mapDeltas`.
Motivation / Background
- `mapDeltas` entries are never removed from mapDeltas except when the tx is mined in a block or conflicted.
- Mostly it is a feature to allow `prioritisetransaction` for a tx that isn't in the mempool {yet, anymore}. A user can may
...
(https://github.com/bitcoin/bitcoin/pull/27501)
Add an RPC to get prioritised transactions (also tells you whether the tx is in mempool or not), helping users clean up `mapDeltas` manually. When `CTxMemPool::PrioritiseTransaction` sets a delta to 0, remove the entry from `mapDeltas`.
Motivation / Background
- `mapDeltas` entries are never removed from mapDeltas except when the tx is mined in a block or conflicted.
- Mostly it is a feature to allow `prioritisetransaction` for a tx that isn't in the mempool {yet, anymore}. A user can may
...
💬 glozow commented on pull request "mempool: keep CPFP'd transactions when loading from mempool.dat":
(https://github.com/bitcoin/bitcoin/pull/27476#issuecomment-1516503220)
**Alternatives Considered**
Feel free to suggest more alternatives, but note potential problems.
(1) Call TrimToSize() at smart intervals, e.g. when a timestamp changes (since packages are submitted with the same timestamp), or every ~25 transactions, or when we hit a certain threshold above maxmempool.
(2) When a transaction fails for fee-related reasons, try again with bypass_limits=true, and then schedule a flush for later.
(3) Create a new mempool.dat format, which contains a list of
...
(https://github.com/bitcoin/bitcoin/pull/27476#issuecomment-1516503220)
**Alternatives Considered**
Feel free to suggest more alternatives, but note potential problems.
(1) Call TrimToSize() at smart intervals, e.g. when a timestamp changes (since packages are submitted with the same timestamp), or every ~25 transactions, or when we hit a certain threshold above maxmempool.
(2) When a transaction fails for fee-related reasons, try again with bypass_limits=true, and then schedule a flush for later.
(3) Create a new mempool.dat format, which contains a list of
...
💬 sipa commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1172739143)
Done.
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1172739143)
Done.
💬 MarcoFalke commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172733241)
```suggestion
"Returns a map of all fee deltas in memory pool by txid, and whether the tx is present in mempool.",
```
nit
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172733241)
```suggestion
"Returns a map of all fee deltas in memory pool by txid, and whether the tx is present in mempool.",
```
nit
👍 MarcoFalke approved a pull request: "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0"
(https://github.com/bitcoin/bitcoin/pull/27501#pullrequestreview-1394217946)
Concept ACK. Wanted to add a `deletepriority` RPC myself, but I guess it is easy to implement a for loop in python utilising the output of `getprioritisationmap`
(https://github.com/bitcoin/bitcoin/pull/27501#pullrequestreview-1394217946)
Concept ACK. Wanted to add a `deletepriority` RPC myself, but I guess it is easy to implement a for loop in python utilising the output of `getprioritisationmap`
💬 MarcoFalke commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172737216)
Won't this need mempool.cs to ensure an atomic result? Otherwise you may get nonsense back on a data race, like a parent missing from the mempool, when the child is in the mempool, no?
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172737216)
Won't this need mempool.cs to ensure an atomic result? Otherwise you may get nonsense back on a data race, like a parent missing from the mempool, when the child is in the mempool, no?