diff --git a/.tx/config b/.tx/config index 7699aee5fffe..a9939bf4f9cd 100644 --- a/.tx/config +++ b/.tx/config @@ -1,7 +1,7 @@ [main] host = https://www.transifex.com -[dash.dash_ents] +[o:dash:p:dash:r:dash_ents] file_filter = src/qt/locale/dash_.ts source_file = src/qt/locale/dash_en.xlf source_lang = en diff --git a/src/Makefile.test.include b/src/Makefile.test.include index bed7081dd6d4..6e9ecedffe44 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -222,6 +222,7 @@ BITCOIN_TESTS += wallet/test/db_tests.cpp endif FUZZ_WALLET_SRC = \ + wallet/test/fuzz/coincontrol.cpp \ wallet/test/fuzz/coinselection.cpp if USE_SQLITE diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 8447a5e927da..bcd497bb320b 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -39,7 +39,9 @@ struct MinerTestingSetup : public TestingSetup { bool TestSequenceLocks(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_node.mempool->cs) { CCoinsViewMemPool view_mempool(&m_node.chainman->ActiveChainstate().CoinsTip(), *m_node.mempool); - return CheckSequenceLocksAtTip(m_node.chainman->ActiveChain().Tip(), view_mempool, tx); + CBlockIndex* tip{m_node.chainman->ActiveChain().Tip()}; + const std::optional lock_points{CalculateLockPointsAtTip(tip, view_mempool, tx)}; + return lock_points.has_value() && CheckSequenceLocksAtTip(tip, *lock_points); } BlockAssembler AssemblerForTest(const CChainParams& params); }; diff --git a/src/validation.cpp b/src/validation.cpp index 823c2ca5d354..29ce4c2184eb 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -185,11 +185,89 @@ bool CheckFinalTxAtTip(const CBlockIndex& active_chain_tip, const CTransaction& return IsFinalTx(tx, nBlockHeight, nBlockTime); } +namespace { +/** + * A helper which calculates heights of inputs of a given transaction. + * + * @param[in] tip The current chain tip. If an input belongs to a mempool + * transaction, we assume it will be confirmed in the next block. + * @param[in] coins Any CCoinsView that provides access to the relevant coins. + * @param[in] tx The transaction being evaluated. + * + * @returns A vector of input heights or nullopt, in case of an error. + */ +std::optional> CalculatePrevHeights( + const CBlockIndex& tip, + const CCoinsView& coins, + const CTransaction& tx) +{ + std::vector prev_heights; + prev_heights.resize(tx.vin.size()); + for (size_t i = 0; i < tx.vin.size(); ++i) { + const CTxIn& txin = tx.vin[i]; + Coin coin; + if (!coins.GetCoin(txin.prevout, coin)) { + LogPrintf("ERROR: %s: Missing input %d in transaction \'%s\'\n", __func__, i, tx.GetHash().GetHex()); + return std::nullopt; + } + if (coin.nHeight == MEMPOOL_HEIGHT) { + // Assume all mempool transaction confirm in the next block. + prev_heights[i] = tip.nHeight + 1; + } else { + prev_heights[i] = coin.nHeight; + } + } + return prev_heights; +} +} // namespace + +std::optional CalculateLockPointsAtTip( + CBlockIndex* tip, + const CCoinsView& coins_view, + const CTransaction& tx) +{ + assert(tip); + + auto prev_heights{CalculatePrevHeights(*tip, coins_view, tx)}; + if (!prev_heights.has_value()) return std::nullopt; + + CBlockIndex next_tip; + next_tip.pprev = tip; + // When SequenceLocks() is called within ConnectBlock(), the height + // of the block *being* evaluated is what is used. + // Thus if we want to know if a transaction can be part of the + // *next* block, we need to use one more than active_chainstate.m_chain.Height() + next_tip.nHeight = tip->nHeight + 1; + const auto [min_height, min_time] = CalculateSequenceLocks(tx, STANDARD_LOCKTIME_VERIFY_FLAGS, prev_heights.value(), next_tip); + + // Also store the hash of the block with the highest height of + // all the blocks which have sequence locked prevouts. + // This hash needs to still be on the chain + // for these LockPoint calculations to be valid + // Note: It is impossible to correctly calculate a maxInputBlock + // if any of the sequence locked inputs depend on unconfirmed txs, + // except in the special case where the relative lock time/height + // is 0, which is equivalent to no sequence lock. Since we assume + // input height of tip+1 for mempool txs and test the resulting + // min_height and min_time from CalculateSequenceLocks against tip+1. + int max_input_height{0}; + for (const int height : prev_heights.value()) { + // Can ignore mempool inputs since we'll fail if they had non-zero locks + if (height != next_tip.nHeight) { + max_input_height = std::max(max_input_height, height); + } + } + + // tip->GetAncestor(max_input_height) should never return a nullptr + // because max_input_height is always less than the tip height. + // It would, however, be a bad bug to continue execution, since a + // LockPoints object with the maxInputBlock member set to nullptr + // signifies no relative lock time. + return LockPoints{min_height, min_time, Assert(tip->GetAncestor(max_input_height))}; +} + bool CheckSequenceLocksAtTip(CBlockIndex* tip, - const CCoinsView& coins_view, - const CTransaction& tx, - LockPoints* lp, - bool useExistingLockPoints) + const LockPoints& lock_points) { assert(tip != nullptr); @@ -203,61 +281,7 @@ bool CheckSequenceLocksAtTip(CBlockIndex* tip, // *next* block, we need to use one more than active_chainstate.m_chain.Height() index.nHeight = tip->nHeight + 1; - std::pair lockPair; - if (useExistingLockPoints) { - assert(lp); - lockPair.first = lp->height; - lockPair.second = lp->time; - } - else { - std::vector prevheights; - prevheights.resize(tx.vin.size()); - for (size_t txinIndex = 0; txinIndex < tx.vin.size(); txinIndex++) { - const CTxIn& txin = tx.vin[txinIndex]; - Coin coin; - if (!coins_view.GetCoin(txin.prevout, coin)) { - return error("%s: Missing input", __func__); - } - if (coin.nHeight == MEMPOOL_HEIGHT) { - // Assume all mempool transaction confirm in the next block - prevheights[txinIndex] = tip->nHeight + 1; - } else { - prevheights[txinIndex] = coin.nHeight; - } - } - lockPair = CalculateSequenceLocks(tx, STANDARD_LOCKTIME_VERIFY_FLAGS, prevheights, index); - if (lp) { - lp->height = lockPair.first; - lp->time = lockPair.second; - // Also store the hash of the block with the highest height of - // all the blocks which have sequence locked prevouts. - // This hash needs to still be on the chain - // for these LockPoint calculations to be valid - // Note: It is impossible to correctly calculate a maxInputBlock - // if any of the sequence locked inputs depend on unconfirmed txs, - // except in the special case where the relative lock time/height - // is 0, which is equivalent to no sequence lock. Since we assume - // input height of tip+1 for mempool txs and test the resulting - // lockPair from CalculateSequenceLocks against tip+1. We know - // EvaluateSequenceLocks will fail if there was a non-zero sequence - // lock on a mempool input, so we can use the return value of - // CheckSequenceLocksAtTip to indicate the LockPoints validity - int maxInputHeight = 0; - for (const int height : prevheights) { - // Can ignore mempool inputs since we'll fail if they had non-zero locks - if (height != tip->nHeight+1) { - maxInputHeight = std::max(maxInputHeight, height); - } - } - // tip->GetAncestor(maxInputHeight) should never return a nullptr - // because maxInputHeight is always less than the tip height. - // It would, however, be a bad bug to continue execution, since a - // LockPoints object with the maxInputBlock member set to nullptr - // signifies no relative lock time. - lp->maxInputBlock = Assert(tip->GetAncestor(maxInputHeight)); - } - } - return EvaluateSequenceLocks(index, lockPair); + return EvaluateSequenceLocks(index, {lock_points.height, lock_points.time}); } bool GetUTXOCoin(CChainState& active_chainstate, const COutPoint& outpoint, Coin& coin) @@ -405,20 +429,23 @@ void CChainState::MaybeUpdateMempoolForReorg( // The transaction must be final. if (!CheckFinalTxAtTip(*Assert(m_chain.Tip()), tx)) return true; - LockPoints lp = it->GetLockPoints(); - const bool validLP{TestLockPointValidity(m_chain, lp)}; - CCoinsViewMemPool view_mempool(&CoinsTip(), *m_mempool); + + const LockPoints& lp = it->GetLockPoints(); // CheckSequenceLocksAtTip checks if the transaction will be final in the next block to be - // created on top of the new chain. We use useExistingLockPoints=false so that, instead of - // using the information in lp (which might now refer to a block that no longer exists in - // the chain), it will update lp to contain LockPoints relevant to the new chain. - if (!CheckSequenceLocksAtTip(m_chain.Tip(), view_mempool, tx, &lp, validLP)) { - // If CheckSequenceLocksAtTip fails, remove the tx and don't depend on the LockPoints. - return true; - } else if (!validLP) { - // If CheckSequenceLocksAtTip succeeded, it also updated the LockPoints. - // Now update the mempool entry lockpoints as well. - m_mempool->mapTx.modify(it, [&lp](CTxMemPoolEntry& e) { e.UpdateLockPoints(lp); }); + // created on top of the new chain. + if (TestLockPointValidity(m_chain, lp)) { + if (!CheckSequenceLocksAtTip(m_chain.Tip(), lp)) { + return true; + } + } else { + const CCoinsViewMemPool view_mempool{&CoinsTip(), *m_mempool}; + const std::optional new_lock_points{CalculateLockPointsAtTip(m_chain.Tip(), view_mempool, tx)}; + if (new_lock_points.has_value() && CheckSequenceLocksAtTip(m_chain.Tip(), *new_lock_points)) { + // Now update the mempool entry lockpoints as well. + m_mempool->mapTx.modify(it, [&new_lock_points](CTxMemPoolEntry& e) { e.UpdateLockPoints(*new_lock_points); }); + } else { + return true; + } } // If the transaction spends any coinbase outputs, it must be mature. @@ -781,7 +808,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } } } - LockPoints lp; m_view.SetBackend(m_viewmempool); const CCoinsViewCache& coins_cache = m_active_chainstate.CoinsTip(); @@ -823,7 +849,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // be mined yet. // Pass in m_view which has all of the relevant inputs cached. Note that, since m_view's // backend was removed, it no longer pulls coins from the mempool. - if (!CheckSequenceLocksAtTip(m_active_chainstate.m_chain.Tip(), m_view, tx, &lp)) { + const std::optional lock_points{CalculateLockPointsAtTip(m_active_chainstate.m_chain.Tip(), m_view, tx)}; + if (!lock_points.has_value() || !CheckSequenceLocksAtTip(m_active_chainstate.m_chain.Tip(), *lock_points)) { return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "non-BIP68-final"); } @@ -854,7 +881,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } entry.reset(new CTxMemPoolEntry(ptx, ws.m_base_fees, nAcceptTime, m_active_chainstate.m_chain.Height(), - fSpendsCoinbase, nSigOps, lp)); + fSpendsCoinbase, nSigOps, lock_points.value())); ws.m_vsize = entry->GetTxSize(); if (nSigOps > MAX_STANDARD_TX_SIGOPS) @@ -1930,11 +1957,21 @@ DisconnectResult CChainState::DisconnectBlock(const CBlock& block, const CBlockI return DISCONNECT_FAILED; } + // Ignore blocks that contain transactions which are 'overwritten' by later transactions, + // unless those are already completely spent. + // See https://github.com/bitcoin/bitcoin/issues/22596 for additional information. + // Note: the blocks specified here are different than the ones used in ConnectBlock because DisconnectBlock + // unwinds the blocks in reverse. As a result, the inconsistency is not discovered until the earlier + // blocks with the duplicate coinbase transactions are disconnected. + bool fEnforceBIP30 = !((pindex->nHeight==91722 && pindex->GetBlockHash() == uint256S("0x00000000000271a2dc26e7667f8419f2e15416dc6955e5a6c6cdf3f2574dd08e")) || + (pindex->nHeight==91812 && pindex->GetBlockHash() == uint256S("0x00000000000af0aed4792b1acee3d966af36cf5def14935db8de83d6f9306f2f"))); + // undo transactions in reverse order for (int i = block.vtx.size() - 1; i >= 0; i--) { const CTransaction &tx = *(block.vtx[i]); uint256 hash = tx.GetHash(); bool is_coinbase = tx.IsCoinBase(); + bool is_bip30_exception = (is_coinbase && !fEnforceBIP30); if (fAddressIndex) { for (unsigned int k = tx.vout.size(); k-- > 0;) { @@ -1963,7 +2000,9 @@ DisconnectResult CChainState::DisconnectBlock(const CBlock& block, const CBlockI Coin coin; bool is_spent = view.SpendCoin(out, &coin); if (!is_spent || tx.vout[o] != coin.out || pindex->nHeight != coin.nHeight || is_coinbase != coin.fCoinBase) { - fClean = false; // transaction output mismatch + if (!is_bip30_exception) { + fClean = false; // transaction output mismatch + } } } } diff --git a/src/validation.h b/src/validation.h index 814a75d842aa..6eb7608491f3 100644 --- a/src/validation.h +++ b/src/validation.h @@ -281,27 +281,39 @@ int GetUTXOConfirmations(CChainState& active_chainstate, const COutPoint& outpoi bool CheckFinalTxAtTip(const CBlockIndex& active_chain_tip, const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** - * Check if transaction will be BIP68 final in the next block to be created on top of tip. - * @param[in] tip Chain tip to check tx sequence locks against. For example, - * the tip of the current active chain. + * Calculate LockPoints required to check if transaction will be BIP68 final in the next block + * to be created on top of tip. + * + * @param[in] tip Chain tip for which tx sequence locks are calculated. For + * example, the tip of the current active chain. * @param[in] coins_view Any CCoinsView that provides access to the relevant coins for * checking sequence locks. For example, it can be a CCoinsViewCache * that isn't connected to anything but contains all the relevant * coins, or a CCoinsViewMemPool that is connected to the - * mempool and chainstate UTXO set. In the latter case, the caller is - * responsible for holding the appropriate locks to ensure that + * mempool and chainstate UTXO set. In the latter case, the caller + * is responsible for holding the appropriate locks to ensure that * calls to GetCoin() return correct coins. + * @param[in] tx The transaction being evaluated. + * + * @returns The resulting height and time calculated and the hash of the block needed for + * calculation, or std::nullopt if there is an error. + */ +std::optional CalculateLockPointsAtTip( + CBlockIndex* tip, + const CCoinsView& coins_view, + const CTransaction& tx); + +/** + * Check if transaction will be BIP68 final in the next block to be created on top of tip. + * @param[in] tip Chain tip to check tx sequence locks against. For example, + * the tip of the current active chain. + * @param[in] lock_points LockPoints containing the height and time at which this + * transaction is final. * Simulates calling SequenceLocks() with data from the tip passed in. - * Optionally stores in LockPoints the resulting height and time calculated and the hash - * of the block needed for calculation or skips the calculation and uses the LockPoints - * passed in for evaluation. * The LockPoints should not be considered valid if CheckSequenceLocksAtTip returns false. */ bool CheckSequenceLocksAtTip(CBlockIndex* tip, - const CCoinsView& coins_view, - const CTransaction& tx, - LockPoints* lp = nullptr, - bool useExistingLockPoints = false); + const LockPoints& lock_points); /** * Closure representing one script verification diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 71ed77e58de4..5aa4e1f65e78 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -778,7 +778,7 @@ static std::optional CreateTransactionInternal( } if (feeCalc.reason == FeeReason::FALLBACK && !wallet.m_allow_fallback_fee) { // eventually allow a fallback fee - error = _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee."); + error = strprintf(_("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable %s."), "-fallbackfee"); return std::nullopt; } diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index fc0fe8ff9e67..2e31688712c2 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -23,9 +23,6 @@ namespace wallet { static constexpr int32_t WALLET_SCHEMA_VERSION = 0; -static Mutex g_sqlite_mutex; -static int g_sqlite_count GUARDED_BY(g_sqlite_mutex) = 0; - static Span SpanFromBlob(sqlite3_stmt* stmt, int col) { return {reinterpret_cast(sqlite3_column_blob(stmt, col)), @@ -104,6 +101,9 @@ static void SetPragma(sqlite3* db, const std::string& key, const std::string& va } } +Mutex SQLiteDatabase::g_sqlite_mutex; +int SQLiteDatabase::g_sqlite_count = 0; + SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, const DatabaseOptions& options, bool mock) : WalletDatabase(), m_mock(mock), m_dir_path(fs::PathToString(dir_path)), m_file_path(fs::PathToString(file_path)), m_use_unsafe_sync(options.use_unsafe_sync) { @@ -167,6 +167,8 @@ SQLiteDatabase::~SQLiteDatabase() void SQLiteDatabase::Cleanup() noexcept { + AssertLockNotHeld(g_sqlite_mutex); + Close(); LOCK(g_sqlite_mutex); diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h index 47b7ebb0ecde..d7135679ff8a 100644 --- a/src/wallet/sqlite.h +++ b/src/wallet/sqlite.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_WALLET_SQLITE_H #define BITCOIN_WALLET_SQLITE_H +#include #include #include @@ -63,7 +64,16 @@ class SQLiteDatabase : public WalletDatabase const std::string m_file_path; - void Cleanup() noexcept; + /** + * This mutex protects SQLite initialization and shutdown. + * sqlite3_config() and sqlite3_shutdown() are not thread-safe (sqlite3_initialize() is). + * Concurrent threads that execute SQLiteDatabase::SQLiteDatabase() should have just one + * of them do the init and the rest wait for it to complete before all can proceed. + */ + static Mutex g_sqlite_mutex; + static int g_sqlite_count GUARDED_BY(g_sqlite_mutex); + + void Cleanup() noexcept EXCLUSIVE_LOCKS_REQUIRED(!g_sqlite_mutex); public: SQLiteDatabase() = delete; diff --git a/src/wallet/test/fuzz/coincontrol.cpp b/src/wallet/test/fuzz/coincontrol.cpp new file mode 100644 index 000000000000..4a47b33f71c9 --- /dev/null +++ b/src/wallet/test/fuzz/coincontrol.cpp @@ -0,0 +1,89 @@ +// Copyright (c) 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 +#include +#include +#include + +namespace wallet { +namespace { + +const TestingSetup* g_setup; + +void initialize_coincontrol() +{ + static const auto testing_setup = MakeNoLogFileContext(); + g_setup = testing_setup.get(); +} + +FUZZ_TARGET(coincontrol, .init = initialize_coincontrol) +{ + FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); + const auto& node = g_setup->m_node; + ArgsManager& args = *node.args; + + // for GetBoolArg to return true sometimes + args.ForceSetArg("-avoidpartialspends", fuzzed_data_provider.ConsumeBool()?"1":"0"); + + CCoinControl coin_control; + COutPoint out_point; + + LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) + { + CallOneOf( + fuzzed_data_provider, + [&] { + std::optional optional_out_point = ConsumeDeserializable(fuzzed_data_provider); + if (!optional_out_point) { + return; + } + out_point = *optional_out_point; + }, + [&] { + (void)coin_control.HasSelected(); + }, + [&] { + (void)coin_control.IsSelected(out_point); + }, + [&] { + (void)coin_control.IsExternalSelected(out_point); + }, + [&] { + CTxOut txout; + (void)coin_control.GetExternalOutput(out_point, txout); + }, + [&] { + (void)coin_control.Select(out_point); + }, + [&] { + const CTxOut tx_out{ConsumeMoney(fuzzed_data_provider), ConsumeScript(fuzzed_data_provider)}; + (void)coin_control.SelectExternal(out_point, tx_out); + }, + [&] { + (void)coin_control.UnSelect(out_point); + }, + [&] { + (void)coin_control.UnSelectAll(); + }, + [&] { + std::vector selected; + coin_control.ListSelected(selected); + }, + [&] { + int64_t weight{fuzzed_data_provider.ConsumeIntegral()}; + (void)coin_control.SetInputWeight(out_point, weight); + }, + [&] { + // Condition to avoid the assertion in GetInputWeight + if (coin_control.HasInputWeight(out_point)) { + (void)coin_control.GetInputWeight(out_point); + } + }); + } +} +} // namespace +} // namespace wallet diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6979c5d49fae..78efc16f35e8 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2971,7 +2971,7 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri if (args.IsArgSet("-fallbackfee")) { std::optional fallback_fee = ParseMoney(args.GetArg("-fallbackfee", "")); if (!fallback_fee) { - error = strprintf(_("Invalid amount for -fallbackfee=: '%s'"), args.GetArg("-fallbackfee", "")); + error = strprintf(_("Invalid amount for %s=: '%s'"), "-fallbackfee", args.GetArg("-fallbackfee", "")); return nullptr; } else if (fallback_fee.value() > HIGH_TX_FEE_PER_KB) { warnings.push_back(AmountHighWarn("-fallbackfee") + Untranslated(" ") + @@ -2985,7 +2985,7 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri if (args.IsArgSet("-discardfee")) { std::optional discard_fee = ParseMoney(args.GetArg("-discardfee", "")); if (!discard_fee) { - error = strprintf(_("Invalid amount for -discardfee=: '%s'"), args.GetArg("-discardfee", "")); + error = strprintf(_("Invalid amount for %s=: '%s'"), "-discardfee", args.GetArg("-discardfee", "")); return nullptr; } else if (discard_fee.value() > HIGH_TX_FEE_PER_KB) { warnings.push_back(AmountHighWarn("-discardfee") + Untranslated(" ") + @@ -3005,8 +3005,8 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri } walletInstance->m_pay_tx_fee = CFeeRate{pay_tx_fee.value(), 1000}; if (chain && walletInstance->m_pay_tx_fee < chain->relayMinFee()) { - error = strprintf(_("Invalid amount for -paytxfee=: '%s' (must be at least %s)"), - args.GetArg("-paytxfee", ""), chain->relayMinFee().ToString()); + error = strprintf(_("Invalid amount for %s=: '%s' (must be at least %s)"), + "-paytxfee", args.GetArg("-paytxfee", ""), chain->relayMinFee().ToString()); return nullptr; } } @@ -3017,11 +3017,11 @@ std::shared_ptr CWallet::Create(WalletContext& context, const std::stri error = AmountErrMsg("maxtxfee", args.GetArg("-maxtxfee", "")); return nullptr; } else if (max_fee.value() > HIGH_MAX_TX_FEE) { - warnings.push_back(_("-maxtxfee is set very high! Fees this large could be paid on a single transaction.")); + warnings.push_back(strprintf(_("%s is set very high! Fees this large could be paid on a single transaction."), "-maxtxfee")); } if (chain && CFeeRate{max_fee.value(), 1000} < chain->relayMinFee()) { - error = strprintf(_("Invalid amount for -maxtxfee=: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)"), - args.GetArg("-maxtxfee", ""), chain->relayMinFee().ToString()); + error = strprintf(_("Invalid amount for %s=: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)"), + "-maxtxfee", args.GetArg("-maxtxfee", ""), chain->relayMinFee().ToString()); return nullptr; } diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py index daf634b16125..a9720eea30c3 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -179,7 +179,7 @@ def assert_submitblock(block, result_str_1, result_str_2=None): self.log.info("getblocktemplate: Test bad timestamps") bad_block = copy.deepcopy(block) - bad_block.nTime = 2**31 - 1 + bad_block.nTime = 2**32 - 1 assert_template(node, bad_block, 'time-too-new') assert_submitblock(bad_block, 'time-too-new', 'time-too-new') bad_block.nTime = 0 diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index b933d359f58d..d18c366d0476 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -92,6 +92,7 @@ def run_test(self): self._test_stopatheight() self._test_waitforblockheight() self._test_getblock() + self._test_y2106() assert self.nodes[0].verifychain(4, 0) def mine_chain(self): @@ -247,6 +248,14 @@ def _test_getblockchaininfo(self): 'active': False}, }) + def _test_y2106(self): + self.log.info("Check that block timestamps work until year 2106") + self.generate(self.nodes[0], 8)[-1] + time_2106 = 2**32 - 1 + self.nodes[0].setmocktime(time_2106) + last = self.generate(self.nodes[0], 6)[-1] + assert_equal(self.nodes[0].getblockheader(last)["mediantime"], time_2106) + def _test_getchaintxstats(self): self.log.info("Test getchaintxstats") diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 8b325b480bd4..452b3929f5bd 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -148,8 +148,6 @@ def __init__(self): print("DEPRECATED: --timeoutscale option is no longer available, please use --timeout-factor instead") if self.options.timeout_factor == 1: self.options.timeout_factor = self.options.timeout_scale - if self.options.timeout_factor == 0 : - self.options.timeout_factor = 99999 self.rpc_timeout = int(self.rpc_timeout * self.options.timeout_factor) # optionally, increase timeout by a factor def main(self): @@ -223,7 +221,7 @@ def parse_args(self): help="run nodes under the valgrind memory error detector: expect at least a ~10x slowdown, valgrind 3.14 or later required. Does not apply to previous release binaries.") parser.add_argument("--randomseed", type=int, help="set a random seed for deterministically reproducing a previous test run") - parser.add_argument('--timeout-factor', dest="timeout_factor", type=float, default=1.0, help='adjust test timeouts by a factor. Setting it to 0 disables all timeouts') + parser.add_argument("--timeout-factor", dest="timeout_factor", type=float, help="adjust test timeouts by a factor. Setting it to 0 disables all timeouts") parser.add_argument("--v2transport", dest="v2transport", default=False, action="store_true", help="use BIP324 v2 connections between all nodes by default") parser.add_argument("--v1transport", dest="v1transport", default=False, action="store_true", @@ -237,6 +235,9 @@ def parse_args(self): self.add_options(parser) self.options = parser.parse_args() + if self.options.timeout_factor == 0: + self.options.timeout_factor = 99999 + self.options.timeout_factor = self.options.timeout_factor or (4 if self.options.valgrind else 1) self.options.previous_releases_path = previous_releases_path config = configparser.ConfigParser()