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

💬 furszy commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1385495818)
> I don't think this is a substantially useful optimization, and it still results in mixing upgrading code with the loading which I would rather not do.

As far as I remember, the last patch I shared doesn't mixes the upgrading code with the loading code.
It loads only the descriptor root key alone at the end of the spkm loading process so we can avoid loading all descriptors keys into a vector, just to obtain the private keys related to each descriptor xpub later in the upgrade process.

I
...

💬 ajtowns commented on pull request "rpc: Add script verification flags to getdeploymentinfo":
(https://github.com/bitcoin/bitcoin/pull/28806#discussion_r1385534115)
> Is this ever hit? If not, maybe it can be removed, as the remaining code should reach the same conclusion anyway in the rare case that this is hit?

It's hit for block 170060. I think it's worth having it be explicit, as it would also be reasonable to return "NONE" (as in `SCRIPT_VERIFY_NONE`) for that case.

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1385542152)
Ah, good catch. Fixed

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1385543718)
I don't think it particularly matters since a wallet without private keys cannot have a global hd key. However I have changed it so that it is set only if private keys are enabled.

💬 achow101 commented on pull request "shutdown: Destroy kernel last, make test shutdown order consistent":
(https://github.com/bitcoin/bitcoin/pull/28224#issuecomment-1800121448)
ACK c1144f0076339c775f41d4b5fcfdc72191440d96

🚀 achow101 merged a pull request: "shutdown: Destroy kernel last, make test shutdown order consistent"
(https://github.com/bitcoin/bitcoin/pull/28224)

💬 achow101 commented on pull request "multiprocess compatibility updates":
(https://github.com/bitcoin/bitcoin/pull/28721#issuecomment-1800179388)
ACK 3b70f7b6156cb110c47a6e482791cf337bb6ad6d

💬 achow101 commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#issuecomment-1800200116)
ACK 1147e00e59e47f27024ec96629993c66a3ce4ef0

💬 achow101 commented on pull request "test: python cryptography required for BIP 324 functional tests":
(https://github.com/bitcoin/bitcoin/pull/28374#issuecomment-1800222458)
ACK c534c0871038ded72dc9078cc91e030ceb746196