💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1706936276)
I don't have a strong view on it, but this commit just reuses the existing `IsHexNumber` tests so I'd rather not change that here.

💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1706949124)
Makes sense, taken, thanks.

💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1706931724)
> though this contains an extra evennes requirement which we may not want here.

Exactly

💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1706935424)
Yes, since we're dealing with numbers specifically.

💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1706946286)
Yeah I like your approach better, taken, thanks.

💬 stickies-v commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1707005179)
Yeah that'll work, ~taken, thanks.

👍 paplorinc approved a pull request: "node: reduce unsafe uint256S usage"
(https://github.com/bitcoin/bitcoin/pull/30569#pullrequestreview-2225466836)
ACK d045fc7ac1729cf29140c43518f20375f2aaa1cc

Note that I don't know the second order effects of constraining previous public input values, somebody else should assess that part

💬 gmaxwell commented on pull request "Halt processing of unrequested transactions v2":
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2273685985)
Running this exposes that nodes are currently making duplicate transaction requests not too infrequently. ::facepalm:: What happens is that when Peer1 announces {parent,child} and Peer2 announces {parent,child} and we request parent from 1 and child from 2, but 2 responds faster, then orphan handling will request parent by txid from 2. Then it gets the parent by txid from 2. Then when peer 1 replies to the request for parent (sending a duplicate transaction), the current behavior of this PR f
...

👍 tdb3 approved a pull request: "ci: Silent Homebrew's reinstall warnings"
(https://github.com/bitcoin/bitcoin/pull/30591#pullrequestreview-2225476443)
cr ut ACK 032ebe5be4dfc3eb3d3693f1284fac6458bacbf3

💬 maflcko commented on issue "implicit-integer-sign-change in ActivateSnapshot":
(https://github.com/bitcoin/bitcoin/issues/30514#issuecomment-2273721994)
I think there was a slight miscommunication. My reply was to your point of a "large fork", whose check (currently) happens after the chain params are asked for the hash: https://github.com/bitcoin/bitcoin/blob/676abd1af754964858a60fddffb9a12c0a6c2749/src/validation.cpp#L5663 .

You also raised a different point about a user downloading a too recent snapshot. This is a different scenario, however the same check in validation will hit it currently. I agree that the error message can be improved
...

💬 maflcko commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#issuecomment-2273742263)
> Thanks for the review everyone. Maintainers: there is still a determinism issue here as per Niklas, which i have yet to re-reproduce.

Are you sure? I think may be confused with https://github.com/bitcoin/bitcoin/pull/28216#issuecomment-2258390205 ?

Even if not, I wonder if stability is a hard blocker for a fuzz target. Obviously, it should be fixed, but this can be done in a follow-up, if the fuzz target otherwise makes sense to merge.

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2273780176)
reviewed through https://github.com/bitcoin/bitcoin/pull/30286/commits/06ea1016ff40ebcf3e668bdb993a0747904cb40f

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1706029402)
in 29cac3e85b23f1264121cbd50e4a09755b0c8352

I think this commit may benefit from being two smaller commits, one that demonstrates the caching, and a second that demonstrates using it to skip the splitting computation?

I do not understand what it's doing, as-is.

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1705789091)
I do realize the argument changes later, so maybe this just on the later commit
```Suggestion
queue.emplace_back(/*inc=*/SetInfo<SetType>{}, /*und=*/std::move(component), /*pot_feerate=*/FeeFrac{});
```

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1707280073)
why 64?

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1707294690)
why 4?

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1705747742)
I think this can be used with DepGraphFormatter during `Unser` with reversed `reordering`

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1704526813)
*non-empty* topological subsets?

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1705913641)
I would have appreciated slightly more pedantic comments demonstrating how the computation matches the comments :sweat:

💬 instagibbs commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#discussion_r1705666485)
move of `m_todo` instantiation seems like a no-op?

🤔 TheCharlatan reviewed a pull request: "multiprocess: Add -ipcbind option to bitcoin-node"
(https://github.com/bitcoin/bitcoin/pull/30509#pullrequestreview-2223960433)
lgtm , I hope we can finally get this project under way now.