💬 naumenkogs commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1853485863)
@theStack I think the philosophy behind v2 was always "prevent the firewall from discriminating authenticated connections by making them look exactly like normal connections". From [BIP](https://github.com/bitcoin/bips/blob/master/bip-0324.mediawiki):

>Encryption, even when it is unauthenticated and only used when both endpoints support v2, impedes eavesdropping by forcing the attacker to become active: either by performing a persistent man-in-the-middle (MitM) attack, by downgrading connect
...

💬 naumenkogs commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#discussion_r1425044177)
c5d41370287f48c638ab87d3e02a117b1a17694a

What if the peer is `v2`, but disconnected us due to other reason? (say a limit of inbounds, or us coming from a blocked network)? Here and in the logic below. In that case we should not attempt reconnection with `v1`, it won't help.

This is probably about `ShouldReconnectV1` behavior. I can't find where this case is handled.

💬 dergoegge 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-1853532671)
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_params.m_cost_of_ch
...

⚠️ maflcko reopened an issue: "fuzz, coinselection: Assertion 'result_bnb->GetChange(coin_params.m_cost_of_change, CAmount{0}) == 0' failed"
(https://github.com/bitcoin/bitcoin/issues/28918)
```
$ echo "PACVlVuVlZWVlZUE3pUAANNRAFEA09NRUb9RUVFR/wD/AP//AP8AWwD//wcErq6urv///////wD/4wAAAAD4a9cA////ra2tra2tra2tra2VlZWVMGA5OTk5OZWVlZWVlZWVlZWVlZWVlYUVJwyq6wEAlZWblZWVlZWVlZWVlZWVlZWblZWVlZWVlZWVlZWV//+V/5WV/////z4AAAAALAtfAgAAX/8=" | base64 --decode > coinselection-d97eed2ff63da56af72c8c858c560a7c6f2aef45.crash
$ FUZZ=coinselection ./src/test/fuzz/fuzz coinselection-d97eed2ff63da56af72c8c858c560a7c6f2aef45.crash
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 189972
...

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 2)":
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1853542223)
It's not easy being green, but including `compat.h` should fix the issue:

```cpp
// ssize_t is POSIX, and not present when using MSVC.
```

The CI fuzzer caught an overflow, so I added a guard against absurd `coinbase_output_max_additional_size` values. I have not tried the fuzzer myself yet, nor does it have complete coverage, so there may still be other issues.

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

💬 paulocoutinhox commented on issue "CMake-based build system tracking issue":
(https://github.com/bitcoin/bitcoin/issues/28607#issuecomment-1853551871)
+1

💬 stickies-v commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1425077125)
Ah, you're right, that'd be bad.

💬 fanquake commented on issue "[bench] Assertion error on wallet_create_tx.cpp, line 133.":
(https://github.com/bitcoin/bitcoin/issues/29069#issuecomment-1853562050)
Duplicate of #29061.

✅ fanquake closed an issue: "[bench] Assertion error on wallet_create_tx.cpp, line 133."
(https://github.com/bitcoin/bitcoin/issues/29069)

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1425084623)
Ok the lazy way clearly doesn't work; I think what we have to do is calculate in-package ancestors and feed that to `ApplyV3Rules` when we call it. This will also have the effect of consolidating our v3 checks so that it's cleaner. Pushing soon...

💬 fanquake commented on pull request "ci: Use Ubuntu 24.04 Noble for asan,tsan,tidy,fuzz":
(https://github.com/bitcoin/bitcoin/pull/28992#issuecomment-1853568692)
> Will be fixed in 17.0.6-2 once noble-proposed is released,

This has shipped, and `17.0.6-2` is available.

🚀 fanquake merged a pull request: "util: Remove DirIsWritable, GetUniquePath"
(https://github.com/bitcoin/bitcoin/pull/28075)

💬 1ma commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1853641110)
@conduition thanks for writing that article, it's a very interesting read but I still feel we are not on the same page yet.

The point of "fighting the spam TXs" is not to prevent all forms of data inscription at all costs, it's to fix Bitcoin's fee market. The crux of the problem as described [here](https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1852569550) and [here](https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1852673575) is that inscriptions dramatically overpay f
...

📝 nikodemas opened a pull request: "test: add TestNode wait_until helper"
(https://github.com/bitcoin/bitcoin/pull/29070)
Add `wait_until` method that wraps the `wait_until_helper_internal` call.

Closes https://github.com/bitcoin/bitcoin/issues/29029.

🤔 BrandonOdiwuor reviewed a pull request: "bench: wallet, fix change position out of range error"
(https://github.com/bitcoin/bitcoin/pull/29065#pullrequestreview-1779322117)
Concept ACK

💬 maflcko commented on pull request "test: add TestNode wait_until helper":
(https://github.com/bitcoin/bitcoin/pull/29070#issuecomment-1853665006)
lgtm ACK bf0f7dbec6590a54ec890e7a2ca5d85427995334

🚀 fanquake merged a pull request: "depends: fix libmultiprocess build on aarch64"
(https://github.com/bitcoin/bitcoin/pull/28846)

💬 fanquake commented on pull request "build: disable external-signer for Windows":
(https://github.com/bitcoin/bitcoin/pull/28967#issuecomment-1853689940)
> Let's at least let builders explicitly enable it?

That would mostly defeat the point of this change, and leave us supporting code that we've deemed unsafe and unreviewed. If someone wants to do that, they are free to understand the risks, and patch their own code.

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