💬 ajtowns commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#issuecomment-3250125812)
Backport at https://github.com/ajtowns/bitcoin/tree/202509-29x-backport-compactblock ; don't think it's urgent enough to try pushing for 29.2 though?
(https://github.com/bitcoin/bitcoin/pull/33253#issuecomment-3250125812)
Backport at https://github.com/ajtowns/bitcoin/tree/202509-29x-backport-compactblock ; don't think it's urgent enough to try pushing for 29.2 though?
⚠️ sixtuselemi73-dev opened an issue: "It may be some router setting to enable it, the more secure routers have these kind of protocols disabled by default. Or maybe it doesn't support it. There isn't any reply, not even 2 NOT_AUTHORIZED (but nice to see the retry code is working 😄 )."
(https://github.com/bitcoin/bitcoin/issues/33295)
It may be some router setting to enable it, the more secure routers have these kind of protocols disabled by default. Or maybe it doesn't support it. There isn't any reply, not even 2 NOT_AUTHORIZED (but nice to see the retry code is working 😄 ).
_Originally posted by @laanwj in https://github.com/bitcoin/bitcoin/issues/30005#issuecomment-2085190180_
(https://github.com/bitcoin/bitcoin/issues/33295)
It may be some router setting to enable it, the more secure routers have these kind of protocols disabled by default. Or maybe it doesn't support it. There isn't any reply, not even 2 NOT_AUTHORIZED (but nice to see the retry code is working 😄 ).
_Originally posted by @laanwj in https://github.com/bitcoin/bitcoin/issues/30005#issuecomment-2085190180_
💬 w0xlt commented on issue "GUI (?): Copying output from console causes large mem usage/OOM":
(https://github.com/bitcoin/bitcoin/issues/33285#issuecomment-3250166108)
On debug build, it seems to use all available memory.
Is it something related to Qt?
(https://github.com/bitcoin/bitcoin/issues/33285#issuecomment-3250166108)
On debug build, it seems to use all available memory.
Is it something related to Qt?
✅ achow101 closed an issue: "."
(https://github.com/bitcoin/bitcoin/issues/33295)
(https://github.com/bitcoin/bitcoin/issues/33295)
🤔 glozow reviewed a pull request: "Cluster mempool implementation"
(https://github.com/bitcoin/bitcoin/pull/28676#pullrequestreview-3181193400)
In the "Still to do" section of the PR description, it seems like most can be checked off other than
> Updating wallet code to be cluster-aware (including mini_miner and coin selection)
> Rework many of our functional tests to be cluster-aware
> Figure out what package validation and package RBF rules should be in this design
(https://github.com/bitcoin/bitcoin/pull/28676#pullrequestreview-3181193400)
In the "Still to do" section of the PR description, it seems like most can be checked off other than
> Updating wallet code to be cluster-aware (including mini_miner and coin selection)
> Rework many of our functional tests to be cluster-aware
> Figure out what package validation and package RBF rules should be in this design
💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319357837)
5bc5edf8743408979add1b15d23cc8ab7b948db Checking limits using changeset is so nice. It could be helpful to add a comment that `changeset` is automatically destroyed at the end of this function and thus doesn't need to be handled/aborted.
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319357837)
5bc5edf8743408979add1b15d23cc8ab7b948db Checking limits using changeset is so nice. It could be helpful to add a comment that `changeset` is automatically destroyed at the end of this function and thus doesn't need to be handled/aborted.
💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319334033)
e72b0b1a7eb31078c998d95b4cfffe9cd1f9113d re-adds `-addrmantest` which was removed in #29007
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319334033)
e72b0b1a7eb31078c998d95b4cfffe9cd1f9113d re-adds `-addrmantest` which was removed in #29007
💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319516863)
nit in aa8eb8c3ab1 Fix miniminer_tests to work with cluster limits, I think it's txp0 to txp31 and txc0 to txc30?
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319516863)
nit in aa8eb8c3ab1 Fix miniminer_tests to work with cluster limits, I think it's txp0 to txp31 and txc0 to txc30?
💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319578206)
Perhaps also add `(maximum: %u), MAX_CLUSTER_COUNT_LIMIT`
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319578206)
Perhaps also add `(maximum: %u), MAX_CLUSTER_COUNT_LIMIT`
💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319395594)
For f172ac5e6c37f446bc3c38419cf40f3adac3ddd8 Use cluster linearization for transaction relay sort order
In addition to changing the inv order, should we also use the transaction's chunk feerate for fee filtering?
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319395594)
For f172ac5e6c37f446bc3c38419cf40f3adac3ddd8 Use cluster linearization for transaction relay sort order
In addition to changing the inv order, should we also use the transaction's chunk feerate for fee filtering?
💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319721247)
34251141eca785578a0b2ca6d1c0932d34b2418d Do not allow mempool clusters to exceed configured limits
This workaround ("Temporarily increase the maximum permitted cluster count so that feature_rbf.py passes. ") didn't seem ideal to me. I shuffled around some of the changes so that feature_rbf is reworked before this commit: https://github.com/glozow/bitcoin/commits/2025-09-cluster-feature_rbf/
It just deletes one of the tests (also see https://github.com/bitcoin/bitcoin/pull/28676#discussion_
...
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2319721247)
34251141eca785578a0b2ca6d1c0932d34b2418d Do not allow mempool clusters to exceed configured limits
This workaround ("Temporarily increase the maximum permitted cluster count so that feature_rbf.py passes. ") didn't seem ideal to me. I shuffled around some of the changes so that feature_rbf is reworked before this commit: https://github.com/glozow/bitcoin/commits/2025-09-cluster-feature_rbf/
It just deletes one of the tests (also see https://github.com/bitcoin/bitcoin/pull/28676#discussion_
...
💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2319744875)
Yes, the specific failure we want to know about is when there is no rescan. Rescanning hides the problem.
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2319744875)
Yes, the specific failure we want to know about is when there is no rescan. Rescanning hides the problem.
💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2319750635)
There's nothing to assert, it either error and throws, or it succeeds and returns. The return value can never be false-y.
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2319750635)
There's nothing to assert, it either error and throws, or it succeeds and returns. The return value can never be false-y.
📝 Crypt-iQ opened a pull request: "net: check for empty header before calling FillBlock"
(https://github.com/bitcoin/bitcoin/pull/33296)
This avoids an Assume crash if multiple blocktxn messages are received. The first call to `FillBlock` would make the header empty via `SetNull` and the second call to `FillBlock` would crash [here](https://github.com/bitcoin/bitcoin/blob/689a32197638e92995dd8eb071425715f5fdc3a4/src/net_processing.cpp#L3333) since `LookupBlockIndex` won't find anything. Fix that by checking for an empty header before the Assume.
(https://github.com/bitcoin/bitcoin/pull/33296)
This avoids an Assume crash if multiple blocktxn messages are received. The first call to `FillBlock` would make the header empty via `SetNull` and the second call to `FillBlock` would crash [here](https://github.com/bitcoin/bitcoin/blob/689a32197638e92995dd8eb071425715f5fdc3a4/src/net_processing.cpp#L3333) since `LookupBlockIndex` won't find anything. Fix that by checking for an empty header before the Assume.
💬 instagibbs commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2319776496)
might note that the header is blanked in FillBlock, but the partial block never deleted, which is why this would catch the behavior
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2319776496)
might note that the header is blanked in FillBlock, but the partial block never deleted, which is why this would catch the behavior
👍 ryanofsky approved a pull request: "Add functional test for IPC interface"
(https://github.com/bitcoin/bitcoin/pull/33201#pullrequestreview-3181743977)
Code review ACK 235016f5b78ba9f472b56df0825690307fffc7e6. Nice to have this ability to test the interfaces directly. Left a few comments but none are important
(https://github.com/bitcoin/bitcoin/pull/33201#pullrequestreview-3181743977)
Code review ACK 235016f5b78ba9f472b56df0825690307fffc7e6. Nice to have this ability to test the interfaces directly. Left a few comments but none are important
💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319744962)
In commit "ci: enable IPC tests in CI" (7eb71775dce6f3f2fd0e4b62f439aab86d1b65a0)
Would seem good to drop --break-system-packages here and only add it on the platforms where it's needed following:
https://github.com/bitcoin/bitcoin/blob/689a32197638e92995dd8eb071425715f5fdc3a4/ci/test/00_setup_env_mac_native.sh#L9-L12
It's also not clear what the problem is if --break-system-packages is not used. Does the package fail to install otherwise?
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319744962)
In commit "ci: enable IPC tests in CI" (7eb71775dce6f3f2fd0e4b62f439aab86d1b65a0)
Would seem good to drop --break-system-packages here and only add it on the platforms where it's needed following:
https://github.com/bitcoin/bitcoin/blob/689a32197638e92995dd8eb071425715f5fdc3a4/ci/test/00_setup_env_mac_native.sh#L9-L12
It's also not clear what the problem is if --break-system-packages is not used. Does the package fail to install otherwise?
💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319703609)
In commit "tests: add functional tests for IPC interface" (592e253f76764f108e9c34fa0da32451091b946a)
capnp_dir is listed twice here
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319703609)
In commit "tests: add functional tests for IPC interface" (592e253f76764f108e9c34fa0da32451091b946a)
capnp_dir is listed twice here
💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319697575)
In commit "tests: add functional tests for IPC interface" (592e253f76764f108e9c34fa0da32451091b946a)
Does this need to say `pip install ./pycapnp`? It seems like on my system without `./` pip ignores the git clone and downloads a wheel instead. Same might apply to the venv instructions below.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319697575)
In commit "tests: add functional tests for IPC interface" (592e253f76764f108e9c34fa0da32451091b946a)
Does this need to say `pip install ./pycapnp`? It seems like on my system without `./` pip ignores the git clone and downloads a wheel instead. Same might apply to the venv instructions below.
💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319701840)
In commit "tests: add functional tests for IPC interface" (592e253f76764f108e9c34fa0da32451091b946a)
s/there is/there is no/
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2319701840)
In commit "tests: add functional tests for IPC interface" (592e253f76764f108e9c34fa0da32451091b946a)
s/there is/there is no/