🤔 murchandamus reviewed a pull request: "MiniMiner changes for package linearization"
(https://github.com/bitcoin/bitcoin/pull/28762#pullrequestreview-1712833868)
I guess now post-merge ACK d9cc99d04e813caed51c5f7b6ebdc4c39c8823c9

💬 murchandamus commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381775670)
Would it be possible for these parameters to be in the same order as the assignments below?

💬 murchandamus commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381839339)
It took me until the commit `[MiniMiner] track inclusion order and add Linearize() function` until I understood what this PR was aiming to achieve. Perhaps you could add another sentence to the PR description about the goal of this PR.

💬 murchandamus commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381902909)
Isn’t it the parent that is fee-bumping?

```suggestion
// 3 pairs of grandparent + fee-bumping parent, plus 1 low-feerate child.
```

💬 murchandamus commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381893848)
Do we not know any outpoints or do we just not know whether the ancestor transactions of the submitted set are confirmed?

💬 murchandamus commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381826714)
I also found `manual_entries` vs `m_entries` make me check back a couple times. My understanding is that the `manual_entries` are just fully specified rather than read from the mempool. Would maybe `manually_submitted_entries` fit the bill?

💬 murchandamus commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1381819567)
Is it important that these descendants are cached? Could they maybe just be `descendants`? The `cached_descendants` vs `descendant_caches` is a bit confusing.

🤔 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.