🤔 furszy reviewed a pull request: "wallet: Have the wallet store the key for automatically generated descriptors"
(https://github.com/bitcoin/bitcoin/pull/26728#pullrequestreview-1713009811)
Code review ACK dfb58e4

💬 furszy commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1381905163)
In 97de2a4af:
A note explaining why this is not needed would be nice.

💬 furszy commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1381885030)
nit:
Could also check that all active descriptors have this new xprv.

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

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1381951356)
I don't think this is a substantially useful optimization, and it still results in mixing upgrading code with the loading which I would rather not do.

📝 glozow opened a pull request: "validation: return more helpful results for reconsiderable fee failures and skipped transactions"
(https://github.com/bitcoin/bitcoin/pull/28785)
Split off from #26711 (suggested in https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1786392253). This is part of #27463.

- Add 2 new TxValidationResults
- `TX_SINGLE_FAILURE` helps us encode transactions who have failed fee checks that can be bypassed using package validation. This is distinguished from `TX_MEMPOOL_POLICY` so that we re-validate a transaction if and only if it is eligible for package CPFP.
- `TX_UNKNOWN` helps us communicate that we aborted package validation
...

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