Page MenuHomePhabricator

[backport#17373] wallet: Various fixes and cleanup to keypool handling in LegacyScriptPubKeyMan and CWallet
ClosedPublic

Authored by majcosta on Oct 1 2020, 01:58.

Details

Summary

886f1731bec4393dd342403ac34069a3a4f95eea Key pool: Fix omitted pre-split count in GetKeyPoolSize (Andrew Chow)
386a994b853bc5b3a2ed0d812673465b8ffa4849 Key pool: Change ReturnDestination interface to take address instead of key (Andrew Chow)
ba41aa4969169cd73d6b4f57444ed7d8d875de10 Key pool: Move LearnRelated and GetDestination calls (Andrew Chow)
65833a74076cddf986037c6eb3b29a9b9dbe31c5 Add OutputType and CPubKey parameters to KeepDestination (Andrew Chow)
9fcf8ce7ae02bf170b9bf0c2887fd709d752cbf7 Rename Keep/ReturnKey to Keep/ReturnDestination and remove the wrapper (Andrew Chow)
596f6460f9fd8273665c8754ccd673d93a4f25f0 Key pool: Move CanGetAddresses call (Andrew Chow)

Pull request description:

  • The pwallet->CanGetAddresses() call in ReserveDestination::GetReservedDestination to LegacyScriptPubKeyMan::GetReservedDestination so that the sanity check results in a failure when a ScriptPubKeyMan individually cannot get a destination, not when any of the ScriptPubKeyMans can't.
  • ScriptPubKeyMan::GetReservedDestination is changed to return the destination so that future ScriptPubKeyMans can return destinations constructed in other ways. This is implemented for LegacyScriptPubKeyMan by moving key-to-destination code from CWallet to LegacyScriptPubKeyMan
  • In order for ScriptPubKeyMan to be generic and work with future ScriptPubKeyMans, ScriptPubKeyMan::ReturnDestination is changed to take a CTxDestination instead of a CPubKey. Since LegacyScriptPubKeyMan still deals with keys internally, a new map m_reserved_key_to_index is added in order to track the keypool indexes that have been reserved.
  • A bug is fixed in how the total keypool size is calculated as it was omitting set_pre_split_keypool which is a bug.

    Split from #17261

Depends on D7690

Backport of Core PR17373

Test Plan
ninja check check-functional

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

majcosta requested review of this revision.Oct 1 2020, 01:58

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

Snippet of first build failure:

[350/409] Running utility command for check-bitcoin-settings_tests
[351/409] bitcoin: testing timedata_tests
[352/409] Running utility command for check-bitcoin-streams_tests
[353/409] Running utility command for check-bitcoin-timedata_tests
[354/409] bitcoin: testing uint256_tests
[355/409] bitcoin: testing undo_tests
[356/409] Running utility command for check-bitcoin-uint256_tests
[357/409] Running utility command for check-bitcoin-undo_tests
[358/409] bitcoin: testing walletdb_tests
[359/409] Running utility command for check-bitcoin-walletdb_tests
[360/409] bitcoin: testing util_threadnames_tests
[361/409] Running utility command for check-bitcoin-util_threadnames_tests
[362/409] bitcoin: testing script_standard_tests
[363/409] Running utility command for check-bitcoin-script_standard_tests
[364/409] bitcoin: testing serialize_tests
[365/409] Running utility command for check-bitcoin-serialize_tests
[366/409] bitcoin: testing radix_tests
[367/409] bitcoin: testing crypto_tests
[368/409] bitcoin: testing blockcheck_tests
[369/409] Running utility command for check-bitcoin-radix_tests
[370/409] Running utility command for check-bitcoin-blockcheck_tests
[371/409] Running utility command for check-bitcoin-crypto_tests
[372/409] bitcoin: testing blockstatus_tests
[373/409] Running utility command for check-bitcoin-blockstatus_tests
[374/409] bitcoin: testing ismine_tests
[375/409] bitcoin: testing cashaddr_tests
[376/409] Running utility command for check-bitcoin-ismine_tests
[377/409] bitcoin: testing validation_block_tests
[378/409] bitcoin: testing versionbits_tests
[379/409] Running utility command for check-bitcoin-cashaddr_tests
[380/409] Running utility command for check-bitcoin-validation_block_tests
[381/409] Running utility command for check-bitcoin-versionbits_tests
[382/409] bitcoin: testing getarg_tests
[383/409] Running utility command for check-bitcoin-getarg_tests
[384/409] bitcoin: testing validation_tests
[385/409] Running utility command for check-bitcoin-validation_tests
[386/409] bitcoin: testing bswap_tests
[387/409] Running utility command for check-bitcoin-bswap_tests
[388/409] bitcoin: testing script_tests
[389/409] Running utility command for check-bitcoin-script_tests
[390/409] bitcoin: testing skiplist_tests
[391/409] Running utility command for check-bitcoin-skiplist_tests
[392/409] bitcoin: testing util_tests
[393/409] Running utility command for check-bitcoin-util_tests
[394/409] Linking CXX executable src/qt/test/test_bitcoin-qt
[395/409] bitcoin: testing cuckoocache_tests
[396/409] Running utility command for check-bitcoin-cuckoocache_tests
[397/409] bitcoin: testing op_reversebytes_tests
[398/409] Running utility command for check-bitcoin-op_reversebytes_tests
[399/409] bitcoin: testing coins_tests
[400/409] Running utility command for check-bitcoin-coins_tests
[401/409] bitcoin-qt: testing test_bitcoin-qt
[402/409] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[403/409] bitcoin: testing transaction_tests
[404/409] Running utility command for check-bitcoin-transaction_tests
[405/409] secp256k1: testing secp256k1-tests
[406/409] Running secp256k1 test suite
PASSED: secp256k1 test suite
Build build-clang timed out after 1200.0s

Snippet of first build failure:

[342/402] bitcoin: testing timedata_tests
[343/402] bitcoin: testing streams_tests
[344/402] Running utility command for check-bitcoin-timedata_tests
[345/402] Running utility command for check-bitcoin-streams_tests
[346/402] bitcoin: testing uint256_tests
[347/402] Running utility command for check-bitcoin-uint256_tests
[348/402] bitcoin: testing undo_tests
[349/402] Running utility command for check-bitcoin-undo_tests
[350/402] bitcoin: testing walletdb_tests
[351/402] Running utility command for check-bitcoin-walletdb_tests
[352/402] bitcoin: testing util_threadnames_tests
[353/402] Running utility command for check-bitcoin-util_threadnames_tests
[354/402] bitcoin: testing serialize_tests
[355/402] Running utility command for check-bitcoin-serialize_tests
[356/402] bitcoin: testing validationinterface_tests
[357/402] Running utility command for check-bitcoin-validationinterface_tests
[358/402] bitcoin: testing script_standard_tests
[359/402] bitcoin: testing radix_tests
[360/402] bitcoin: testing blockcheck_tests
[361/402] Running utility command for check-bitcoin-radix_tests
[362/402] Running utility command for check-bitcoin-script_standard_tests
[363/402] Running utility command for check-bitcoin-blockcheck_tests
[364/402] bitcoin: testing crypto_tests
[365/402] Running utility command for check-bitcoin-crypto_tests
[366/402] bitcoin: testing ismine_tests
[367/402] bitcoin: testing blockstatus_tests
[368/402] Running utility command for check-bitcoin-ismine_tests
[369/402] Running utility command for check-bitcoin-blockstatus_tests
[370/402] bitcoin: testing validation_tests
[371/402] bitcoin: testing cashaddr_tests
[372/402] Running utility command for check-bitcoin-validation_tests
[373/402] Running utility command for check-bitcoin-cashaddr_tests
[374/402] bitcoin: testing script_tests
[375/402] bitcoin: testing getarg_tests
[376/402] Running utility command for check-bitcoin-script_tests
[377/402] bitcoin: testing versionbits_tests
[378/402] Running utility command for check-bitcoin-getarg_tests
[379/402] Running utility command for check-bitcoin-versionbits_tests
[380/402] bitcoin: testing validation_block_tests
[381/402] bitcoin: testing bswap_tests
[382/402] Running utility command for check-bitcoin-validation_block_tests
[383/402] Running utility command for check-bitcoin-bswap_tests
[384/402] bitcoin: testing skiplist_tests
[385/402] Running utility command for check-bitcoin-skiplist_tests
[386/402] bitcoin: testing transaction_tests
[387/402] Running utility command for check-bitcoin-transaction_tests
[388/402] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/wallettests.cpp.o
[389/402] bitcoin: testing util_tests
[390/402] Running utility command for check-bitcoin-util_tests
[391/402] bitcoin: testing cuckoocache_tests
[392/402] Running utility command for check-bitcoin-cuckoocache_tests
[393/402] bitcoin: testing op_reversebytes_tests
[394/402] Running utility command for check-bitcoin-op_reversebytes_tests
[395/402] Linking CXX executable src/qt/test/test_bitcoin-qt
[396/402] bitcoin: testing coins_tests
[397/402] Running utility command for check-bitcoin-coins_tests
[398/402] bitcoin-qt: testing test_bitcoin-qt
[399/402] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
Build build-clang-tidy timed out after 1800.0s
This revision is now accepted and ready to land.Oct 1 2020, 08:15
This revision was landed with ongoing or failed builds.Oct 1 2020, 13:32
This revision was automatically updated to reflect the committed changes.