💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831889)
Thanks, fixed the comment.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831920)
Ah, that is much better, thanks.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831966)
Oops, eliminated now.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831990)
Eliminated the comment.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832046)
Looks like this was leftover from some earlier efforts I had to replace `setEntries` with a vector of `txiter`s instead, but I've lost track of where I was at with that. Eliminated this new `Entries` type for now.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832310)
Looks like a rebase issue (I think there may have been an earlier version of the code where there was a bigger change, and this accidental line reordering was what was left over). Eliminated this commit.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832329)
Thanks, good point! Making this 100x for now.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832356)
I added a later commit to the CTxMemPool class as a whole that adds some documentation for this variable; do you think that is sufficient or would it be helpful to add more?

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832438)
Thanks, fixed.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832459)
This function is gone now.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832531)
Thanks, eliminated.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832556)
Fixed.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832576)
Made some improvements, please let me know how it looks now.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832591)
Thanks, fixed.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832613)
Done!

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346832621)
Thanks, fixed.

💬 sipa commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-3288622966)
Will need rebase after merge of #33354, and including the updated #33157 would be nice to get the reduced TxGraph memory usage savings.

💬 151henry151 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3288868691)
Just following up to make sure this is ready for review. If anybody sees anything that should be addressed or that is not correct here please let me know.

⚠️ vostrnad opened an issue: "Default ASmap file path is not used unless -asmap is set"
(https://github.com/bitcoin/bitcoin/issues/33386)
The `-asmap` option supposedly has a default value `ip_asn.map` (as defined by `DEFAULT_ASMAP_FILENAME`). However, when I tried to put a file with this name into my datadir, nothing happened. It turns out that this default is not actually used unless the `-asmap` option is set, which seems weird since setting the option normally overrides the default. There is a way to set it without overriding the default on the command line (`bitcoind -asmap`), but in bitcoin.conf there doesn't seem to be a wa
...

💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#issuecomment-3289052254)
> It might be nice to have a warning that your headers chain doesn't meet minchainwork though

Given how confusing https://github.com/bitcoin/bitcoin/pull/33336#issuecomment-3277968407 was for many of us, it may be beneficial to extend this PR to show the reason for why script verification was enabled.

Given the number of acks and time constraints, I have to ask @yuvicc, @Eunovo, @hodlinator, @TheCharlatan, @ajtowns, @w0xlt: would that be welcome change, see https://github.com/l0rinc/bitcoi
...