diff options
-rw-r--r-- | doc/gitian-building.md | 6 | ||||
-rw-r--r-- | doc/gitian-building/system_settings.png | bin | 0 -> 76448 bytes | |||
-rw-r--r-- | doc/release-process.md | 6 | ||||
-rw-r--r-- | src/rpc/blockchain.cpp | 1 | ||||
-rw-r--r-- | src/test/addrman_tests.cpp | 40 | ||||
-rw-r--r-- | src/validation.cpp | 163 | ||||
-rw-r--r-- | src/wallet/rpcdump.cpp | 5 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 166 | ||||
-rw-r--r-- | src/wallet/wallet.h | 30 | ||||
-rwxr-xr-x | test/functional/example_test.py | 8 | ||||
-rwxr-xr-x | test/functional/keypool-topup.py | 75 | ||||
-rwxr-xr-x | test/functional/resendwallettransactions.py | 31 | ||||
-rwxr-xr-x | test/functional/sendheaders.py | 3 | ||||
-rwxr-xr-x | test/functional/test_framework/test_framework.py | 7 | ||||
-rwxr-xr-x | test/functional/test_runner.py | 2 | ||||
-rwxr-xr-x | test/functional/wallet-hd.py | 10 |
16 files changed, 348 insertions, 205 deletions
diff --git a/doc/gitian-building.md b/doc/gitian-building.md index 9f9afaf04f..636686b391 100644 --- a/doc/gitian-building.md +++ b/doc/gitian-building.md @@ -76,7 +76,11 @@ In the VirtualBox GUI click "New" and choose the following parameters in the wiz After creating the VM, we need to configure it. -- Click the `Settings` button, then go to the `Network` tab. Adapter 1 should be attached to `NAT`. +- Click the `Settings` button, then go to `System` tab and `Processor` sub-tab. Increase the number of processors to the number of cores on your machine if you want builds to be faster. + +![](gitian-building/system_settings.png) + +- Go to the `Network` tab. Adapter 1 should be attached to `NAT`. ![](gitian-building/network_settings.png) diff --git a/doc/gitian-building/system_settings.png b/doc/gitian-building/system_settings.png Binary files differnew file mode 100644 index 0000000000..a5720ef3a3 --- /dev/null +++ b/doc/gitian-building/system_settings.png diff --git a/doc/release-process.md b/doc/release-process.md index 5a99b726f1..f429b4bbdb 100644 --- a/doc/release-process.md +++ b/doc/release-process.md @@ -111,16 +111,16 @@ The gbuild invocations below <b>DO NOT DO THIS</b> by default. ### Build and sign Bitcoin Core for Linux, Windows, and OS X: pushd ./gitian-builder - ./bin/gbuild --memory 3000 --commit bitcoin=v${VERSION} ../bitcoin/contrib/gitian-descriptors/gitian-linux.yml + ./bin/gbuild --num-make 2 --memory 3000 --commit bitcoin=v${VERSION} ../bitcoin/contrib/gitian-descriptors/gitian-linux.yml ./bin/gsign --signer $SIGNER --release ${VERSION}-linux --destination ../gitian.sigs/ ../bitcoin/contrib/gitian-descriptors/gitian-linux.yml mv build/out/bitcoin-*.tar.gz build/out/src/bitcoin-*.tar.gz ../ - ./bin/gbuild --memory 3000 --commit bitcoin=v${VERSION} ../bitcoin/contrib/gitian-descriptors/gitian-win.yml + ./bin/gbuild --num-make 2 --memory 3000 --commit bitcoin=v${VERSION} ../bitcoin/contrib/gitian-descriptors/gitian-win.yml ./bin/gsign --signer $SIGNER --release ${VERSION}-win-unsigned --destination ../gitian.sigs/ ../bitcoin/contrib/gitian-descriptors/gitian-win.yml mv build/out/bitcoin-*-win-unsigned.tar.gz inputs/bitcoin-win-unsigned.tar.gz mv build/out/bitcoin-*.zip build/out/bitcoin-*.exe ../ - ./bin/gbuild --memory 3000 --commit bitcoin=v${VERSION} ../bitcoin/contrib/gitian-descriptors/gitian-osx.yml + ./bin/gbuild --num-make 2 --memory 3000 --commit bitcoin=v${VERSION} ../bitcoin/contrib/gitian-descriptors/gitian-osx.yml ./bin/gsign --signer $SIGNER --release ${VERSION}-osx-unsigned --destination ../gitian.sigs/ ../bitcoin/contrib/gitian-descriptors/gitian-osx.yml mv build/out/bitcoin-*-osx-unsigned.tar.gz inputs/bitcoin-osx-unsigned.tar.gz mv build/out/bitcoin-*.tar.gz build/out/bitcoin-*.dmg ../ diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index d65e107e3c..607262f770 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -962,7 +962,6 @@ UniValue gettxout(const JSONRPCRequest& request) " ,...\n" " ]\n" " },\n" - " \"version\" : n, (numeric) The version\n" " \"coinbase\" : true|false (boolean) Coinbase or not\n" "}\n" diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index bc6aef2c11..c53744b495 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -15,9 +15,14 @@ class CAddrManTest : public CAddrMan uint64_t state; public: - CAddrManTest() + CAddrManTest(bool makeDeterministic = true) { state = 1; + + if (makeDeterministic) { + // Set addrman addr placement to be deterministic. + MakeDeterministic(); + } } //! Ensure that bucket placement is always the same for testing purposes. @@ -79,9 +84,6 @@ BOOST_AUTO_TEST_CASE(addrman_simple) { CAddrManTest addrman; - // Set addrman addr placement to be deterministic. - addrman.MakeDeterministic(); - CNetAddr source = ResolveIP("252.2.2.2"); // Test: Does Addrman respond correctly when empty. @@ -131,9 +133,6 @@ BOOST_AUTO_TEST_CASE(addrman_ports) { CAddrManTest addrman; - // Set addrman addr placement to be deterministic. - addrman.MakeDeterministic(); - CNetAddr source = ResolveIP("252.2.2.2"); BOOST_CHECK_EQUAL(addrman.size(), 0); @@ -163,9 +162,6 @@ BOOST_AUTO_TEST_CASE(addrman_select) { CAddrManTest addrman; - // Set addrman addr placement to be deterministic. - addrman.MakeDeterministic(); - CNetAddr source = ResolveIP("252.2.2.2"); // Test: Select from new with 1 addr in new. @@ -225,9 +221,6 @@ BOOST_AUTO_TEST_CASE(addrman_new_collisions) { CAddrManTest addrman; - // Set addrman addr placement to be deterministic. - addrman.MakeDeterministic(); - CNetAddr source = ResolveIP("252.2.2.2"); BOOST_CHECK_EQUAL(addrman.size(), 0); @@ -254,9 +247,6 @@ BOOST_AUTO_TEST_CASE(addrman_tried_collisions) { CAddrManTest addrman; - // Set addrman addr placement to be deterministic. - addrman.MakeDeterministic(); - CNetAddr source = ResolveIP("252.2.2.2"); BOOST_CHECK_EQUAL(addrman.size(), 0); @@ -284,9 +274,6 @@ BOOST_AUTO_TEST_CASE(addrman_find) { CAddrManTest addrman; - // Set addrman addr placement to be deterministic. - addrman.MakeDeterministic(); - BOOST_CHECK_EQUAL(addrman.size(), 0); CAddress addr1 = CAddress(ResolveService("250.1.2.1", 8333), NODE_NONE); @@ -320,9 +307,6 @@ BOOST_AUTO_TEST_CASE(addrman_create) { CAddrManTest addrman; - // Set addrman addr placement to be deterministic. - addrman.MakeDeterministic(); - BOOST_CHECK_EQUAL(addrman.size(), 0); CAddress addr1 = CAddress(ResolveService("250.1.2.1", 8333), NODE_NONE); @@ -343,9 +327,6 @@ BOOST_AUTO_TEST_CASE(addrman_delete) { CAddrManTest addrman; - // Set addrman addr placement to be deterministic. - addrman.MakeDeterministic(); - BOOST_CHECK_EQUAL(addrman.size(), 0); CAddress addr1 = CAddress(ResolveService("250.1.2.1", 8333), NODE_NONE); @@ -366,9 +347,6 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) { CAddrManTest addrman; - // Set addrman addr placement to be deterministic. - addrman.MakeDeterministic(); - // Test: Sanity check, GetAddr should never return anything if addrman // is empty. BOOST_CHECK_EQUAL(addrman.size(), 0); @@ -430,9 +408,6 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket) { CAddrManTest addrman; - // Set addrman addr placement to be deterministic. - addrman.MakeDeterministic(); - CAddress addr1 = CAddress(ResolveService("250.1.1.1", 8333), NODE_NONE); CAddress addr2 = CAddress(ResolveService("250.1.1.1", 9999), NODE_NONE); @@ -487,9 +462,6 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) { CAddrManTest addrman; - // Set addrman addr placement to be deterministic. - addrman.MakeDeterministic(); - CAddress addr1 = CAddress(ResolveService("250.1.2.1", 8333), NODE_NONE); CAddress addr2 = CAddress(ResolveService("250.1.2.1", 9999), NODE_NONE); diff --git a/src/validation.cpp b/src/validation.cpp index 28d45195d1..0f9f6436d8 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1863,95 +1863,100 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd */ bool static FlushStateToDisk(const CChainParams& chainparams, CValidationState &state, FlushStateMode mode, int nManualPruneHeight) { int64_t nMempoolUsage = mempool.DynamicMemoryUsage(); - LOCK2(cs_main, cs_LastBlockFile); + LOCK(cs_main); static int64_t nLastWrite = 0; static int64_t nLastFlush = 0; static int64_t nLastSetChain = 0; std::set<int> setFilesToPrune; bool fFlushForPrune = false; + bool fDoFullFlush = false; + int64_t nNow = 0; try { - if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) { - if (nManualPruneHeight > 0) { - FindFilesToPruneManual(setFilesToPrune, nManualPruneHeight); - } else { - FindFilesToPrune(setFilesToPrune, chainparams.PruneAfterHeight()); - fCheckForPruning = false; - } - if (!setFilesToPrune.empty()) { - fFlushForPrune = true; - if (!fHavePruned) { - pblocktree->WriteFlag("prunedblockfiles", true); - fHavePruned = true; - } - } - } - int64_t nNow = GetTimeMicros(); - // Avoid writing/flushing immediately after startup. - if (nLastWrite == 0) { - nLastWrite = nNow; - } - if (nLastFlush == 0) { - nLastFlush = nNow; - } - if (nLastSetChain == 0) { - nLastSetChain = nNow; - } - int64_t nMempoolSizeMax = GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000; - int64_t cacheSize = pcoinsTip->DynamicMemoryUsage(); - int64_t nTotalSpace = nCoinCacheUsage + std::max<int64_t>(nMempoolSizeMax - nMempoolUsage, 0); - // The cache is large and we're within 10% and 10 MiB of the limit, but we have time now (not in the middle of a block processing). - bool fCacheLarge = mode == FLUSH_STATE_PERIODIC && cacheSize > std::max((9 * nTotalSpace) / 10, nTotalSpace - MAX_BLOCK_COINSDB_USAGE * 1024 * 1024); - // The cache is over the limit, we have to write now. - bool fCacheCritical = mode == FLUSH_STATE_IF_NEEDED && cacheSize > nTotalSpace; - // It's been a while since we wrote the block index to disk. Do this frequently, so we don't need to redownload after a crash. - bool fPeriodicWrite = mode == FLUSH_STATE_PERIODIC && nNow > nLastWrite + (int64_t)DATABASE_WRITE_INTERVAL * 1000000; - // It's been very long since we flushed the cache. Do this infrequently, to optimize cache usage. - bool fPeriodicFlush = mode == FLUSH_STATE_PERIODIC && nNow > nLastFlush + (int64_t)DATABASE_FLUSH_INTERVAL * 1000000; - // Combine all conditions that result in a full cache flush. - bool fDoFullFlush = (mode == FLUSH_STATE_ALWAYS) || fCacheLarge || fCacheCritical || fPeriodicFlush || fFlushForPrune; - // Write blocks and block index to disk. - if (fDoFullFlush || fPeriodicWrite) { - // Depend on nMinDiskSpace to ensure we can write block index - if (!CheckDiskSpace(0)) - return state.Error("out of disk space"); - // First make sure all block and undo data is flushed to disk. - FlushBlockFile(); - // Then update all block file information (which may refer to block and undo files). - { - std::vector<std::pair<int, const CBlockFileInfo*> > vFiles; - vFiles.reserve(setDirtyFileInfo.size()); - for (std::set<int>::iterator it = setDirtyFileInfo.begin(); it != setDirtyFileInfo.end(); ) { - vFiles.push_back(std::make_pair(*it, &vinfoBlockFile[*it])); - setDirtyFileInfo.erase(it++); + { + LOCK(cs_LastBlockFile); + if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) { + if (nManualPruneHeight > 0) { + FindFilesToPruneManual(setFilesToPrune, nManualPruneHeight); + } else { + FindFilesToPrune(setFilesToPrune, chainparams.PruneAfterHeight()); + fCheckForPruning = false; } - std::vector<const CBlockIndex*> vBlocks; - vBlocks.reserve(setDirtyBlockIndex.size()); - for (std::set<CBlockIndex*>::iterator it = setDirtyBlockIndex.begin(); it != setDirtyBlockIndex.end(); ) { - vBlocks.push_back(*it); - setDirtyBlockIndex.erase(it++); + if (!setFilesToPrune.empty()) { + fFlushForPrune = true; + if (!fHavePruned) { + pblocktree->WriteFlag("prunedblockfiles", true); + fHavePruned = true; + } } - if (!pblocktree->WriteBatchSync(vFiles, nLastBlockFile, vBlocks)) { - return AbortNode(state, "Failed to write to block index database"); + } + nNow = GetTimeMicros(); + // Avoid writing/flushing immediately after startup. + if (nLastWrite == 0) { + nLastWrite = nNow; + } + if (nLastFlush == 0) { + nLastFlush = nNow; + } + if (nLastSetChain == 0) { + nLastSetChain = nNow; + } + int64_t nMempoolSizeMax = GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000; + int64_t cacheSize = pcoinsTip->DynamicMemoryUsage(); + int64_t nTotalSpace = nCoinCacheUsage + std::max<int64_t>(nMempoolSizeMax - nMempoolUsage, 0); + // The cache is large and we're within 10% and 10 MiB of the limit, but we have time now (not in the middle of a block processing). + bool fCacheLarge = mode == FLUSH_STATE_PERIODIC && cacheSize > std::max((9 * nTotalSpace) / 10, nTotalSpace - MAX_BLOCK_COINSDB_USAGE * 1024 * 1024); + // The cache is over the limit, we have to write now. + bool fCacheCritical = mode == FLUSH_STATE_IF_NEEDED && cacheSize > nTotalSpace; + // It's been a while since we wrote the block index to disk. Do this frequently, so we don't need to redownload after a crash. + bool fPeriodicWrite = mode == FLUSH_STATE_PERIODIC && nNow > nLastWrite + (int64_t)DATABASE_WRITE_INTERVAL * 1000000; + // It's been very long since we flushed the cache. Do this infrequently, to optimize cache usage. + bool fPeriodicFlush = mode == FLUSH_STATE_PERIODIC && nNow > nLastFlush + (int64_t)DATABASE_FLUSH_INTERVAL * 1000000; + // Combine all conditions that result in a full cache flush. + fDoFullFlush = (mode == FLUSH_STATE_ALWAYS) || fCacheLarge || fCacheCritical || fPeriodicFlush || fFlushForPrune; + // Write blocks and block index to disk. + if (fDoFullFlush || fPeriodicWrite) { + // Depend on nMinDiskSpace to ensure we can write block index + if (!CheckDiskSpace(0)) + return state.Error("out of disk space"); + // First make sure all block and undo data is flushed to disk. + FlushBlockFile(); + // Then update all block file information (which may refer to block and undo files). + { + std::vector<std::pair<int, const CBlockFileInfo*> > vFiles; + vFiles.reserve(setDirtyFileInfo.size()); + for (std::set<int>::iterator it = setDirtyFileInfo.begin(); it != setDirtyFileInfo.end(); ) { + vFiles.push_back(std::make_pair(*it, &vinfoBlockFile[*it])); + setDirtyFileInfo.erase(it++); + } + std::vector<const CBlockIndex*> vBlocks; + vBlocks.reserve(setDirtyBlockIndex.size()); + for (std::set<CBlockIndex*>::iterator it = setDirtyBlockIndex.begin(); it != setDirtyBlockIndex.end(); ) { + vBlocks.push_back(*it); + setDirtyBlockIndex.erase(it++); + } + if (!pblocktree->WriteBatchSync(vFiles, nLastBlockFile, vBlocks)) { + return AbortNode(state, "Failed to write to block index database"); + } } + // Finally remove any pruned files + if (fFlushForPrune) + UnlinkPrunedFiles(setFilesToPrune); + nLastWrite = nNow; + } + // Flush best chain related state. This can only be done if the blocks / block index write was also done. + if (fDoFullFlush) { + // Typical Coin structures on disk are around 48 bytes in size. + // Pushing a new one to the database can cause it to be written + // twice (once in the log, and once in the tables). This is already + // an overestimation, as most will delete an existing entry or + // overwrite one. Still, use a conservative safety factor of 2. + if (!CheckDiskSpace(48 * 2 * 2 * pcoinsTip->GetCacheSize())) + return state.Error("out of disk space"); + // Flush the chainstate (which may refer to block index entries). + if (!pcoinsTip->Flush()) + return AbortNode(state, "Failed to write to coin database"); + nLastFlush = nNow; } - // Finally remove any pruned files - if (fFlushForPrune) - UnlinkPrunedFiles(setFilesToPrune); - nLastWrite = nNow; - } - // Flush best chain related state. This can only be done if the blocks / block index write was also done. - if (fDoFullFlush) { - // Typical Coin structures on disk are around 48 bytes in size. - // Pushing a new one to the database can cause it to be written - // twice (once in the log, and once in the tables). This is already - // an overestimation, as most will delete an existing entry or - // overwrite one. Still, use a conservative safety factor of 2. - if (!CheckDiskSpace(48 * 2 * 2 * pcoinsTip->GetCacheSize())) - return state.Error("out of disk space"); - // Flush the chainstate (which may refer to block index entries). - if (!pcoinsTip->Flush()) - return AbortNode(state, "Failed to write to coin database"); - nLastFlush = nNow; } if (fDoFullFlush || ((mode == FLUSH_STATE_ALWAYS || mode == FLUSH_STATE_PERIODIC) && nNow > nLastSetChain + (int64_t)DATABASE_WRITE_INTERVAL * 1000000)) { // Update best block in wallet (so we can detect restored wallets). diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 5abf32480a..67c6d9ec64 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -619,9 +619,8 @@ UniValue dumpwallet(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file"); std::map<CTxDestination, int64_t> mapKeyBirth; - std::set<CKeyID> setKeyPool; + const std::map<CKeyID, int64_t>& mapKeyPool = pwallet->GetAllReserveKeys(); pwallet->GetKeyBirthTimes(mapKeyBirth); - pwallet->GetAllReserveKeys(setKeyPool); // sort time/key pairs std::vector<std::pair<int64_t, CKeyID> > vKeyBirth; @@ -666,7 +665,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) file << strprintf("label=%s", EncodeDumpString(pwallet->mapAddressBook[keyid].name)); } else if (keyid == masterKeyID) { file << "hdmaster=1"; - } else if (setKeyPool.count(keyid)) { + } else if (mapKeyPool.count(keyid)) { file << "reserve=1"; } else if (pwallet->mapKeyMetadata[keyid].hdKeypath == "m") { file << "inactivehdmaster=1"; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 18cc3bd028..8b03421691 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -12,6 +12,7 @@ #include "consensus/consensus.h" #include "consensus/validation.h" #include "fs.h" +#include "init.h" #include "key.h" #include "keystore.h" #include "validation.h" @@ -80,6 +81,38 @@ std::string COutput::ToString() const return strprintf("COutput(%s, %d, %d) [%s]", tx->GetHash().ToString(), i, nDepth, FormatMoney(tx->tx->vout[i].nValue)); } +class CAffectedKeysVisitor : public boost::static_visitor<void> { +private: + const CKeyStore &keystore; + std::vector<CKeyID> &vKeys; + +public: + CAffectedKeysVisitor(const CKeyStore &keystoreIn, std::vector<CKeyID> &vKeysIn) : keystore(keystoreIn), vKeys(vKeysIn) {} + + void Process(const CScript &script) { + txnouttype type; + std::vector<CTxDestination> vDest; + int nRequired; + if (ExtractDestinations(script, type, vDest, nRequired)) { + for (const CTxDestination &dest : vDest) + boost::apply_visitor(*this, dest); + } + } + + void operator()(const CKeyID &keyId) { + if (keystore.HaveKey(keyId)) + vKeys.push_back(keyId); + } + + void operator()(const CScriptID &scriptId) { + CScript script; + if (keystore.GetCScript(scriptId, script)) + Process(script); + } + + void operator()(const CNoDestination &none) {} +}; + const CWalletTx* CWallet::GetWalletTx(const uint256& hash) const { LOCK(cs_wallet); @@ -1021,6 +1054,30 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const CBlockI if (fExisted && !fUpdate) return false; if (fExisted || IsMine(tx) || IsFromMe(tx)) { + /* Check if any keys in the wallet keypool that were supposed to be unused + * have appeared in a new transaction. If so, remove those keys from the keypool. + * This can happen when restoring an old wallet backup that does not contain + * the mostly recently created transactions from newer versions of the wallet. + */ + + // loop though all outputs + for (const CTxOut& txout: tx.vout) { + // extract addresses and check if they match with an unused keypool key + std::vector<CKeyID> vAffected; + CAffectedKeysVisitor(*this, vAffected).Process(txout.scriptPubKey); + for (const CKeyID &keyid : vAffected) { + std::map<CKeyID, int64_t>::const_iterator mi = m_pool_key_to_index.find(keyid); + if (mi != m_pool_key_to_index.end()) { + LogPrintf("%s: Detected a used keypool key, mark all keypool key up to this key as used\n", __func__); + MarkReserveKeysAsUsed(mi->second); + + if (!TopUpKeyPool()) { + LogPrintf("%s: Topping up keypool failed (locked wallet)\n", __func__); + } + } + } + } + CWalletTx wtx(this, ptx); // Get merkle branch if transaction was found in a block @@ -3050,6 +3107,7 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) LOCK(cs_wallet); setInternalKeyPool.clear(); setExternalKeyPool.clear(); + m_pool_key_to_index.clear(); // Note: can't top-up keypool here, because wallet is locked. // User will be prompted to unlock wallet the next operation // that requires a new key. @@ -3079,6 +3137,7 @@ DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256 { setInternalKeyPool.clear(); setExternalKeyPool.clear(); + m_pool_key_to_index.clear(); // Note: can't top-up keypool here, because wallet is locked. // User will be prompted to unlock wallet the next operation // that requires a new key. @@ -3105,6 +3164,7 @@ DBErrors CWallet::ZapWalletTx(std::vector<CWalletTx>& vWtx) LOCK(cs_wallet); setInternalKeyPool.clear(); setExternalKeyPool.clear(); + m_pool_key_to_index.clear(); // Note: can't top-up keypool here, because wallet is locked. // User will be prompted to unlock wallet the next operation // that requires a new key. @@ -3199,6 +3259,8 @@ bool CWallet::NewKeyPool() } setExternalKeyPool.clear(); + m_pool_key_to_index.clear(); + if (!TopUpKeyPool()) { return false; } @@ -3213,6 +3275,25 @@ size_t CWallet::KeypoolCountExternalKeys() return setExternalKeyPool.size(); } +void CWallet::LoadKeyPool(int64_t nIndex, const CKeyPool &keypool) +{ + AssertLockHeld(cs_wallet); + if (keypool.fInternal) { + setInternalKeyPool.insert(nIndex); + } else { + setExternalKeyPool.insert(nIndex); + } + m_max_keypool_index = std::max(m_max_keypool_index, nIndex); + m_pool_key_to_index[keypool.vchPubKey.GetID()] = nIndex; + + // If no metadata exists yet, create a default with the pool key's + // creation time. Note that this may be overwritten by actually + // stored metadata for that key later, which is fine. + CKeyID keyid = keypool.vchPubKey.GetID(); + if (mapKeyMetadata.count(keyid) == 0) + mapKeyMetadata[keyid] = CKeyMetadata(keypool.nTime); +} + bool CWallet::TopUpKeyPool(unsigned int kpSize) { { @@ -3249,7 +3330,8 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize) assert(m_max_keypool_index < std::numeric_limits<int64_t>::max()); // How in the hell did you use so many keys? int64_t index = ++m_max_keypool_index; - if (!walletdb.WritePool(index, CKeyPool(GenerateNewKey(walletdb, internal), internal))) { + CPubKey pubkey(GenerateNewKey(walletdb, internal)); + if (!walletdb.WritePool(index, CKeyPool(pubkey, internal))) { throw std::runtime_error(std::string(__func__) + ": writing generated key failed"); } @@ -3258,6 +3340,7 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize) } else { setExternalKeyPool.insert(index); } + m_pool_key_to_index[pubkey.GetID()] = index; } if (missingInternal + missingExternal > 0) { LogPrintf("keypool added %d keys (%d internal), size=%u (%u internal)\n", missingInternal + missingExternal, missingInternal, setInternalKeyPool.size() + setExternalKeyPool.size(), setInternalKeyPool.size()); @@ -3299,6 +3382,7 @@ void CWallet::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fRe } assert(keypool.vchPubKey.IsValid()); + m_pool_key_to_index.erase(keypool.vchPubKey.GetID()); LogPrintf("keypool reserve %d\n", nIndex); } } @@ -3311,7 +3395,7 @@ void CWallet::KeepKey(int64_t nIndex) LogPrintf("keypool keep %d\n", nIndex); } -void CWallet::ReturnKey(int64_t nIndex, bool fInternal) +void CWallet::ReturnKey(int64_t nIndex, bool fInternal, const CPubKey& pubkey) { // Return to key pool { @@ -3321,6 +3405,7 @@ void CWallet::ReturnKey(int64_t nIndex, bool fInternal) } else { setExternalKeyPool.insert(nIndex); } + m_pool_key_to_index[pubkey.GetID()] = nIndex; } LogPrintf("keypool return %d\n", nIndex); } @@ -3550,41 +3635,39 @@ void CReserveKey::KeepKey() void CReserveKey::ReturnKey() { if (nIndex != -1) { - pwallet->ReturnKey(nIndex, fInternal); + pwallet->ReturnKey(nIndex, fInternal, vchPubKey); } nIndex = -1; vchPubKey = CPubKey(); } -static void LoadReserveKeysToSet(std::set<CKeyID>& setAddress, const std::set<int64_t>& setKeyPool, CWalletDB& walletdb) { - for (const int64_t& id : setKeyPool) - { - CKeyPool keypool; - if (!walletdb.ReadPool(id, keypool)) - throw std::runtime_error(std::string(__func__) + ": read failed"); - assert(keypool.vchPubKey.IsValid()); - CKeyID keyID = keypool.vchPubKey.GetID(); - setAddress.insert(keyID); - } -} - -void CWallet::GetAllReserveKeys(std::set<CKeyID>& setAddress) const +void CWallet::MarkReserveKeysAsUsed(int64_t keypool_id) { - setAddress.clear(); + AssertLockHeld(cs_wallet); + bool internal = setInternalKeyPool.count(keypool_id); + if (!internal) assert(setExternalKeyPool.count(keypool_id)); + std::set<int64_t> *setKeyPool = internal ? &setInternalKeyPool : &setExternalKeyPool; + auto it = setKeyPool->begin(); CWalletDB walletdb(*dbw); + while (it != std::end(*setKeyPool)) { + const int64_t& index = *(it); + if (index > keypool_id) break; // set*KeyPool is ordered - LOCK2(cs_main, cs_wallet); - LoadReserveKeysToSet(setAddress, setInternalKeyPool, walletdb); - LoadReserveKeysToSet(setAddress, setExternalKeyPool, walletdb); - - for (const CKeyID& keyID : setAddress) { - if (!HaveKey(keyID)) { - throw std::runtime_error(std::string(__func__) + ": unknown key in key pool"); + CKeyPool keypool; + if (walletdb.ReadPool(index, keypool)) { //TODO: This should be unnecessary + m_pool_key_to_index.erase(keypool.vchPubKey.GetID()); } + walletdb.ErasePool(index); + it = setKeyPool->erase(it); } } +bool CWallet::HasUnusedKeys(int min_keys) const +{ + return setExternalKeyPool.size() >= min_keys && (setInternalKeyPool.size() >= min_keys || !CanSupportFeature(FEATURE_HD_SPLIT)); +} + void CWallet::GetScriptForMining(std::shared_ptr<CReserveScript> &script) { std::shared_ptr<CReserveKey> rKey = std::make_shared<CReserveKey>(this); @@ -3634,38 +3717,6 @@ void CWallet::ListLockedCoins(std::vector<COutPoint>& vOutpts) const /** @} */ // end of Actions -class CAffectedKeysVisitor : public boost::static_visitor<void> { -private: - const CKeyStore &keystore; - std::vector<CKeyID> &vKeys; - -public: - CAffectedKeysVisitor(const CKeyStore &keystoreIn, std::vector<CKeyID> &vKeysIn) : keystore(keystoreIn), vKeys(vKeysIn) {} - - void Process(const CScript &script) { - txnouttype type; - std::vector<CTxDestination> vDest; - int nRequired; - if (ExtractDestinations(script, type, vDest, nRequired)) { - for (const CTxDestination &dest : vDest) - boost::apply_visitor(*this, dest); - } - } - - void operator()(const CKeyID &keyId) { - if (keystore.HaveKey(keyId)) - vKeys.push_back(keyId); - } - - void operator()(const CScriptID &scriptId) { - CScript script; - if (keystore.GetCScript(scriptId, script)) - Process(script); - } - - void operator()(const CNoDestination &none) {} -}; - void CWallet::GetKeyBirthTimes(std::map<CTxDestination, int64_t> &mapKeyBirth) const { AssertLockHeld(cs_wallet); // mapKeyMetadata mapKeyBirth.clear(); @@ -3990,6 +4041,9 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) RegisterValidationInterface(walletInstance); + // Try to top up keypool. No-op if the wallet is locked. + walletInstance->TopUpKeyPool(); + CBlockIndex *pindexRescan = chainActive.Genesis(); if (!GetBoolArg("-rescan", false)) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 3a02d1305d..0e26d6cc6a 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -705,6 +705,7 @@ private: std::set<int64_t> setInternalKeyPool; std::set<int64_t> setExternalKeyPool; int64_t m_max_keypool_index; + std::map<CKeyID, int64_t> m_pool_key_to_index; int64_t nTimeFirstKey; @@ -747,22 +748,7 @@ public: } } - void LoadKeyPool(int64_t nIndex, const CKeyPool &keypool) - { - if (keypool.fInternal) { - setInternalKeyPool.insert(nIndex); - } else { - setExternalKeyPool.insert(nIndex); - } - m_max_keypool_index = std::max(m_max_keypool_index, nIndex); - - // If no metadata exists yet, create a default with the pool key's - // creation time. Note that this may be overwritten by actually - // stored metadata for that key later, which is fine. - CKeyID keyid = keypool.vchPubKey.GetID(); - if (mapKeyMetadata.count(keyid) == 0) - mapKeyMetadata[keyid] = CKeyMetadata(keypool.nTime); - } + void LoadKeyPool(int64_t nIndex, const CKeyPool &keypool); // Map from Key ID (for regular keys) or Script ID (for watch-only keys) to // key metadata. @@ -828,7 +814,7 @@ public: const CWalletTx* GetWalletTx(const uint256& hash) const; //! check whether we are allowed to upgrade (or already support) to the named feature - bool CanSupportFeature(enum WalletFeature wf) { AssertLockHeld(cs_wallet); return nWalletMaxVersion >= wf; } + bool CanSupportFeature(enum WalletFeature wf) const { AssertLockHeld(cs_wallet); return nWalletMaxVersion >= wf; } /** * populate vCoins with vector of available COutputs. @@ -990,10 +976,16 @@ public: bool TopUpKeyPool(unsigned int kpSize = 0); void ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool fRequestedInternal); void KeepKey(int64_t nIndex); - void ReturnKey(int64_t nIndex, bool fInternal); + void ReturnKey(int64_t nIndex, bool fInternal, const CPubKey& pubkey); bool GetKeyFromPool(CPubKey &key, bool internal = false); int64_t GetOldestKeyPoolTime(); - void GetAllReserveKeys(std::set<CKeyID>& setAddress) const; + /** + * Marks all keys in the keypool up to and including reserve_key as used. + */ + void MarkReserveKeysAsUsed(int64_t keypool_id); + const std::map<CKeyID, int64_t>& GetAllReserveKeys() const { return m_pool_key_to_index; } + /** Does the wallet have at least min_keys in the keypool? */ + bool HasUnusedKeys(int min_keys) const; std::set< std::set<CTxDestination> > GetAddressGroupings(); std::map<CTxDestination, CAmount> GetAddressBalances(); diff --git a/test/functional/example_test.py b/test/functional/example_test.py index 1ba5f756cd..7709524f23 100755 --- a/test/functional/example_test.py +++ b/test/functional/example_test.py @@ -58,6 +58,10 @@ class BaseNode(NodeConnCB): message.block.calc_sha256() self.block_receive_map[message.block.sha256] += 1 + def on_inv(self, conn, message): + """Override the standard on_inv callback""" + pass + def custom_function(): """Do some custom behaviour @@ -198,10 +202,10 @@ class ExampleTest(BitcoinTestFramework): self.log.info("Wait for node2 reach current tip. Test that it has propogated all the blocks to us") + getdata_request = msg_getdata() for block in blocks: - getdata_request = msg_getdata() getdata_request.inv.append(CInv(2, block)) - node2.send_message(getdata_request) + node2.send_message(getdata_request) # wait_until() will loop until a predicate condition is met. Use it to test properties of the # NodeConnCB objects. diff --git a/test/functional/keypool-topup.py b/test/functional/keypool-topup.py new file mode 100755 index 0000000000..0e0c0ea74b --- /dev/null +++ b/test/functional/keypool-topup.py @@ -0,0 +1,75 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test HD Wallet keypool restore function. + +Two nodes. Node1 is under test. Node0 is providing transactions and generating blocks. + +- Start node1, shutdown and backup wallet. +- Generate 110 keys (enough to drain the keypool). Store key 90 (in the initial keypool) and key 110 (beyond the initial keypool). Send funds to key 90 and key 110. +- Stop node1, clear the datadir, move wallet file back into the datadir and restart node1. +- connect node1 to node0. Verify that they sync and node1 receives its funds.""" +import shutil + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + connect_nodes_bi, + sync_blocks, +) + +class KeypoolRestoreTest(BitcoinTestFramework): + def __init__(self): + super().__init__() + self.setup_clean_chain = True + self.num_nodes = 2 + self.extra_args = [['-usehd=0'], ['-usehd=1', '-keypool=100', '-keypoolmin=20']] + + def run_test(self): + self.tmpdir = self.options.tmpdir + self.nodes[0].generate(101) + + self.log.info("Make backup of wallet") + + self.stop_node(1) + + shutil.copyfile(self.tmpdir + "/node1/regtest/wallet.dat", self.tmpdir + "/wallet.bak") + self.nodes[1] = self.start_node(1, self.tmpdir, self.extra_args[1]) + connect_nodes_bi(self.nodes, 0, 1) + + self.log.info("Generate keys for wallet") + + for _ in range(90): + addr_oldpool = self.nodes[1].getnewaddress() + for _ in range(20): + addr_extpool = self.nodes[1].getnewaddress() + + self.log.info("Send funds to wallet") + + self.nodes[0].sendtoaddress(addr_oldpool, 10) + self.nodes[0].generate(1) + self.nodes[0].sendtoaddress(addr_extpool, 5) + self.nodes[0].generate(1) + sync_blocks(self.nodes) + + self.log.info("Restart node with wallet backup") + + self.stop_node(1) + + shutil.copyfile(self.tmpdir + "/wallet.bak", self.tmpdir + "/node1/regtest/wallet.dat") + + self.log.info("Verify keypool is restored and balance is correct") + + self.nodes[1] = self.start_node(1, self.tmpdir, self.extra_args[1]) + connect_nodes_bi(self.nodes, 0, 1) + self.sync_all() + + assert_equal(self.nodes[1].getbalance(), 15) + assert_equal(self.nodes[1].listtransactions()[0]['category'], "receive") + + # Check that we have marked all keys up to the used keypool key as used + assert_equal(self.nodes[1].validateaddress(self.nodes[1].getnewaddress())['hdkeypath'], "m/0'/0'/111'") + +if __name__ == '__main__': + KeypoolRestoreTest().main() diff --git a/test/functional/resendwallettransactions.py b/test/functional/resendwallettransactions.py new file mode 100755 index 0000000000..5059aa106e --- /dev/null +++ b/test/functional/resendwallettransactions.py @@ -0,0 +1,31 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test resendwallettransactions RPC.""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal, assert_raises_jsonrpc + +class ResendWalletTransactionsTest(BitcoinTestFramework): + + def __init__(self): + super().__init__() + self.extra_args = [['--walletbroadcast=false']] + self.num_nodes = 1 + + def run_test(self): + # Should raise RPC_WALLET_ERROR (-4) if walletbroadcast is disabled. + assert_raises_jsonrpc(-4, "Error: Wallet transaction broadcasting is disabled with -walletbroadcast", self.nodes[0].resendwallettransactions) + + # Should return an empty array if there aren't unconfirmed wallet transactions. + self.stop_node(0) + self.nodes[0] = self.start_node(0, self.options.tmpdir) + assert_equal(self.nodes[0].resendwallettransactions(), []) + + # Should return an array with the unconfirmed wallet transaction. + txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) + assert_equal(self.nodes[0].resendwallettransactions(), [txid]) + +if __name__ == '__main__': + ResendWalletTransactionsTest().main() diff --git a/test/functional/sendheaders.py b/test/functional/sendheaders.py index 44c357c6db..e47e07fb86 100755 --- a/test/functional/sendheaders.py +++ b/test/functional/sendheaders.py @@ -121,9 +121,6 @@ class TestNode(NodeConnCB): message.headers[-1].calc_sha256() self.last_blockhash_announced = message.headers[-1].sha256 - def on_block(self, conn, message): - self.last_message["block"].calc_sha256() - # Test whether the last announcement we received had the # right header or the right inv # inv and headers should be lists of block hashes diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 8d698a7327..e562d11938 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -11,6 +11,7 @@ import http.client import logging import optparse import os +import pdb import shutil import subprocess import sys @@ -125,6 +126,8 @@ class BitcoinTestFramework(object): help="Write tested RPC commands into this directory") parser.add_option("--configfile", dest="configfile", help="Location of the test framework config file") + parser.add_option("--pdbonfailure", dest="pdbonfailure", default=False, action="store_true", + help="Attach a python debugger if test fails") self.add_options(parser) (self.options, self.args) = parser.parse_args() @@ -162,6 +165,10 @@ class BitcoinTestFramework(object): except KeyboardInterrupt as e: self.log.warning("Exiting after keyboard interrupt") + if success == TestStatus.FAILED and self.options.pdbonfailure: + print("Testcase failed. Attaching python debugger. Enter ? for help") + pdb.set_trace() + if not self.options.noshutdown: self.log.info("Stopping nodes") if self.nodes: diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index dfd3b89895..93f180555d 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -79,6 +79,7 @@ BASE_SCRIPTS= [ 'rawtransactions.py', 'reindex.py', # vv Tests less than 30s vv + 'keypool-topup.py', 'zmq_test.py', 'mempool_resurrect_test.py', 'txn_doublespend.py --mineblock', @@ -118,6 +119,7 @@ BASE_SCRIPTS= [ 'bipdersig-p2p.py', 'bip65-cltv-p2p.py', 'uptime.py', + 'resendwallettransactions.py', ] EXTENDED_SCRIPTS = [ diff --git a/test/functional/wallet-hd.py b/test/functional/wallet-hd.py index dfd3dc83c5..821575ed19 100755 --- a/test/functional/wallet-hd.py +++ b/test/functional/wallet-hd.py @@ -9,7 +9,6 @@ from test_framework.util import ( assert_equal, connect_nodes_bi, ) -import os import shutil @@ -72,10 +71,12 @@ class WalletHDTest(BitcoinTestFramework): self.log.info("Restore backup ...") self.stop_node(1) - os.remove(self.options.tmpdir + "/node1/regtest/wallet.dat") + # we need to delete the complete regtest directory + # otherwise node1 would auto-recover all funds in flag the keypool keys as used + shutil.rmtree(tmpdir + "/node1/regtest/blocks") + shutil.rmtree(tmpdir + "/node1/regtest/chainstate") shutil.copyfile(tmpdir + "/hd.bak", tmpdir + "/node1/regtest/wallet.dat") self.nodes[1] = self.start_node(1, self.options.tmpdir, self.extra_args[1]) - #connect_nodes_bi(self.nodes, 0, 1) # Assert that derivation is deterministic hd_add_2 = None @@ -85,11 +86,12 @@ class WalletHDTest(BitcoinTestFramework): assert_equal(hd_info_2["hdkeypath"], "m/0'/0'/"+str(_+1)+"'") assert_equal(hd_info_2["hdmasterkeyid"], masterkeyid) assert_equal(hd_add, hd_add_2) + connect_nodes_bi(self.nodes, 0, 1) + self.sync_all() # Needs rescan self.stop_node(1) self.nodes[1] = self.start_node(1, self.options.tmpdir, self.extra_args[1] + ['-rescan']) - #connect_nodes_bi(self.nodes, 0, 1) assert_equal(self.nodes[1].getbalance(), num_hd_adds + 1) # send a tx and make sure its using the internal chain for the changeoutput |