💬 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
...

💬 ellenkampguus commented on issue "v30.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/33368#issuecomment-3289341502)
Maybe not the place to put this remark, but I am very concerned with the fact that the Core maintainers keep pushing through with the controversial removal of the OP_RETURN size filter. To me this change should be at least postponed. If not, I guess it can be concluded that the people controlling Bitcoin Core have been compromised in some way, which I consider a very bad thing. Bitcoin is supposed to be what Satoshi intended, no matter that some changes may be accepted by the community. But it s
...

💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3289368227)
Since most of the reviewers are in favour of using a single table instead of two tables for string and convert params. My recent commit does this; the implementation is motivated by the ryanofsky suggested commit [b998cc5](https://github.com/bitcoin/bitcoin/commit/b998cc52d51b48db9271fdba0bd69e9aaccb7999).