💬 vasild commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#discussion_r1381986214)
Yes, this was my doubt too. But then it is unrealistic to expect that banman ser/deser will produce identical result as the input. Another try - what about feeding it anything (including invalid stuff), but keeping track if we ever fed it an invalid address or subnet and if yes, then don't do the assert at the end `assert(banmap == banmap_read);`?

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1381994842)
Done

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1381995099)
This ended up being unnecessary and I just forgot to remove it.

💬 maflcko commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#discussion_r1381998308)
> Another try - what about feeding it anything (including invalid stuff), but keeping track if we ever fed it an invalid address or subnet and if yes, then don't do the assert at the end `assert(banmap == banmap_read);`?

sgtm

🤔 theStack reviewed a pull request: "MiniMiner changes for package linearization"
(https://github.com/bitcoin/bitcoin/pull/28762#pullrequestreview-1713192441)
post-merge code-review ACK d9cc99d04e813caed51c5f7b6ebdc4c39c8823c9

💬 theStack commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1382002278)
```suggestion
// CPFP low + double low
```

💬 instagibbs commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#issuecomment-1792853401)
think you'll have to touch `CheckATMPInvariants` in the tx_pool fuzz test to add `TX_SINGLE_FAILURE` case

💬 TheCharlatan commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1382012974)
Yeah, missed that. Will add.

📝 fanquake opened a pull request: "guix: switch to 6.1 kernel headers over 5.15"
(https://github.com/bitcoin/bitcoin/pull/28786)
6.1 is the current longterm release: https://kernel.org/.

Note that using an older version of the kernel headers inside Guix, is not a "hack" for compatibility, and is explicitly recommended against by glibc:

https://sourceware.org/glibc/wiki/FAQ#What_version_of_the_Linux_kernel_headers_should_be_used.3F.

so using the latest version of the longterm headers seems appropriate.

The last time we changed this was when we consolidated all builds to 5.15, in #25006.

<!--
*** Please remo
...

💬 brunoerg commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#discussion_r1382026027)
> Yes, this was my doubt too. But then it is unrealistic to expect that banman ser/deser will produce identical result as the input. Another try - what about feeding it anything (including invalid stuff), but keeping track if we ever fed it an invalid address or subnet and if yes, then don't do the assert at the end assert(banmap == banmap_read);?

sgtm as well. Can I change the approach so?

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1792874071)
> it would also be nice to test the scenario where the process does not find the expected number of active descriptors. This should result in the wallet having no active xpub key.

Added to a test that imports a few descriptors whose xpub won't be chosen for active xpub.

💬 maflcko commented on pull request "guix: switch to 6.1 kernel headers over 5.15":
(https://github.com/bitcoin/bitcoin/pull/28786#issuecomment-1792875439)
Is there a way to just use the latest headers or the latest LTS header, so that this is automatically and implicitly done in a time-machine bump?

🤔 mzumsande reviewed a pull request: "net: improves addnode / m_added_nodes logic"
(https://github.com/bitcoin/bitcoin/pull/28155#pullrequestreview-1713240605)
Tested ACK 0420f99f429ce2382057e101859067f40de47be0

💬 glozow commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1382032899)
It's just "I can't give an answer if I don't know what the question is." When using this constructor, the user is not specifying which outpoints they want bump fees for.

💬 glozow commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#issuecomment-1792885494)
Thanks for all the reviews :)

💬 fanquake commented on pull request "guix: switch to 6.1 kernel headers over 5.15":
(https://github.com/bitcoin/bitcoin/pull/28786#issuecomment-1792902309)
I can't see anything obvious that would do that. Currently the only non-versioned package in Guix (linux-kernel-headers), is a pinned, older version of the 5.15.x branch.

💬 TheCharlatan commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1792927186)
Updated fc5e4dea6ffa21191e852a79103a2dfc9c241df1 -> a20c1d2b81a702f97267d4f098f13ed80e9320cf ([simplifyMemPoolInteractions_6](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_6) -> [simplifyMemPoolInteractions_7](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_7), [compare](https://github.com/TheCharlatan/bitcoin/compare/simplifyMemPoolInteractions_6..simplifyMemPoolInteractions_7))

* Addressed @glozow's [comment](https://github.com/bitcoin/bi
...

💬 hashbender commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1792957112)
It would be nice if it was on by default

📝 theStack opened a pull request: "init: completely remove `-zapwallettxes` (remaining hidden option)"
(https://github.com/bitcoin/bitcoin/pull/28787)
The `-zapwallettxes` functionality has been removed in v0.21.0 (see commit 3340dbadd38f5624642cf0e14dddbe6f83a3863b / PR #19671), with the parameter being kept as hidden option, to inform users via an exit error that `abandontransaction` should be used instead.

As any guides that still suggest to use `-zapwallettxes` would refer to a Bitcoin Core version that is EOL since many years (i.e. <= v0.20.x), it is highly unlikely that the error caused by the option is still relevant for any user, he
...

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1382115534)
Thank you removed