💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1293979442)
I checked that 1ṩ may break `assert(result_bnb->GetChange(coin_params.m_cost_of_change, CAmount{0}) == 0);`

💬 glozow commented on pull request "Package Relay 1/3: Introduce TxPackageTracker as Orphan Resolution Module":
(https://github.com/bitcoin/bitcoin/pull/28031#discussion_r1293981371)
This is gone now, replaced with registration of `ConnectionInfo` when the node first connects.

💬 glozow commented on pull request "Package Relay 1/3: Introduce TxPackageTracker as Orphan Resolution Module":
(https://github.com/bitcoin/bitcoin/pull/28031#discussion_r1293981811)
Removed the special casing. I can't remember why it comes into play later, but if I do, I'll add it when it's needed.

💬 achow101 commented on pull request "Break up script/standard.{h/cpp}":
(https://github.com/bitcoin/bitcoin/pull/28244#discussion_r1293985018)
leaving as is since it is only moved.

💬 achow101 commented on pull request "Break up script/standard.{h/cpp}":
(https://github.com/bitcoin/bitcoin/pull/28244#discussion_r1293997139)
Done

💬 achow101 commented on pull request "Break up script/standard.{h/cpp}":
(https://github.com/bitcoin/bitcoin/pull/28244#discussion_r1293997252)
Added comment as suggested

💬 achow101 commented on pull request "Break up script/standard.{h/cpp}":
(https://github.com/bitcoin/bitcoin/pull/28244#discussion_r1293997485)
Cherry picked 461259c4ecc1e48d028eaea56061e72a6667ce4f from #26291

💬 achow101 commented on pull request "Break up script/standard.{h/cpp}":
(https://github.com/bitcoin/bitcoin/pull/28244#discussion_r1293997590)
Dropped the reference

💬 achow101 commented on pull request "Break up script/standard.{h/cpp}":
(https://github.com/bitcoin/bitcoin/pull/28244#discussion_r1293997660)
Dropped those comments.

💬 achow101 commented on pull request "Break up script/standard.{h/cpp}":
(https://github.com/bitcoin/bitcoin/pull/28244#issuecomment-1678113685)
> Did you use some script for [b3af9ce](https://github.com/bitcoin/bitcoin/commit/b3af9cea5806d26dc3e8f397a1de870065611648)?

Ran IWYU with manual cleanup.

💬 Retropex commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#discussion_r1294000727)
@jonatack Do I have to add other elements?

💬 murchandamus commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1294000827)
Ah, my bad, I thought this target referred to the sum of recipient outputs, but it’s actually `recipient_sum + not_input_fees`.

Is it possible that this then would need to be at least as large as the cost for the header + change output? 1000 ṩ/vB might not be enough for every feerate then.

💬 achow101 commented on pull request "wallet: Use CTxDestination in CRecipient instead of just scriptPubKey":
(https://github.com/bitcoin/bitcoin/pull/28246#discussion_r1294012514)
Should be fixed.

💬 achow101 commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1678144469)
Please see https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#release-notes for how release notes should be added for a PR.

💬 iBeizsley commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1678145113)
> You're arguing a logical fallacy here. You might as well argue that the vast majority of people don't even run nodes, so there's no reason for any node to distribute any transactions or blocks at all.

Not at all. I'm arguing that

> the purpose of a Bitcoin node is to relay transactions

Is false. It's one job of some nodes, but it's optional, and which transactions get relayed is dependent on all kinds of things, including whether your mempool is currently full. I could shrink my mempool to
...

💬 jonatack commented on pull request "test doc: tests `acceptstalefeeestimates` option is only supported on regtest chain":
(https://github.com/bitcoin/bitcoin/pull/28157#discussion_r1294018470)
Thanks for looking into it! Running the updated test with mainnet/testnet3/signet nodes running, the mainnet test passed. However, it still fails for the testnet3 and signet cases.

💬 jonatack commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#discussion_r1294027661)
The note should probably refer to the `-permitbaremultisig` configuration option.

🤔 glozow reviewed a pull request: "validate package transactions with their in-package ancestor sets"
(https://github.com/bitcoin/bitcoin/pull/26711#pullrequestreview-1577754051)
Rebased on top of #28251 (which also knocked out 2 commits) and addressed comments

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1294031657)
Added that to the comment :+1:

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1294032183)
Yeah agreed, makes it much more important that there isn't e.g. a crash bug in there somewhere, and if anything goes wrong we should quit gracefully and default to topo sort.