💬 optout21 commented on pull request "miner: fix empty mempool case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3401411223)
Concept ACK
Maybe add the new test in a first commit (showing how it fails), and then the fix in a new commit?
For test change without fix, and fix in see commits 3a7e58ec81fb2caeef1cf369d47b0a247d923180 and 3220ce81b1df090895daa32dfb5a1af003d6d88b
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3401411223)
Concept ACK
Maybe add the new test in a first commit (showing how it fails), and then the fix in a new commit?
For test change without fix, and fix in see commits 3a7e58ec81fb2caeef1cf369d47b0a247d923180 and 3220ce81b1df090895daa32dfb5a1af003d6d88b
🤔 naiyoma reviewed a pull request: "Broadcast own transactions only via short-lived Tor or I2P connections"
(https://github.com/bitcoin/bitcoin/pull/29415#pullrequestreview-3335207073)
Concept ACK
Tested consecutively for 5 days on Signet,
<details>
<summary> my data from those 5 days</summary>
- Day 1
Time to hear back: 11 seconds
Connections: 3 attempted, 3 successful
- Day 2
Time to hear back: 72 seconds (1 min 12 sec)
Connections: 3 attempted, 1 successful
- Day 3
Time to hear back: 14 seconds
Connections: 3 attempted, 3 successful
- Day 4
Time to hear back: 15 seconds
Connections: 3 attempted, 1 successful
- Day 5
Time to he
...
(https://github.com/bitcoin/bitcoin/pull/29415#pullrequestreview-3335207073)
Concept ACK
Tested consecutively for 5 days on Signet,
<details>
<summary> my data from those 5 days</summary>
- Day 1
Time to hear back: 11 seconds
Connections: 3 attempted, 3 successful
- Day 2
Time to hear back: 72 seconds (1 min 12 sec)
Connections: 3 attempted, 1 successful
- Day 3
Time to hear back: 14 seconds
Connections: 3 attempted, 3 successful
- Day 4
Time to hear back: 15 seconds
Connections: 3 attempted, 1 successful
- Day 5
Time to he
...
💬 Sjors commented on pull request "miner: fix empty mempool case for waitNext()":
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3401436340)
@optout21 the test-each-commit job enforces that each commit has a passing test.
(https://github.com/bitcoin/bitcoin/pull/33566#issuecomment-3401436340)
@optout21 the test-each-commit job enforces that each commit has a passing test.
💬 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.