💬 optout21 commented on pull request "miner: fix empty mempool case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3401479381)
> @optout21 the test-each-commit job enforces that each commit has a passing test.
I'm not sure I understand this correctly, but if you mean that all tests should pass with each commit -- yes, first commit has test checks matching the non-fixed code (i.e. `BOOST_REQUIRE(should_be_nullptr == nullptr);`), and fix commit also adapts this one check. It's more clear looking at the fix commit what is the change in behavior, if adding the test and fix are separated.
Leaving in one commit is also fi
...
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3401479381)
> @optout21 the test-each-commit job enforces that each commit has a passing test.
I'm not sure I understand this correctly, but if you mean that all tests should pass with each commit -- yes, first commit has test checks matching the non-fixed code (i.e. `BOOST_REQUIRE(should_be_nullptr == nullptr);`), and fix commit also adapts this one check. It's more clear looking at the fix commit what is the change in behavior, if adding the test and fix are separated.
Leaving in one commit is also fi
...
🤔 ismaelsadeeq reviewed a pull request: "cluster mempool: control/optimize TxGraph memory usage"
(https://github.com/bitcoin/bitcoin/pull/33157#pullrequestreview-3335404536)
reACK 023cd5a5469ad61205bf7bb1135895f2b4a20ea9 🚢
[ad350d7..023cd5a](https://github.com/bitcoin/bitcoin/compare/7421e58dee402ee8b5b58a18684953a89ad350d7..023cd5a5469ad61205bf7bb1135895f2b4a20ea9)
Several nits from recent comments were addressed, clarifying comments were added for clarity.
(https://github.com/bitcoin/bitcoin/pull/33157#pullrequestreview-3335404536)
reACK 023cd5a5469ad61205bf7bb1135895f2b4a20ea9 🚢
[ad350d7..023cd5a](https://github.com/bitcoin/bitcoin/compare/7421e58dee402ee8b5b58a18684953a89ad350d7..023cd5a5469ad61205bf7bb1135895f2b4a20ea9)
Several nits from recent comments were addressed, clarifying comments were added for clarity.
📝 yancyribbens opened a pull request: "docs: add doc comment for SRD selection algorithm"
(https://github.com/bitcoin/bitcoin/pull/33622)
Other selection algorithms provide a description of workings and parameters. This PR adds such a comment for SRD.
(https://github.com/bitcoin/bitcoin/pull/33622)
Other selection algorithms provide a description of workings and parameters. This PR adds such a comment for SRD.
👍 TheCharlatan approved a pull request: "multiprocess: Fix high overhead from message logging"
(https://github.com/bitcoin/bitcoin/pull/33517#pullrequestreview-3335516834)
ACK 0626b90f507db68610a69feec86deb712dd095a1
(https://github.com/bitcoin/bitcoin/pull/33517#pullrequestreview-3335516834)
ACK 0626b90f507db68610a69feec86deb712dd095a1
📝 willcl-ark opened a pull request: "doc: document capnproto and libmultiprocess deps in 29.x"
(https://github.com/bitcoin/bitcoin/pull/33623)
Closes: #33576
These dependencies are both currently undocumented, and libmultiprocess has a relatively special requirement in that the documented version is the **latest** that will ever work with this version of Bitcoin Core.
(https://github.com/bitcoin/bitcoin/pull/33623)
Closes: #33576
These dependencies are both currently undocumented, and libmultiprocess has a relatively special requirement in that the documented version is the **latest** that will ever work with this version of Bitcoin Core.
💬 vasild commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2429141020)
Change to `InitiateTxBroadcastToAll()`?
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2429141020)
Change to `InitiateTxBroadcastToAll()`?
💬 sipa commented on pull request "doc: document capnproto and libmultiprocess deps in 29.x":
(https://github.com/bitcoin/bitcoin/pull/33623#issuecomment-3401795935)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33623#issuecomment-3401795935)
Concept ACK
🤔 sipa reviewed a pull request: "multiprocess: Fix high overhead from message logging"
(https://github.com/bitcoin/bitcoin/pull/33517#pullrequestreview-3335624700)
utACK 0626b90f507db68610a69feec86deb712dd095a1
(https://github.com/bitcoin/bitcoin/pull/33517#pullrequestreview-3335624700)
utACK 0626b90f507db68610a69feec86deb712dd095a1
💬 sipa commented on pull request "test, refactor: Embedded ASmap selected preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2429188245)
Sounds like we need to agree on what approach to take here before this can move forward?
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2429188245)
Sounds like we need to agree on what approach to take here before this can move forward?
💬 achow101 commented on issue "`generatetoaddress` 2-3x slower on v30 compared to v29":
(https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3401869814)
> It seems the commit that changed the behavior was [7bacabb](https://github.com/bitcoin/bitcoin/commit/7bacabb204b6c34f9545f0b37e2c66296ad2c0de)
That doesn't make any sense. That commit doesn't touch anything related to how addresses are generated.
(https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3401869814)
> It seems the commit that changed the behavior was [7bacabb](https://github.com/bitcoin/bitcoin/commit/7bacabb204b6c34f9545f0b37e2c66296ad2c0de)
That doesn't make any sense. That commit doesn't touch anything related to how addresses are generated.
✅ stickies-v closed a pull request: "rest/rpc: use more util::Join"
(https://github.com/bitcoin/bitcoin/pull/32942)
(https://github.com/bitcoin/bitcoin/pull/32942)
💬 stickies-v commented on pull request "rest/rpc: use more util::Join":
(https://github.com/bitcoin/bitcoin/pull/32942#issuecomment-3401889578)
Closing for lack of reviewer interest, it's not a critical change.
(https://github.com/bitcoin/bitcoin/pull/32942#issuecomment-3401889578)
Closing for lack of reviewer interest, it's not a critical change.
💬 sipa commented on issue "`generatetoaddress` 2-3x slower on v30 compared to v29":
(https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3401891333)
@achow101 But it does touch things related to new blocks being created. Maybe the overhead of wallet writing the best block to the wallet more frequently has an impact here?
(https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3401891333)
@achow101 But it does touch things related to new blocks being created. Maybe the overhead of wallet writing the best block to the wallet more frequently has an impact here?
✅ stickies-v closed a pull request: "cmake: fatal error when PIE not supported"
(https://github.com/bitcoin/bitcoin/pull/33282)
(https://github.com/bitcoin/bitcoin/pull/33282)
💬 stickies-v commented on pull request "cmake: fatal error when PIE not supported":
(https://github.com/bitcoin/bitcoin/pull/33282#issuecomment-3401891361)
Closing for lack of reviewer interest, it's not a critical change.
(https://github.com/bitcoin/bitcoin/pull/33282#issuecomment-3401891361)
Closing for lack of reviewer interest, it's not a critical change.
✅ stickies-v closed a pull request: "cmake: make missing Python interpreter behaviour more explicit"
(https://github.com/bitcoin/bitcoin/pull/33278)
(https://github.com/bitcoin/bitcoin/pull/33278)
💬 stickies-v commented on pull request "cmake: make missing Python interpreter behaviour more explicit":
(https://github.com/bitcoin/bitcoin/pull/33278#issuecomment-3401892689)
Closing for lack of reviewer interest, it's not a critical change.
(https://github.com/bitcoin/bitcoin/pull/33278#issuecomment-3401892689)
Closing for lack of reviewer interest, it's not a critical change.
💬 purpleKarrot commented on pull request "Update `minisketch` subtree and switch to its build script":
(https://github.com/bitcoin/bitcoin/pull/32856#discussion_r2429246951)
I am not suggesting to use `block()` here. What I am suggesting is to use `function()` in a way that can be replaced with `block()` as soon as the minimum required version can be increased. Since we already know the direction of development, why add more legacy code just for the sake of consistency?
(https://github.com/bitcoin/bitcoin/pull/32856#discussion_r2429246951)
I am not suggesting to use `block()` here. What I am suggesting is to use `function()` in a way that can be replaced with `block()` as soon as the minimum required version can be increased. Since we already know the direction of development, why add more legacy code just for the sake of consistency?
💬 fjahr commented on pull request "test, refactor: Embedded ASmap selected preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2429247548)
There has been some further discussion on this in #33386 and I wasn't able to address it yet. I will drop the commit from here and open two separate PRs because there doesn't seem to be a clear outcome of the discussion yet. One adding the commit here and improving docs and the other splitting asmap= into asmap=/asmapfile= which is certainly the cleaner alternative but changes behavior and it's a bit messy to redefine behavior of asmap= just slightly.
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2429247548)
There has been some further discussion on this in #33386 and I wasn't able to address it yet. I will drop the commit from here and open two separate PRs because there doesn't seem to be a clear outcome of the discussion yet. One adding the commit here and improving docs and the other splitting asmap= into asmap=/asmapfile= which is certainly the cleaner alternative but changes behavior and it's a bit messy to redefine behavior of asmap= just slightly.
💬 furszy commented on issue "`generatetoaddress` 2-3x slower on v30 compared to v29":
(https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3401965782)
> But it does touch things related to new blocks being created. Maybe the overhead of wallet writing the best block to the wallet more frequently has an impact here?
we might want to revive #25297 in that case.
(https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3401965782)
> But it does touch things related to new blocks being created. Maybe the overhead of wallet writing the best block to the wallet more frequently has an impact here?
we might want to revive #25297 in that case.