💬 maflcko commented on pull request "fuzz: Avoid timeout and bloat in bloom_filter target":
(https://github.com/bitcoin/bitcoin/pull/28815#issuecomment-1799139133)
Can be tested by running https://github.com/bitcoin/bitcoin/files/13280645/clusterfuzz-testcase-bloom_filter-5968259782148096.dmp.not.txt and looking at the runtime

🤔 glozow reviewed a pull request: "Fee Estimator updates from Validation Interface/CScheduler thread"
(https://github.com/bitcoin/bitcoin/pull/28368#pullrequestreview-1718119056)
Maybe add a `SyncWithValidationInterfaceQueue` to the start of the fee estimation RPCs? To avoid spurious failures in `feature_fee_estimation.py`. Also, I can imagine scripts that grab fee estimates once per block as soon as the block arrives - should probably make sure they're fresh.

💬 glozow commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1385183779)
09436b21e84cc6cd979fe4942ba1c415c4bc91be can be squashed

💬 glozow commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1385215139)
ac82f58e45421aa5aec1d3e6e992d1c369472a10

```suggestion
const auto& hash = tx_info.m_tx->GetHash();
```

💬 glozow commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1385207240)
6f40c32fa8dfc742cf5ca7daa28615e066872aec

maybe stop making a copy, and let it be a `Txid`

```suggestion
const auto& hash = it->GetTx().GetHash();
```

💬 glozow commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1385219874)
you're changing how the fuzz inputs are processed here - maybe have `m_has_no_mempool_parents = ConsumeBool()` and make the others = false/false/true?

💬 glozow commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1385224078)
ac82f58e45421aa5aec1d3e6e992d1c369472a10
a bit less verbose
```suggestion
const NewMempoolTransactionInfo tx_info{MakeTransactionRef(tx),
```

💬 fanquake commented on pull request "sanitizer: symbolizer improvements":
(https://github.com/bitcoin/bitcoin/pull/28814#issuecomment-1799198972)
Changed the approach slightly, which should fix the failing job. This should continue to work everywhere. Might also send a change to LLVM, to try and consolidate the `LLVM_SYMBOLIZER_PATH` usage.

💬 maflcko commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1799301221)
Ok, feel free to add it to any task, for example `ci/test/00_setup_env_native_qt5.sh`.

💬 maflcko commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1799311451)
Flame graph for mocked_descriptor_parse:

(Most of the time is spent in pubkey derive)


💬 mzumsande commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1799340396)
> Ok, feel free to add it to any task, for example `ci/test/00_setup_env_native_qt5.sh`.

Thanks, though I won't add it to this PR.
I'm open to suggestions, but in my opinion the best way to proceed might be to first get #24748 in to upgrade the python p2p framework. I expect we will then need a similar PR to this one to fix some minor issues with the existing tests before turning it on for all tests (something I started working on), and maybe only after we should make a `-v2transport=1` task
...

💬 maflcko commented on pull request "sanitizer: symbolizer improvements":
(https://github.com/bitcoin/bitcoin/pull/28814#issuecomment-1799415022)
lgtm ACK 49d953281df5618430728c0a88471695207f086b

✅ maflcko closed a pull request: "test: Revert "suppressions: note that 'type:ClassName::MethodName' should be used""
(https://github.com/bitcoin/bitcoin/pull/28804)

💬 achow101 commented on pull request "Do the SOCKS5 handshake reliably":
(https://github.com/bitcoin/bitcoin/pull/28649#issuecomment-1799645189)
ACK af0fca530e4d8311bcb24a14c416e5ad7c30ff78

🚀 achow101 merged a pull request: "Do the SOCKS5 handshake reliably"
(https://github.com/bitcoin/bitcoin/pull/28649)

💬 achow101 commented on pull request "refactor: move GetServicesNames from rpc/util.{h,cpp} to rpc/net.cpp":
(https://github.com/bitcoin/bitcoin/pull/28136#issuecomment-1799651071)
ACK bbb68ffdbdafb6717dcadac074f6098750b8aa77

🚀 achow101 merged a pull request: "refactor: move GetServicesNames from rpc/util.{h,cpp} to rpc/net.cpp"
(https://github.com/bitcoin/bitcoin/pull/28136)

🤔 mzumsande reviewed a pull request: "test: Add missing sync on send_version in peer_connect"
(https://github.com/bitcoin/bitcoin/pull/28782#pullrequestreview-1718560252)
ACK fa025984698270d8c9a621aef80f1d788ea2ab2e

I tested this by adding a sleep to `connection_made` before sending the message.
However `p2p_sendtxrcncl.py` will then fail later in the `SENDTXRCNCL if block-relay-only triggers a disconnect` part because `add_outbound_p2p_connection` has the same problem. Do you want to fix that too here, or in another PR?

👍 BrandonOdiwuor approved a pull request: "init: completely remove `-zapwallettxes` (remaining hidden option)"
(https://github.com/bitcoin/bitcoin/pull/28787#pullrequestreview-1718560371)
ACK 5039c346ca87d6112ea1eb124bdc622ba9e9a513

It’s been >3 years since the removal of -zapwallettxes and replaced by InitError to use abandontransaction

🤔 furszy reviewed a pull request: "wallet: Have the wallet store the key for automatically generated descriptors"
(https://github.com/bitcoin/bitcoin/pull/26728#pullrequestreview-1718519262)
Reviewed, left two comments.

-------

> > it would also be nice to test the scenario where the process does not find the expected number of active descriptors. This should result in the wallet having no active xpub key.
>
> Added to a test that imports a few descriptors whose xpub won't be chosen for active xpub.

Cool.
Future thing; it would be nice to improve the backwards compatibility test code organization. The more we add there, the harder is to follow.
Other test cases coming
...