📝 maflcko opened a pull request: "refactor: Remove Span operator==, Use std::ranges::equal"
(https://github.com/bitcoin/bitcoin/pull/29071)
`std::span` removed the comparison operators, so it makes sense to remove them for the `Span` "backport" as well. Using `std::ranges::equal` also has the benefit that some `Span` temporary constructions can now be dropped.

👋 BrandonOdiwuor's pull request is ready for review: "Wallet: Calculate used balance from GetBalance(...)"
(https://github.com/bitcoin/bitcoin/pull/29062)

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1425181714)
deleted

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1425182035)
added error string checks everywhere

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1425182152)
done

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1425181660)
deleted

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1425181145)
Ok there are now only 2 places where v3 is checked: package sanity checks, and then `ApplyV3Rules` for each tx. I think the sanity check is very useful, as there is some heavy-ish computation done to calculate each transaction's ancestor set (including in-package and their mempool ancestors) to pass on to `ApplyV3Rules`. I've moved inheritance checks into `ApplyV3Rules` and made all errors "v3-rule-violation" with more details in the debug string.

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1425180669)
yikes, fixed

💬 fanquake commented on pull request "test: Actually fail when a python unit test fails":
(https://github.com/bitcoin/bitcoin/pull/29068#issuecomment-1853733189)
> Fun fact: This problem would not exist in a type-safe language.

It's time to start rewriting code in such a language.

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1853734787)
https://github.com/bitcoin/bitcoin/pull/28948#pullrequestreview-1777711045

Added this test but changed the `tx_v3_child_heavy` test assuming that it wasn't supposed to be submitted with `tx_v3_parent_2_normal` (it fails package topo checks then), lmk if you had something else in mind?

🚀 fanquake merged a pull request: "test: Actually fail when a python unit test fails"
(https://github.com/bitcoin/bitcoin/pull/29068)

🚀 fanquake merged a pull request: "build: disable external-signer for Windows"
(https://github.com/bitcoin/bitcoin/pull/28967)

📝 dergoegge converted_to_draft a pull request: "fuzz: Improve fuzzing stability for minisketch harness"
(https://github.com/bitcoin/bitcoin/pull/29064)
The `minisketch` harness has low stability due to:
* Rng internal to minisketch
* Benchmarkning for the best minisketch impl

Fix this by seeding the rng and fixing the impl to 0.

Also see #29018.

📝 fanquake opened a pull request: "build: use `-no_exported_symbols` on macOS"
(https://github.com/bitcoin/bitcoin/pull/29072)
This reduces the size of the binary by 2-3% when building with `--enable-reduce-exports`.

We wont get this in release builds (yet), as that would require moving to a newer cctools/ld64 (at least ld 907). However, this flag is also supported in lld, so we would end up getting it once that migration happens.

> -no_exported_symbols
> Useful for main executable that don't have plugins and thus need no symbol exports.

Can be tested with `dyld_info -exports src/bitcoind`. The only exported s
...

💬 furszy commented on issue "fuzz, coinselection: Assertion 'result_bnb->GetChange(coin_params.m_cost_of_change, CAmount{0}) == 0' failed":
(https://github.com/bitcoin/bitcoin/issues/28918#issuecomment-1853803534)
> This is still broken, please reopen:
>
> ```
> $ echo "AQ0N0NfM0B7QEK1AL3m3AJWVlQAAgQAAAAAAlZWVAACBAAAAAACVlZUAAAAAAAAAAAAAAAAAXQAAgQAAAAAAAAAAAAAA" | base64 --decode > coinselection-90005214eeb91a419ca37def8d958bf6bb670edf.crash
> $ FUZZ=coinselection ./src/test/fuzz/fuzz coinselection-90005214eeb91a419ca37def8d958bf6bb670edf.crash
> fuzz_libfuzzer: wallet/test/fuzz/coinselection.cpp:122: void wallet::coinselection_fuzz_target(FuzzBufferType): Assertion `result_bnb->GetChange(coin_param
...

💬 furszy commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#issuecomment-1853803752)
> See [#28918 (comment)](https://github.com/bitcoin/bitcoin/issues/28918#issuecomment-1853532671). This doesn't seem to have fixed the fuzzer.

It fixed the initially reported one (https://github.com/bitcoin/bitcoin/issues/28918#issue-2002147628). The latest one is different.

📝 maflcko converted_to_draft a pull request: "refactor: Remove Span operator==, Use std::ranges::equal"
(https://github.com/bitcoin/bitcoin/pull/29071)
`std::span` removed the comparison operators, so it makes sense to remove them for the `Span` "backport" as well. Using `std::ranges::equal` also has the benefit that some `Span` temporary constructions can now be dropped.

💬 maflcko commented on pull request "refactor: Remove Span operator==, Use std::ranges::equal":
(https://github.com/bitcoin/bitcoin/pull/29071#discussion_r1425233553)
Review note: `Span{privkey}` can not be removed, for now. This is because the type of `Span{privkey}` and `Span{privkey.begin(), privkey.end()}` are different.

💬 stevenroose commented on pull request "Add OP_TXHASH and OP_CHECKTXHASHVERIFY opcodes":
(https://github.com/bitcoin/bitcoin/pull/29050#issuecomment-1853850878)
I added a commit that has a few question marks about which contexts should have their own TxHashCache. I'm not entirely understanding which contexts have the `PrecomputedTxData` either. The `MutableTransactionSignatureCreator` is the most troubling, because since the `TransactionSignatureChecker` takes a ptr to `TxHashCache`, some cache needs to live outside of the `MutableTransactionSignatureCreator` while actually that cache could be empty on every call. So it seems like we actually might have
...

💬 furszy commented on issue "fuzz, coinselection: Assertion 'result_bnb->GetChange(coin_params.m_cost_of_change, CAmount{0}) == 0' failed":
(https://github.com/bitcoin/bitcoin/issues/28918#issuecomment-1853853445)
First glance, the solution is to calculate the minimum viable change variable in the same way we do inside the wallet (not setting it randomly as we currently do) and provide it to `GetChange()` instead of providing `cost_of_change`.
There is more information about this inside the long convo Murch and I had on https://github.com/bitcoin/bitcoin/pull/28395#pullrequestreview-1620951490.

The issue arises when `cost_of_change < selected_value - target`. Because we provide the cost of creating ch
...

⚠️ fanquake opened an issue: "ci: failure in `wallet_multiwallet.py --legacy-wallet`"
(https://github.com/bitcoin/bitcoin/issues/29073)
https://cirrus-ci.com/task/5692425725804544?logs=ci#L3370:
```bash
node0 2023-12-13T12:44:27.733356Z [httpworker.1] [rpc/request.cpp:187] [parse] [rpc] ThreadRPCServer method=unloadwallet user=__cookie__
test 2023-12-13T12:44:27.782000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_fr
...