💬 stringintech commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2346797667)
Ah, I see. Thanks!
Just noticed another potential race condition between user checking for index out of bounds and the actual `btck_chain_get_by_height()` call. Like [here](https://github.com/TheCharlatan/rust-bitcoinkernel/blob/92f9e51555dcafb00702e665c865e5d9715a2f4d/src/lib.rs#L1774:L1786) in the rust wrapper; which could trigger the assert condition. 🤔

📝 fanquake reopened a pull request: "contrib: add bash completion for new bitcoin command"
(https://github.com/bitcoin/bitcoin/pull/33385)
Adds a bash completion script for the new bitcoin command-line tool (introduced in #31375), which unifies the main Bitcoin Core executables under a single interface. This feature improves usability, reduces errors, and makes the command-line tools more easily discoverable for users working in a Linux bash environment.

The completion script dynamically lists available commands and options by parsing `bitcoin --help` and `bitcoin help`. It also incorporates the existing bash completions for `bi
...

💬 caesrcd commented on pull request "Bash completion for the new bitcoin wrapper command":
(https://github.com/bitcoin/bitcoin/pull/33383#issuecomment-3288553563)
@fanquake I opened a new (third) PR: #33385
Unfortunately, the bot closed it again, likely due to the same heuristic applied to the previously closed PRs (first-time contributor + new file).

However, as requested, the third PR has been created from a new branch. If possible, I kindly ask that this new PR be reopened.

💬 sipa commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346822062)
I'm wrong about this: https://godbolt.org/z/zKq3fMrEc

Marking resolved.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346825428)
Is it possible that we might have failure in the individual setting with a cluster size limit being exceeded, which then is not the case in a package setting, if some additional package transaction conflicts with transactions in the cluster being added to?

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346830449)
Reworked this a bit -- please let me know if this looks better now or if additional comments would be helpful.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346830740)
Yes, I think so -- I've deleted this comment, as well as the already-commented-out test that is no longer relevant.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346830781)
Thanks! Guessing this was a rebase error; fixed.

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

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

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

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831052)
Thank you! In the process of looking at this again, I noticed that the `mempool_tests` unit test is failing in some intermediate commits here as well. Will go back over that as well.

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

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831407)
Sounds good, I added a commit that does this, if it looks right I'll squash into place.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831450)
Addressed with the inlining of `CalculateParentsOf()`.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831547)
This seems to work if I do:

```c++
const auto& [worst_chunk, feeperweight] = m_txgraph->GetWorstMainChunk();
```

Fixed now.

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

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831692)
Done, and I renamed this variable to `chunk_txs` for clarity.

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

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346831841)
Oops, reverted my change.