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

⚠️ fanquake pinned an issue: "30.0 RC Testing Guide Feedback"
(https://github.com/bitcoin/bitcoin/issues/33369)
This issue is to discuss the [30.0 Release Candidate Testing Guide](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/30.0-Release-Candidate-Testing-Guide). If you have any feedback on the document, please leave a comment here.

Note: This is for feedback on the document, not on Bitcoin Core or on the 30.0 changes. Create a new issue or give feedback in the [v30.0 Testing](https://github.com/bitcoin/bitcoin/issues/33368) issue

Thank you for taking a look at the guide and leaving your feedbac
...

⚠️ TheCharlatan opened an issue: "Assertion hit on shutdown of bitcoin-node with connected mining interface client"
(https://github.com/bitcoin/bitcoin/issues/33387)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

Running c0894a0a2be032cd9a5d5945643689230ab10255 `bitcoin-node` with Sjor's [sv2-tp] connected resulted in an unclean shutdown:
```
2025-09-14T10:24:34Z CreateNewBlock(): block weight: 460455 txs: 371 fees: 230840 sigops 1336
2025-09-14T10:25:05Z CreateNewBlock(): block weight: 513153 txs: 441 fees: 290027 sigops 1489
2025-09-14T10:25:36Z CreateNewBlock(): block weight: 635048 txs: 541 f
...

✅ TheCharlatan closed an issue: "Assertion hit on shutdown of bitcoin-node with connected mining interface client"
(https://github.com/bitcoin/bitcoin/issues/33387)

💬 TheCharlatan commented on issue "Assertion hit on shutdown of bitcoin-node with connected mining interface client":
(https://github.com/bitcoin/bitcoin/issues/33387#issuecomment-3289467468)
Tried reproducing it again from scratch and could not. I think this was just a version mismatch. There is a crash on the sv2 side, but that is for a different issue tracker. Closing.

💬 jonasnick commented on issue "v30.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/33368#issuecomment-3289485882)
rc1 passes the nix-bitcoin integration test framework (except for the joinmarket tests because the latest joinmarket release requires the legacy wallet). You can run the tests with [this script](https://gist.github.com/jonasnick/cd52b4463b720a5fb1406c2cbabaf936) (requires nix).

It seems like Cap'n Proto is a new dependency. Should it be mentioned in the release notes?

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2347326031)
Currently we trigger an assertion failure if a passed in index is out of bounds when we have a function that allows the caller to first check the allowed range. In the chain data structure, we don't have a direct function for this at the moment, the user is forced to first call `btck_chain_get_tip` and then call `btck_chain_get_by_height`. I think we should probably add another function there to get the height of the chain directly to make the api a bit more symmetric. We could be a bit friendli
...

💬 sipa commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2347345010)
This works:

```c++
std::sort(iters.begin(), iters.end(), [this](const auto& a, const auto& b) EXCLUSIVE_LOCKS_REQUIRED(cs) noexcept {
return this->m_txgraph->CompareMainOrder(*a, *b) < 0;
});
```

💬 stringintech commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2347348366)
It is not clear to me what other mutable data structures we can have in the future, but at least for the chain it seems better to return nullptr than allow potential crashes in race conditions (though I am not sure how often it could happen in practice).

💬 sipa commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-3289572395)
See https://github.com/sipa/bitcoin/commits/pr28676 for a branch with:
* Rebase after #33354 merge.
* Updated #33157 as base.
* Addressed https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2328805583