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

💬 furszy commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1385459943)
In 3e250b10:

There is a discrepancy here. The upgrade mechanism is skipped for private key disabled wallets without set the `WALLET_FLAG_GLOBAL_HD_KEY`. This differs from what is being done in the wallet creation and the migration processes where we set this flag for all types (only requirement is `WALLET_FLAG_DESCRIPTORS`).

Is this intended?

💬 furszy commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1385428552)
In 4ad97b519e7:
As the default value for "internal" is false, I infer you wanted `internal=true`?.

Same for the other one that is below.

💬 brunoerg commented on pull request "fuzz: Avoid timeout and bloat in fuzz targets":
(https://github.com/bitcoin/bitcoin/pull/28815#issuecomment-1799788517)
Concept ACK

Question: How is the difference compared to #27552 ("partial" fuzzers)?

💬 achow101 commented on pull request "Add support for RNDR/RNDRRS for AArch64 on Linux":
(https://github.com/bitcoin/bitcoin/pull/26839#issuecomment-1799804463)
ACK aee5404e02e203a256c1a97b629b9b107cc8bb07

The code looks correct to me, although I do not have hardware to test it.

✅ achow101 closed an issue: "Request support for HRNG on ARM using their new RNDR / RNDRRS instructions."
(https://github.com/bitcoin/bitcoin/issues/26796)

🚀 achow101 merged a pull request: "Add support for RNDR/RNDRRS for AArch64 on Linux"
(https://github.com/bitcoin/bitcoin/pull/26839)