Merge #17681: wallet: Keep inactive seeds after sethdseed and derive keys from them as needed

1ed52fbb4d Remove IBD check in sethdseed (Andrew Chow)
b1810a145a Test that keys from inactive seeds are generated (Andrew Chow)
c93082ece4 Generate new keys for inactive seeds after marking used (Andrew Chow)
45f2f6a0e8 Determine inactive HD seeds from key metadata and track them in LegacyScriptPubKeyMan (Andrew Chow)
b59b4504ab have GenerateNewKey and DeriveNewChildKey take a CHDChain as an argument (Andrew Chow)

Pull request description:

  Largely implements the suggestion from https://github.com/bitcoin/bitcoin/pull/17484#issuecomment-560845316.

  After `sethdseed` is called, the CHDChain for the old seed is kept in the wallet. It is kept on the file as a new `inactivehdseed` record and in memory in a map `m_inactive_hd_seeds`. In `LegacyScriptPubKeyMan::MarkUnusedAddresses` we check each used key's metadata for whether it was derived from an inactive seed. If it is, we then check to see how many keys after that key were derived from the inactive seed. If that number does not match the keypool parameter, we derive more keys from the inactive seed until it does match. This way we won't miss transactions belonging to keys outside of the range of the keypool initially.

  The indexes and internal-ness of a key is gotten by checking it's key origin data.

  Because of this change, we no longer need to wait for IBD to finish before `sethdseed` can work so that check is also removed.

  A test case for this is added as well which fails on master.

ACKs for top commit:
  ryanofsky:
    Code review ACK 1ed52fbb4d. Changes since last review: various commit message, code comment, log message, error checking improvements, and fix for topping up inactive seeds if wallet isn't reloaded after calling sethdseed and test for this
  ariard:
    Code Review ACK 1ed52fb
  jonatack:
    ACK 1ed52fbb4d thanks for addressing the previous review feedback; would be happy to see the new review questions answered and feedback addressed and re-ack.

Tree-SHA512: e658ae0e1dab94be55d2b62cdda506c94815e73a6881533fd30d41cc77477f82fee2095144957a3a1df0c129e256bdd7b7abe3737d515f393610446cae4edf1c
This commit is contained in:
Samuel Dobson
2020-05-22 13:11:26 +12:00
6 changed files with 301 additions and 32 deletions

View File

@@ -10,6 +10,7 @@
#include <protocol.h>
#include <serialize.h>
#include <sync.h>
#include <util/bip32.h>
#include <util/system.h>
#include <util/time.h>
#include <wallet/wallet.h>
@@ -256,6 +257,7 @@ public:
std::map<uint256, DescriptorCache> m_descriptor_caches;
std::map<std::pair<uint256, CKeyID>, CKey> m_descriptor_keys;
std::map<std::pair<uint256, CKeyID>, std::pair<CPubKey, std::vector<unsigned char>>> m_descriptor_crypt_keys;
std::map<uint160, CHDChain> m_hd_chains;
CWalletScanState() {
}
@@ -435,6 +437,65 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
ssValue >> keyMeta;
wss.nKeyMeta++;
pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadKeyMetadata(vchPubKey.GetID(), keyMeta);
// Extract some CHDChain info from this metadata if it has any
if (keyMeta.nVersion >= CKeyMetadata::VERSION_WITH_HDDATA && !keyMeta.hd_seed_id.IsNull() && keyMeta.hdKeypath.size() > 0) {
// Get the path from the key origin or from the path string
// Not applicable when path is "s" as that indicates a seed
bool internal = false;
uint32_t index = 0;
if (keyMeta.hdKeypath != "s") {
std::vector<uint32_t> path;
if (keyMeta.has_key_origin) {
// We have a key origin, so pull it from its path vector
path = keyMeta.key_origin.path;
} else {
// No key origin, have to parse the string
if (!ParseHDKeypath(keyMeta.hdKeypath, path)) {
strErr = "Error reading wallet database: keymeta with invalid HD keypath";
return false;
}
}
// Extract the index and internal from the path
// Path string is m/0'/k'/i'
// Path vector is [0', k', i'] (but as ints OR'd with the hardened bit
// k == 0 for external, 1 for internal. i is the index
if (path.size() != 3) {
strErr = "Error reading wallet database: keymeta found with unexpected path";
return false;
}
if (path[0] != 0x80000000) {
strErr = strprintf("Unexpected path index of 0x%08x (expected 0x80000000) for the element at index 0", path[0]);
return false;
}
if (path[1] != 0x80000000 && path[1] != (1 | 0x80000000)) {
strErr = strprintf("Unexpected path index of 0x%08x (expected 0x80000000 or 0x80000001) for the element at index 1", path[1]);
return false;
}
if ((path[2] & 0x80000000) == 0) {
strErr = strprintf("Unexpected path index of 0x%08x (expected to be greater than or equal to 0x80000000)", path[2]);
return false;
}
internal = path[1] == (1 | 0x80000000);
index = path[2] & ~0x80000000;
}
// Insert a new CHDChain, or get the one that already exists
auto ins = wss.m_hd_chains.emplace(keyMeta.hd_seed_id, CHDChain());
CHDChain& chain = ins.first->second;
if (ins.second) {
// For new chains, we want to default to VERSION_HD_BASE until we see an internal
chain.nVersion = CHDChain::VERSION_HD_BASE;
chain.seed_id = keyMeta.hd_seed_id;
}
if (internal) {
chain.nVersion = CHDChain::VERSION_HD_CHAIN_SPLIT;
chain.nInternalChainCounter = std::max(chain.nInternalChainCounter, index);
} else {
chain.nExternalChainCounter = std::max(chain.nExternalChainCounter, index);
}
}
} else if (strType == DBKeys::WATCHMETA) {
CScript script;
ssKey >> script;
@@ -758,6 +819,20 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
result = DBErrors::CORRUPT;
}
// Set the inactive chain
if (wss.m_hd_chains.size() > 0) {
LegacyScriptPubKeyMan* legacy_spkm = pwallet->GetLegacyScriptPubKeyMan();
if (!legacy_spkm) {
pwallet->WalletLogPrintf("Inactive HD Chains found but no Legacy ScriptPubKeyMan\n");
return DBErrors::CORRUPT;
}
for (const auto& chain_pair : wss.m_hd_chains) {
if (chain_pair.first != pwallet->GetLegacyScriptPubKeyMan()->GetHDChain().seed_id) {
pwallet->GetLegacyScriptPubKeyMan()->AddInactiveHDChain(chain_pair.second);
}
}
}
return result;
}