💬 petertodd commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2064504791)
I included this patch and @darosior's patch verbatim.

💬 achow101 commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2064505718)
Indeed, removed.

💬 achow101 commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2064508912)
Apparently since this PR was opened, descriptor specific tests were added to those files. I've removed the legacy ones from them and added back to the `CMakeLists.txt`.

💬 petertodd commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#discussion_r2064510978)
Fixed.

💬 achow101 commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2064511069)
Removed

💬 achow101 commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2064514419)
Removed from the help text.

I've cleaned up some of the legacy wallet mentions that are no longer relevant. Some are still relevant, and others are less obviously legacy wallet only things so I will leave those for a followup.

💬 achow101 commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2064514946)
Done

💬 achow101 commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2064515535)
Removed

💬 petertodd commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2836467665)
@Sjors @darosior I think I made all the code corrections you both suggested. Let me know if I missed anything.

💬 achow101 commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2836470205)
> And a few details:

Removed these

> Also it might be good to rename the `importprivkey` test helper to e.g. `importkey`, so it's not confused with the disappeared RPC method.

I'm going to leave that for a followup.

> You can use `--patience`. Maybe add this to the commit message?

Done, also mentioned `--histogram`.

💬 BullishNode commented on issue "Allow sending untrusted utxos in the sendtoaddress api":
(https://github.com/bitcoin/bitcoin/issues/32034#issuecomment-2836503702)
Yes. Unconfirmed change is considered trusted.

💬 achow101 commented on pull request "ci: Use previous releases in tests on Windows":
(https://github.com/bitcoin/bitcoin/pull/32363#discussion_r2064558105)
Why is this restricted to 22.0 and above? The naming scheme for the Windows zips hasn't changed.

💬 achow101 commented on pull request "ci: Use previous releases in tests on Windows":
(https://github.com/bitcoin/bitcoin/pull/32363#discussion_r2064566628)
imports go at the top of the file.

💬 achow101 commented on pull request "ci: Use previous releases in tests on Windows":
(https://github.com/bitcoin/bitcoin/pull/32363#discussion_r2064565125)
Why restrict to 22.0 and above?

💬 achow101 commented on pull request "ci: Use previous releases in tests on Windows":
(https://github.com/bitcoin/bitcoin/pull/32363#discussion_r2064544864)
Please put these with their respective versions above.

If we're going to do Windows testing as well, then we should be downloading all of the versions above, not just this small subset. This subset doesn't even make sense, there's no single test that uses just these versions. `wallet_migration.py` uses only 28.0, the rest require the older versions too.

💬 achow101 commented on pull request "ci: Use previous releases in tests on Windows":
(https://github.com/bitcoin/bitcoin/pull/32363#discussion_r2064578902)
What is the purpose of this host triple? We don't make Windows binaries with any other host triple.

💬 achow101 commented on pull request "ci: Use previous releases in tests on Windows":
(https://github.com/bitcoin/bitcoin/pull/32363#discussion_r2064585207)
What is the purpose of this change? It seems entirely unnecessary and unrelated to the purpose of this PR.

💬 1ma commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2836538928)
> Concept ACK.
>
> It's time to come to terms with the fact that Bitcoin is desirable for some people to use as a data anchor and they will find a way to use it as such, thus we should be asking what the preferred means of data anchoring is and how to incentivize it over less preferable options.

@jlopp I think it would be appropriate for you to disclose that you're one of the VCs backing Citrea, a company that is currently abusing unspendable Taproot outputs to embed arbitrary data onchain
...

💬 BullishNode commented on issue "Allow sending untrusted utxos in the sendtoaddress api":
(https://github.com/bitcoin/bitcoin/issues/32034#issuecomment-2836545266)
@achow101 I know you once nacked this conecpt so tagging you here for discussion maybe I can change your mind.

💬 achow101 commented on pull request "ci: Use previous releases in tests on Windows":
(https://github.com/bitcoin/bitcoin/pull/32363#discussion_r2064603713)
Why does `wallet_migration.py` need to be run separately, and none of the other tests that require previous releases? These can all be run at the same time from the `test_runner.py` invocation if the `PREVIOUS_RELEASES_DIR` environment variable is set.