wallet: Add deriveHDKey interface #36070

This case passes even when the derivation comes from a different key: origin.path just echoes the argument, and IsValid() holds for any derived key. I made SelectHDKey return a CExtKey the wallet doesn't hold and all six cases still passed, while wallet_derivehdkey.py failed on the same mutation.

Comparing against the wallet's own active HD key would catch this, the way bip32_derive_ext_key does for DeriveExtKey:

    LOCK(wallet->cs_wallet);
    const auto active{wallet->GetHDPubKeys(CWallet::HDKeyFilter::Active)};
    BOOST_REQUIRE_EQUAL(active.size(), 1U);
    const auto xprv{wallet->GetExtKey(active.begin()->first)};
    BOOST_REQUIRE(xprv);
    const auto expected{DeriveExtKey(*xprv, path)};
    BOOST_REQUIRE(expected);
    BOOST_CHECK(result->first == expected->first.Neuter());
    BOOST_CHECK(result->second == expected->second);

Is this branch reachable? EnsureWalletIsUnlocked runs a few lines above with cs_wallet already held, and vMasterKey is GUARDED_BY(cs_wallet), so nothing can lock the wallet in between. Asking because the commit message says the RPC maps it.

Right, removed this branch

in 5de0c29e06f41f6901fd0d0ec69c9359612371e5 wallet: Return WalletError from SelectHDKey

This is dead code, it can never be executed because if the wallet is locked, EnsureWalletIsUnlocked(*wallet); (L1006) will throw before.

in 11953500e91920bc6357190bcd8956212913743d wallet: Move derivehdkey HD key selection into CWallet

I would make this return Expected since the beginning. Probably commit 1 and 2 should be squashed, you are already changing the return type here, so why not go directly to the "final" version?

Squashed, SelectHDKey returns util::Expected from the start now.

in a1250d2f413016c4b4524d50823ef816af9aff40 wallet: Add deriveHDKey interface

All this is duplicated from RPC code. Why not just make both the interface and the RPC rely on the same code? Because of both using different code functions, they both return different error codes for same errors.

Also probably this commit could be split in different commits. First a preparatory commit that creates the DeriveHDKey function + testing, then a commit rebasing the RPC to use it, then a commit creating the interface. You can check #34861 for a commit structure idea :)

Done. Split into "Add CWallet::DeriveHDKey" and "Add deriveHDKey interface".

On sharing code with the RPC: it already calls SelectHDKey(). What's left is the watch-only, hardened-path and unlock guards, which pin the error codes wallet_derivehdkey.py asserts (-4, -8, -13).

DeriveHDKey() returns GenericError for every failure, so routing the rest through it would turn "Unable to derive HD key at the requested path" from -8 into -5. Keeping -8 would need a new WalletErrorCode just so one caller can pick a different number, which src/wallet/types.h:48-50 warns against.

Same problem the interface test had: origin.path just echoes the argument, and IsValid() holds for any key. With SelectHDKey returning a key the wallet doesn't hold, this one passes while the interface test fails; same for blanking the origin fingerprint or returning the key before derivation.

    const auto active{wallet->GetHDPubKeys(CWallet::HDKeyFilter::Active)};
    BOOST_REQUIRE_EQUAL(active.size(), 1U);
    const auto xprv{wallet->GetExtKey(active.begin()->first)};
    BOOST_REQUIRE(xprv);
    const auto expected{DeriveExtKey(*xprv, path)};
    BOOST_REQUIRE(expected);
    BOOST_CHECK(result->first == expected->first);
    BOOST_CHECK(result->second == expected->second);