diff --git a/configure.ac b/configure.ac index 915dc8d49d720..fb5297522b638 100644 --- a/configure.ac +++ b/configure.ac @@ -1974,16 +1974,16 @@ echo " boost process = $with_boost_process" echo " multiprocess = $build_multiprocess" echo " with libs = $build_bitcoin_libs" echo " with wallet = $enable_wallet" -echo " with gui / qt = $bitcoin_enable_qt" if test "x$enable_wallet" != "xno"; then - echo " with sqlite = $use_sqlite" - echo " with bdb = $use_bdb" + echo " with sqlite = $use_sqlite" + echo " with bdb = $use_bdb" fi +echo " with gui / qt = $bitcoin_enable_qt" if test x$bitcoin_enable_qt != xno; then echo " with qr = $use_qr" fi echo " with zmq = $use_zmq" -if test x$enable_fuzz == xno; then +if test x$enable_fuzz = xno; then echo " with test = $use_tests" else echo " with test = not building test_dash because fuzzing is enabled" diff --git a/doc/release-notes-6530.md b/doc/release-notes-6530.md new file mode 100644 index 0000000000000..4988cbfb55a02 --- /dev/null +++ b/doc/release-notes-6530.md @@ -0,0 +1,15 @@ +Notable changes +=============== + +Updated RPCs +------------ + +- `lockunspent` now optionally takes a third parameter, `persistent`, which +causes the lock to be written persistently to the wallet database. This +allows UTXOs to remain locked even after node restarts or crashes. + +GUI changes +----------- + +- UTXOs which are locked via the GUI are now stored persistently in the +wallet database, so are not lost on node shutdown or crash. diff --git a/src/Makefile.am b/src/Makefile.am index 98f3ab4ea865b..665102a2cadd5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -372,6 +372,7 @@ BITCOIN_CORE_H = \ util/sock.h \ util/string.h \ util/spanparsing.h \ + util/syserror.h \ util/system.h \ util/time.h \ util/thread.h \ @@ -825,6 +826,7 @@ libbitcoin_util_a_SOURCES = \ util/hasher.cpp \ util/getuniquepath.cpp \ util/sock.cpp \ + util/syserror.cpp \ util/system.cpp \ util/message.cpp \ util/moneystr.cpp \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index c859010acf23f..00e0429688fd7 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -139,6 +139,7 @@ BITCOIN_TESTS =\ test/net_peer_eviction_tests.cpp \ test/net_tests.cpp \ test/netbase_tests.cpp \ + test/orphanage_tests.cpp \ test/pmt_tests.cpp \ test/policyestimator_tests.cpp \ test/pool_tests.cpp \ diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index a3b9910e2d1c3..0442c3351d67a 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -212,7 +213,7 @@ static bool AppInit(NodeContext& node, int argc, char* argv[]) } break; case -1: // Error happened. - return InitError(Untranslated(strprintf("fork_daemon() failed: %s\n", strerror(errno)))); + return InitError(Untranslated(strprintf("fork_daemon() failed: %s\n", SysErrorString(errno)))); default: { // Parent: wait and exit. int token = daemon_ep.TokenRead(); if (token) { // Success diff --git a/src/fs.cpp b/src/fs.cpp index 03afb11ba4091..7dabd249e869a 100644 --- a/src/fs.cpp +++ b/src/fs.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #ifndef WIN32 #include @@ -41,7 +42,7 @@ fs::path AbsPathJoin(const fs::path& base, const fs::path& path) static std::string GetErrorReason() { - return std::strerror(errno); + return SysErrorString(errno); } FileLock::FileLock(const fs::path& file) diff --git a/src/init.cpp b/src/init.cpp index 258be50fee660..dbd3e4ce11ade 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -66,6 +66,7 @@ #include #include #include +#include #include #include #include @@ -165,7 +166,7 @@ static fs::path GetPidFile(const ArgsManager& args) #endif return true; } else { - return InitError(strprintf(_("Unable to create the PID file '%s': %s"), fs::PathToString(GetPidFile(args)), std::strerror(errno))); + return InitError(strprintf(_("Unable to create the PID file '%s': %s"), fs::PathToString(GetPidFile(args)), SysErrorString(errno))); } } @@ -1833,48 +1834,48 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) uiInterface.InitMessage(_("Loading block index…").translated); const auto load_block_index_start_time{SteadyClock::now()}; - std::optional rv; + std::optional maybe_load_error; try { - rv = LoadChainstate(fReset, - chainman, - *node.govman, - *node.mn_metaman, - *node.mn_sync, - *node.sporkman, - node.mn_activeman, - node.chain_helper, - node.cpoolman, - node.dmnman, - node.evodb, - node.mnhf_manager, - node.llmq_ctx, - Assert(node.mempool.get()), - fPruneMode, - args.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX), - is_governance_enabled, - args.GetBoolArg("-spentindex", DEFAULT_SPENTINDEX), - args.GetBoolArg("-timestampindex", DEFAULT_TIMESTAMPINDEX), - args.GetBoolArg("-txindex", DEFAULT_TXINDEX), - chainparams.GetConsensus(), - chainparams.NetworkIDString(), - fReindexChainState, - cache_sizes.block_tree_db, - cache_sizes.coins_db, - cache_sizes.coins, - /*block_tree_db_in_memory=*/false, - /*coins_db_in_memory=*/false, - ShutdownRequested, - []() { - uiInterface.ThreadSafeMessageBox( - _("Error reading from database, shutting down."), - "", CClientUIInterface::MSG_ERROR); - }); + maybe_load_error = LoadChainstate(fReset, + chainman, + *node.govman, + *node.mn_metaman, + *node.mn_sync, + *node.sporkman, + node.mn_activeman, + node.chain_helper, + node.cpoolman, + node.dmnman, + node.evodb, + node.mnhf_manager, + node.llmq_ctx, + Assert(node.mempool.get()), + fPruneMode, + args.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX), + is_governance_enabled, + args.GetBoolArg("-spentindex", DEFAULT_SPENTINDEX), + args.GetBoolArg("-timestampindex", DEFAULT_TIMESTAMPINDEX), + args.GetBoolArg("-txindex", DEFAULT_TXINDEX), + chainparams.GetConsensus(), + chainparams.NetworkIDString(), + fReindexChainState, + cache_sizes.block_tree_db, + cache_sizes.coins_db, + cache_sizes.coins, + /*block_tree_db_in_memory=*/false, + /*coins_db_in_memory=*/false, + /*shutdown_requested=*/ShutdownRequested, + /*coins_error_cb=*/[]() { + uiInterface.ThreadSafeMessageBox( + _("Error reading from database, shutting down."), + "", CClientUIInterface::MSG_ERROR); + }); } catch (const std::exception& e) { LogPrintf("%s\n", e.what()); - rv = ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED; + maybe_load_error = ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED; } - if (rv.has_value()) { - switch (rv.value()) { + if (maybe_load_error.has_value()) { + switch (maybe_load_error.value()) { case ChainstateLoadingError::ERROR_LOADING_BLOCK_DB: strLoadError = _("Error loading block database"); break; @@ -1930,7 +1931,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) LogPrintf("%s: timestamp index %s\n", __func__, fTimestampIndex ? "enabled" : "disabled"); LogPrintf("%s: spent index %s\n", __func__, fSpentIndex ? "enabled" : "disabled"); - std::optional rv2; + std::optional maybe_verify_error; try { uiInterface.InitMessage(_("Verifying blocks…").translated); auto check_blocks = args.GetArg("-checkblocks", DEFAULT_CHECKBLOCKS); @@ -1938,23 +1939,23 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) LogPrintf("Prune: pruned datadir may not have more than %d blocks; only checking available blocks\n", MIN_BLOCKS_TO_KEEP); } - rv2 = VerifyLoadedChainstate(chainman, - *Assert(node.evodb.get()), - fReset, - fReindexChainState, - chainparams.GetConsensus(), - check_blocks, - args.GetArg("-checklevel", DEFAULT_CHECKLEVEL), - static_cast(GetTime), - [](bool bls_state) { - LogPrintf("%s: bls_legacy_scheme=%d\n", __func__, bls_state); - }); + maybe_verify_error = VerifyLoadedChainstate(chainman, + *Assert(node.evodb.get()), + fReset, + fReindexChainState, + chainparams.GetConsensus(), + check_blocks, + args.GetArg("-checklevel", DEFAULT_CHECKLEVEL), + /*get_unix_time_seconds=*/static_cast(GetTime), + [](bool bls_state) { + LogPrintf("%s: bls_legacy_scheme=%d\n", __func__, bls_state); + }); } catch (const std::exception& e) { LogPrintf("%s\n", e.what()); - rv2 = ChainstateLoadVerifyError::ERROR_GENERIC_FAILURE; + maybe_verify_error = ChainstateLoadVerifyError::ERROR_GENERIC_FAILURE; } - if (rv2.has_value()) { - switch (rv2.value()) { + if (maybe_verify_error.has_value()) { + switch (maybe_verify_error.value()) { case ChainstateLoadVerifyError::ERROR_BLOCK_FROM_FUTURE: strLoadError = _("The block database contains a block which appears to be from the future. " "This may be due to your computer's date and time being set incorrectly. " diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index 9e9ba4ec27366..6de6776ef5d26 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -136,10 +136,10 @@ class Wallet virtual bool setAddressReceiveRequest(const CTxDestination& dest, const std::string& id, const std::string& value) = 0; //! Lock coin. - virtual void lockCoin(const COutPoint& output) = 0; + virtual bool lockCoin(const COutPoint& output, const bool write_to_db) = 0; //! Unlock coin. - virtual void unlockCoin(const COutPoint& output) = 0; + virtual bool unlockCoin(const COutPoint& output) = 0; //! Return whether coin is locked. virtual bool isLockedCoin(const COutPoint& output) = 0; diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index d184be669fbdf..0f00280734981 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -289,8 +289,13 @@ bool BlockManager::LoadBlockIndex(const Consensus::Params& consensus_params) std::sort(vSortedByHeight.begin(), vSortedByHeight.end(), CBlockIndexHeightOnlyComparator()); + CBlockIndex* previous_index{nullptr}; for (CBlockIndex* pindex : vSortedByHeight) { if (ShutdownRequested()) return false; + if (previous_index && pindex->nHeight > previous_index->nHeight + 1) { + return error("%s: block index is non-contiguous, index of height %d missing", __func__, previous_index->nHeight + 1); + } + previous_index = pindex; pindex->nChainWork = (pindex->pprev ? pindex->pprev->nChainWork : 0) + GetBlockProof(*pindex); pindex->nTimeMax = (pindex->pprev ? std::max(pindex->pprev->nTimeMax, pindex->nTime) : pindex->nTime); diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index d379a0ee6531d..dde3c8b021882 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -261,8 +261,8 @@ std::optional VerifyLoadedChainstate(ChainstateManage bool fReset, bool fReindexChainState, const Consensus::Params& consensus_params, - unsigned int check_blocks, - unsigned int check_level, + int check_blocks, + int check_level, std::function get_unix_time_seconds, std::function notify_bls_state) { @@ -275,7 +275,7 @@ std::optional VerifyLoadedChainstate(ChainstateManage for (CChainState* chainstate : chainman.GetAll()) { if (!is_coinsview_empty(chainstate)) { const CBlockIndex* tip = chainstate->m_chain.Tip(); - if (tip && tip->nTime > get_unix_time_seconds() + 2 * 60 * 60) { + if (tip && tip->nTime > get_unix_time_seconds() + MAX_FUTURE_BLOCK_TIME) { return ChainstateLoadVerifyError::ERROR_BLOCK_FROM_FUTURE; } const bool v19active{DeploymentActiveAfter(tip, consensus_params, Consensus::DEPLOYMENT_V19)}; diff --git a/src/node/chainstate.h b/src/node/chainstate.h index c7b399177e412..8b0c738a85baa 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -71,7 +71,7 @@ enum class ChainstateLoadingError { * this sequence, when we explicitly checked shutdown_requested() at * arbitrary points, one of those calls returned true". Therefore, a * return value other than SHUTDOWN_PROBED does not guarantee that - * shutdown_requested() hasn't been called indirectly. + * shutdown hasn't been called indirectly. * - else * - Success! */ @@ -143,8 +143,8 @@ std::optional VerifyLoadedChainstate(ChainstateManage bool fReset, bool fReindexChainState, const Consensus::Params& consensus_params, - unsigned int check_blocks, - unsigned int check_level, + int check_blocks, + int check_level, std::function get_unix_time_seconds, std::function notify_bls_state = nullptr); diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index 89206909d3dad..541e8b2980365 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -207,7 +207,7 @@ void CoinControlDialog::buttonToggleLockClicked() item->setIcon(COLUMN_CHECKBOX, QIcon()); } else{ - model->wallet().lockCoin(outpt); + model->wallet().lockCoin(outpt, /*write_to_db=*/true); item->setDisabled(true); item->setIcon(COLUMN_CHECKBOX, GUIUtil::getIcon("lock_closed", GUIUtil::ThemedColor::RED)); } @@ -300,7 +300,7 @@ void CoinControlDialog::lockCoin() contextMenuItem->setCheckState(COLUMN_CHECKBOX, Qt::Unchecked); COutPoint outpt(uint256S(contextMenuItem->data(COLUMN_ADDRESS, TxHashRole).toString().toStdString()), contextMenuItem->data(COLUMN_ADDRESS, VOutRole).toUInt()); - model->wallet().lockCoin(outpt); + model->wallet().lockCoin(outpt, /*write_to_db=*/true); contextMenuItem->setDisabled(true); contextMenuItem->setIcon(COLUMN_CHECKBOX, GUIUtil::getIcon("lock_closed", GUIUtil::ThemedColor::RED)); updateLabelLocked(); diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 90c8590214677..f28247a165399 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -154,6 +154,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "gettxoutsetinfo", 2, "use_index"}, { "lockunspent", 0, "unlock" }, { "lockunspent", 1, "transactions" }, + { "lockunspent", 2, "persistent" }, { "send", 0, "outputs" }, { "send", 1, "conf_target" }, { "send", 3, "fee_rate"}, diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 3d3495328091f..2ecdb8d12b6b9 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -456,121 +455,4 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) peerLogic->FinalizeNode(dummyNode); } -class TxOrphanageTest : public TxOrphanage -{ -public: - inline size_t CountOrphans() const EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans) - { - return m_orphans.size(); - } - - CTransactionRef RandomOrphan() EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans) - { - std::map::iterator it; - it = m_orphans.lower_bound(InsecureRand256()); - if (it == m_orphans.end()) - it = m_orphans.begin(); - return it->second.tx; - } -}; - -static void MakeNewKeyWithFastRandomContext(CKey& key) -{ - std::vector keydata; - keydata = g_insecure_rand_ctx.randbytes(32); - key.Set(keydata.data(), keydata.data() + keydata.size(), /*fCompressedIn*/ true); - assert(key.IsValid()); -} - -BOOST_AUTO_TEST_CASE(DoS_mapOrphans) -{ - // This test had non-deterministic coverage due to - // randomly selected seeds. - // This seed is chosen so that all branches of the function - // ecdsa_signature_parse_der_lax are executed during this test. - // Specifically branches that run only when an ECDSA - // signature's R and S values have leading zeros. - g_insecure_rand_ctx = FastRandomContext{uint256{33}}; - - TxOrphanageTest orphanage; - CKey key; - MakeNewKeyWithFastRandomContext(key); - FillableSigningProvider keystore; - BOOST_CHECK(keystore.AddKey(key)); - - LOCK(g_cs_orphans); - - // 50 orphan transactions: - for (int i = 0; i < 50; i++) - { - CMutableTransaction tx; - tx.vin.resize(1); - tx.vin[0].prevout.n = 0; - tx.vin[0].prevout.hash = InsecureRand256(); - tx.vin[0].scriptSig << OP_1; - tx.vout.resize(1); - tx.vout[0].nValue = 1*CENT; - tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey())); - - orphanage.AddTx(MakeTransactionRef(tx), i); - } - - // ... and 50 that depend on other orphans: - for (int i = 0; i < 50; i++) - { - CTransactionRef txPrev = orphanage.RandomOrphan(); - - CMutableTransaction tx; - tx.vin.resize(1); - tx.vin[0].prevout.n = 0; - tx.vin[0].prevout.hash = txPrev->GetHash(); - tx.vout.resize(1); - tx.vout[0].nValue = 1*CENT; - tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey())); - BOOST_CHECK(SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL)); - - orphanage.AddTx(MakeTransactionRef(tx), i); - } - - // This really-big orphan should be ignored: - for (int i = 0; i < 10; i++) - { - CTransactionRef txPrev = orphanage.RandomOrphan(); - - CMutableTransaction tx; - tx.vout.resize(1); - tx.vout[0].nValue = 1*CENT; - tx.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key.GetPubKey())); - tx.vin.resize(2777); - for (unsigned int j = 0; j < tx.vin.size(); j++) - { - tx.vin[j].prevout.n = j; - tx.vin[j].prevout.hash = txPrev->GetHash(); - } - BOOST_CHECK(SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL)); - // Re-use same signature for other inputs - // (they don't have to be valid for this test) - for (unsigned int j = 1; j < tx.vin.size(); j++) - tx.vin[j].scriptSig = tx.vin[0].scriptSig; - - BOOST_CHECK(!orphanage.AddTx(MakeTransactionRef(tx), i)); - } - - // Test EraseOrphansFor: - for (NodeId i = 0; i < 3; i++) - { - size_t sizeBefore = orphanage.CountOrphans(); - orphanage.EraseForPeer(i); - BOOST_CHECK(orphanage.CountOrphans() < sizeBefore); - } - - // Test LimitOrphanTxSize() function: - orphanage.LimitOrphans(40); - BOOST_CHECK(orphanage.CountOrphans() <= 40); - orphanage.LimitOrphans(10); - BOOST_CHECK(orphanage.CountOrphans() <= 10); - orphanage.LimitOrphans(0); - BOOST_CHECK(orphanage.CountOrphans() == 0); -} - BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/orphanage_tests.cpp b/src/test/orphanage_tests.cpp new file mode 100644 index 0000000000000..842daa8bd4d4b --- /dev/null +++ b/src/test/orphanage_tests.cpp @@ -0,0 +1,137 @@ +// Copyright (c) 2011-2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include