💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2064733664)
Done

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2064735230)
I don't think a separate function is necessary. This code is unlikely to be reused.

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2064737641)
I've added size checks. I did not gate behind a `IsPayToTaproot` as the fields should only be set if the output script is taproot anyways.

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2064738574)
Meh, maybe someone can do that in a followup.

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2064740358)
I don't think it's really necessary as these log lines are in the PSBT test, and the only way to sign with PSBTs is with `*processpsbt`.

💬 achow101 commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2064741305)
Added a sentence to the commit message.

🚀 glozow merged a pull request: "test: Test that migration automatically repairs corrupted metadata with doubled derivation path"
(https://github.com/bitcoin/bitcoin/pull/29124)

🤔 achow101 reviewed a pull request: "descriptors: Reject + sign while parsing unsigned"
(https://github.com/bitcoin/bitcoin/pull/32365#pullrequestreview-2800976553)
ACK fa655da159876861251d1149a5c31a830bd35596

💬 achow101 commented on pull request "descriptors: Reject + sign while parsing unsigned":
(https://github.com/bitcoin/bitcoin/pull/32365#discussion_r2064772334)
In fa6f77ed3c152e0ff695c36b7a4ebb2c2efe916f "descriptors: Reject + sign in ParseKeyPathNum"

Perhaps also a test for `-`?

💬 achow101 commented on pull request "test: Add missing check for empty stderr in util tester":
(https://github.com/bitcoin/bitcoin/pull/32327#issuecomment-2836763846)
ACK fadf12a56c294696052c4cb6ee5284030ada7498

💬 dani69654 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2836782764)
How a P2P money network became a file storage system will make a great case study in the future.

🚀 achow101 merged a pull request: "test: Add missing check for empty stderr in util tester"
(https://github.com/bitcoin/bitcoin/pull/32327)

📝 achow101 opened a pull request: "test: Use the correct node for doubled keypath test"
(https://github.com/bitcoin/bitcoin/pull/32369)
#29124 had a silent merge conflict with #32350 which resulted in it using the wrong node. Fix the test to use the correct v22 node.

💬 andrewtoth commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2836929583)
> An entire file can be serialized directly after an OP_RETURN as-is. So later, when a Bitcoin user goes to do data recovery, or has their system seized by a government and they do a cursory analysis of its contents, both will have to contend with the now completely plaintext arbitrary data that any anonymous person has now effectively forced them to store and/or process on their system in order to participate in the Bitcoin network.

Luckily, blocks and mempool data are stored obfuscated on d
...

💬 cmdruid commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2837019460)
The limitation to a single OP_RETURN output per tx is indeed dumb and should be removed, but I don't see why you would remove filtering rules if people actively use them and build clients around them.

There's always going to be a difference in policy between mempool and onchain. Just because you can bribe a miner to side-step mempool consensus isn't really an argument to remove filtering rules, you may as well remove the dust limit while you're at it.

🤔 Turtlecute33 reviewed a pull request: "Remove arbitrary limits on OP_Return (datacarrier) outputs"
(https://github.com/bitcoin/bitcoin/pull/32359#pullrequestreview-2801335235)
NACK

Removing the limits on OP_RETURN does not mean that people minting ordinals will stop using Taproot outputs, so it will not prevent the continued growth of the UTXO set.

Any non-definitive restriction on mempool policy is essentially useless, as it can be easily circumvented via slipstream techniques and accelerator services. Meanwhile, removing arbitrary limits on data carrier outputs does not guarantee any meaningful improvement or change in the behavior of ordinal minters.

Given
...

🤔 danielabrozzoni reviewed a pull request: "Remove arbitrary limits on OP_Return (datacarrier) outputs"
(https://github.com/bitcoin/bitcoin/pull/32359#pullrequestreview-2801386763)
While there are valid arguments on both sides of this discussion, I am leaning towards a NACK

It is true that mempool policy filters have not been completely successful at preventing transaction spam, and they will never be. However, they do provide a form of friction if enough people use them; for example, if all mempools refuse transactions with OP_RETURNs above the current limit, spammers have no choice but to use mempool accelerators and pay higher fees for their transactions.

I also a
...

💬 petertodd commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2837164692)
I want to remind people that you should actually read the mailing list discussion first, and understand what exactly lead to this pull-req: https://groups.google.com/g/bitcoindev/c/d6ZO7gXGYbQ

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r2065255362)
I had forgotten why I did it that way, but when I tried that, testing the first commit results in warnings that the helper functions are unused.

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r2065270057)
Taken, thanks