💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1792767373)
> add TX_SINGLE_FAILURE / TX_UNKNOWN + tests (+ package-fee-too-low?)

This has been split off in #28785

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1381955563)
Changed to "must"

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1381956847)
Changed to `MarkAsInMempool` to also include the case where it has been found as well as the case where we have submitted it.

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1381957028)
Taken

💬 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-1792771581)
> so that we re-validate a transaction if and only if it is eligible for package CPFP.

and in the future we'll use this for making sure we'd re-request this transaction as part of another package at p2p, correct?

💬 glozow commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#issuecomment-1792774024)
> and in the future we'll use this for making sure we'd re-request this transaction as part of another package at p2p, correct?

Correct :+1:

💬 theuni commented on pull request "build: remove `-bind_at_load` usage":
(https://github.com/bitcoin/bitcoin/pull/28783#issuecomment-1792795467)
> Yuck. The only thing I can think of is a sledgehammer hack similar to this ancient one: [a98356f](https://github.com/bitcoin/bitcoin/commit/a98356fee8a44d7d1cb37f22c876fff8f244365e)

See https://github.com/theuni/bitcoin/commit/c61c9c5c772780be95ba7a8221ea13a72fe97d99 which is ugly but seems to work as intended.

💬 glozow commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1381974204)
Concept ACK to handing out references to the entry instead of iterators into mapTx

ISTM that this should require holding `CTxMemPool::cs`?

💬 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