diff options
Diffstat (limited to 'src/validation.cpp')
| -rw-r--r-- | src/validation.cpp | 540 |
1 files changed, 363 insertions, 177 deletions
diff --git a/src/validation.cpp b/src/validation.cpp index d398ec7406..3e9ba08bb1 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -27,14 +27,15 @@ #include <kernel/mempool_entry.h> #include <kernel/messagestartchars.h> #include <kernel/notifications_interface.h> +#include <kernel/warning.h> #include <logging.h> #include <logging/timer.h> #include <node/blockstorage.h> #include <node/utxo_snapshot.h> -#include <policy/v3_policy.h> #include <policy/policy.h> #include <policy/rbf.h> #include <policy/settings.h> +#include <policy/v3_policy.h> #include <pow.h> #include <primitives/block.h> #include <primitives/transaction.h> @@ -57,11 +58,11 @@ #include <util/result.h> #include <util/signalinterrupt.h> #include <util/strencodings.h> +#include <util/string.h> #include <util/time.h> #include <util/trace.h> #include <util/translation.h> #include <validationinterface.h> -#include <warnings.h> #include <algorithm> #include <cassert> @@ -455,11 +456,14 @@ public: * the mempool. */ std::vector<COutPoint>& m_coins_to_uncache; + /** When true, the transaction or package will not be submitted to the mempool. */ const bool m_test_accept; - /** Whether we allow transactions to replace mempool transactions by BIP125 rules. If false, + /** Whether we allow transactions to replace mempool transactions. If false, * any transaction spending the same inputs as a transaction in the mempool is considered * a conflict. */ const bool m_allow_replacement; + /** When true, allow sibling eviction. This only occurs in single transaction package settings. */ + const bool m_allow_sibling_eviction; /** When true, the mempool will not be trimmed when any transactions are submitted in * Finalize(). Instead, limits should be enforced at the end to ensure the package is not * partially submitted. @@ -475,6 +479,9 @@ public: */ const std::optional<CFeeRate> m_client_maxfeerate; + /** Whether CPFP carveout and RBF carveout are granted. */ + const bool m_allow_carveouts; + /** Parameters for single transaction mempool validation. */ static ATMPArgs SingleAccept(const CChainParams& chainparams, int64_t accept_time, bool bypass_limits, std::vector<COutPoint>& coins_to_uncache, @@ -485,9 +492,11 @@ public: /* m_coins_to_uncache */ coins_to_uncache, /* m_test_accept */ test_accept, /* m_allow_replacement */ true, + /* m_allow_sibling_eviction */ true, /* m_package_submission */ false, /* m_package_feerates */ false, /* m_client_maxfeerate */ {}, // checked by caller + /* m_allow_carveouts */ true, }; } @@ -500,9 +509,11 @@ public: /* m_coins_to_uncache */ coins_to_uncache, /* m_test_accept */ true, /* m_allow_replacement */ false, + /* m_allow_sibling_eviction */ false, /* m_package_submission */ false, // not submitting to mempool /* m_package_feerates */ false, /* m_client_maxfeerate */ {}, // checked by caller + /* m_allow_carveouts */ false, }; } @@ -514,10 +525,12 @@ public: /* m_bypass_limits */ false, /* m_coins_to_uncache */ coins_to_uncache, /* m_test_accept */ false, - /* m_allow_replacement */ false, + /* m_allow_replacement */ true, + /* m_allow_sibling_eviction */ false, /* m_package_submission */ true, /* m_package_feerates */ true, /* m_client_maxfeerate */ client_maxfeerate, + /* m_allow_carveouts */ false, }; } @@ -529,9 +542,11 @@ public: /* m_coins_to_uncache */ package_args.m_coins_to_uncache, /* m_test_accept */ package_args.m_test_accept, /* m_allow_replacement */ true, + /* m_allow_sibling_eviction */ true, /* m_package_submission */ true, // do not LimitMempoolSize in Finalize() /* m_package_feerates */ false, // only 1 transaction /* m_client_maxfeerate */ package_args.m_client_maxfeerate, + /* m_allow_carveouts */ false, }; } @@ -544,19 +559,31 @@ public: std::vector<COutPoint>& coins_to_uncache, bool test_accept, bool allow_replacement, + bool allow_sibling_eviction, bool package_submission, bool package_feerates, - std::optional<CFeeRate> client_maxfeerate) + std::optional<CFeeRate> client_maxfeerate, + bool allow_carveouts) : m_chainparams{chainparams}, m_accept_time{accept_time}, m_bypass_limits{bypass_limits}, m_coins_to_uncache{coins_to_uncache}, m_test_accept{test_accept}, m_allow_replacement{allow_replacement}, + m_allow_sibling_eviction{allow_sibling_eviction}, m_package_submission{package_submission}, m_package_feerates{package_feerates}, - m_client_maxfeerate{client_maxfeerate} + m_client_maxfeerate{client_maxfeerate}, + m_allow_carveouts{allow_carveouts} { + // If we are using package feerates, we must be doing package submission. + // It also means carveouts and sibling eviction are not permitted. + if (m_package_feerates) { + Assume(m_package_submission); + Assume(!m_allow_carveouts); + Assume(!m_allow_sibling_eviction); + } + if (m_allow_sibling_eviction) Assume(m_allow_replacement); } }; @@ -576,8 +603,8 @@ public: /** * Submission of a subpackage. * If subpackage size == 1, calls AcceptSingleTransaction() with adjusted ATMPArgs to avoid - * package policy restrictions like no CPFP carve out (PackageMempoolChecks) and disabled RBF - * (m_allow_replacement), and creates a PackageMempoolAcceptResult wrapping the result. + * package policy restrictions like no CPFP carve out (PackageMempoolChecks) + * and creates a PackageMempoolAcceptResult wrapping the result. * * If subpackage size > 1, calls AcceptMultipleTransactions() with the provided ATMPArgs. * @@ -603,18 +630,11 @@ private: /** Iterators to mempool entries that this transaction directly conflicts with or may * replace via sibling eviction. */ CTxMemPool::setEntries m_iters_conflicting; - /** Iterators to all mempool entries that would be replaced by this transaction, including - * m_conflicts and their descendants. */ - CTxMemPool::setEntries m_all_conflicting; /** All mempool ancestors of this transaction. */ CTxMemPool::setEntries m_ancestors; /** Mempool entry constructed for this transaction. Constructed in PreChecks() but not * inserted into the mempool until Finalize(). */ std::unique_ptr<CTxMemPoolEntry> m_entry; - /** Pointers to the transactions that have been removed from the mempool and replaced by - * this transaction (everything in m_all_conflicting), used to return to the MemPoolAccept caller. Only populated if - * validation is successful and the original transactions are removed. */ - std::list<CTransactionRef> m_replaced_transactions; /** Whether RBF-related data structures (m_conflicts, m_iters_conflicting, m_all_conflicting, * m_replaced_transactions) include a sibling in addition to txns with conflicting inputs. */ bool m_sibling_eviction{false}; @@ -626,10 +646,6 @@ private: CAmount m_base_fees; /** Base fees + any fee delta set by the user with prioritisetransaction. */ CAmount m_modified_fees; - /** Total modified fees of all transactions being replaced. */ - CAmount m_conflicting_fees{0}; - /** Total virtual size of all transactions being replaced. */ - size_t m_conflicting_size{0}; /** If we're doing package validation (i.e. m_package_feerates=true), the "effective" * package feerate of this transaction is the total fees divided by the total size of @@ -651,12 +667,13 @@ private: // only tests that are fast should be done here (to avoid CPU DoS). bool PreChecks(ATMPArgs& args, Workspace& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); - // Run checks for mempool replace-by-fee. + // Run checks for mempool replace-by-fee, only used in AcceptSingleTransaction. bool ReplacementChecks(Workspace& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); // Enforce package mempool ancestor/descendant limits (distinct from individual - // ancestor/descendant limits done in PreChecks). + // ancestor/descendant limits done in PreChecks) and run Package RBF checks. bool PackageMempoolChecks(const std::vector<CTransactionRef>& txns, + std::vector<Workspace>& workspaces, int64_t total_vsize, PackageValidationState& package_state) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); @@ -707,9 +724,39 @@ private: Chainstate& m_active_chainstate; - /** Whether the transaction(s) would replace any mempool transactions and/or evict any siblings. - * If so, RBF rules apply. */ - bool m_rbf{false}; + // Fields below are per *sub*package state and must be reset prior to subsequent + // AcceptSingleTransaction and AcceptMultipleTransactions invocations + struct SubPackageState { + /** Aggregated modified fees of all transactions, used to calculate package feerate. */ + CAmount m_total_modified_fees{0}; + /** Aggregated virtual size of all transactions, used to calculate package feerate. */ + int64_t m_total_vsize{0}; + + // RBF-related members + /** Whether the transaction(s) would replace any mempool transactions and/or evict any siblings. + * If so, RBF rules apply. */ + bool m_rbf{false}; + /** All directly conflicting mempool transactions and their descendants. */ + CTxMemPool::setEntries m_all_conflicts; + /** Mempool transactions that were replaced. */ + std::list<CTransactionRef> m_replaced_transactions; + + /** Total modified fees of mempool transactions being replaced. */ + CAmount m_conflicting_fees{0}; + /** Total size (in virtual bytes) of mempool transactions being replaced. */ + size_t m_conflicting_size{0}; + }; + + struct SubPackageState m_subpackage; + + /** Re-set sub-package state to not leak between evaluations */ + void ClearSubPackageState() EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs) + { + m_subpackage = SubPackageState{}; + + // And clean coins while at it + CleanupTemporaryCoins(); + } }; bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) @@ -786,7 +833,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // // Replaceability signaling of the original transactions may be // ignored due to node setting. - const bool allow_rbf{m_pool.m_opts.full_rbf || SignalsOptInRBF(*ptxConflicting) || ptxConflicting->nVersion == 3}; + const bool allow_rbf{m_pool.m_opts.full_rbf || SignalsOptInRBF(*ptxConflicting) || ptxConflicting->version == TRUC_VERSION}; if (!allow_rbf) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "txn-mempool-conflict"); } @@ -890,7 +937,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // method of ensuring the tx remains bumped. For example, the fee-bumping child could disappear // due to a replacement. // The only exception is v3 transactions. - if (!bypass_limits && ws.m_ptx->nVersion != 3 && ws.m_modified_fees < m_pool.m_opts.min_relay_feerate.GetFee(ws.m_vsize)) { + if (!bypass_limits && ws.m_ptx->version != TRUC_VERSION && ws.m_modified_fees < m_pool.m_opts.min_relay_feerate.GetFee(ws.m_vsize)) { // Even though this is a fee-related failure, this result is TX_MEMPOOL_POLICY, not // TX_RECONSIDERABLE, because it cannot be bypassed using package validation. return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "min relay fee not met", @@ -904,11 +951,11 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) ws.m_iters_conflicting = m_pool.GetIterSet(ws.m_conflicts); // Note that these modifications are only applicable to single transaction scenarios; - // carve-outs and package RBF are disabled for multi-transaction evaluations. + // carve-outs are disabled for multi-transaction evaluations. CTxMemPool::Limits maybe_rbf_limits = m_pool.m_opts.limits; // Calculate in-mempool ancestors, up to a limit. - if (ws.m_conflicts.size() == 1) { + if (ws.m_conflicts.size() == 1 && args.m_allow_carveouts) { // In general, when we receive an RBF transaction with mempool conflicts, we want to know whether we // would meet the chain limits after the conflicts have been removed. However, there isn't a practical // way to do this short of calculating the ancestor and descendant sets with an overlay cache of @@ -947,11 +994,19 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) ws.m_ancestors = std::move(*ancestors); } else { // If CalculateMemPoolAncestors fails second time, we want the original error string. + const auto error_message{util::ErrorString(ancestors).original}; + + // Carve-out is not allowed in this context; fail + if (!args.m_allow_carveouts) { + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message); + } + // Contracting/payment channels CPFP carve-out: // If the new transaction is relatively small (up to 40k weight) // and has at most one ancestor (ie ancestor limit of 2, including // the new transaction), allow it if its parent has exactly the - // descendant limit descendants. + // descendant limit descendants. The transaction also cannot be v3, + // as its topology restrictions do not allow a second child. // // This allows protocols which rely on distrusting counterparties // being able to broadcast descendants of an unconfirmed transaction @@ -964,8 +1019,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) .descendant_count = maybe_rbf_limits.descendant_count + 1, .descendant_size_vbytes = maybe_rbf_limits.descendant_size_vbytes + EXTRA_DESCENDANT_TX_SIZE_LIMIT, }; - const auto error_message{util::ErrorString(ancestors).original}; - if (ws.m_vsize > EXTRA_DESCENDANT_TX_SIZE_LIMIT) { + if (ws.m_vsize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || ws.m_ptx->version == TRUC_VERSION) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message); } if (auto ancestors_retry{m_pool.CalculateMemPoolAncestors(*entry, cpfp_carve_out_limits)}) { @@ -979,8 +1033,11 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // check using the full ancestor set here because it's more convenient to use what we have // already calculated. if (const auto err{SingleV3Checks(ws.m_ptx, ws.m_ancestors, ws.m_conflicts, ws.m_vsize)}) { - // Disabled within package validation. - if (err->second != nullptr && args.m_allow_replacement) { + // Single transaction contexts only. + if (args.m_allow_sibling_eviction && err->second != nullptr) { + // We should only be considering where replacement is considered valid as well. + Assume(args.m_allow_replacement); + // Potential sibling eviction. Add the sibling to our list of mempool conflicts to be // included in RBF checks. ws.m_conflicts.insert(err->second->GetHash()); @@ -1008,7 +1065,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-spends-conflicting-tx", *err_string); } - m_rbf = !ws.m_conflicts.empty(); + // We want to detect conflicts in any tx in a package to trigger package RBF logic + m_subpackage.m_rbf |= !ws.m_conflicts.empty(); return true; } @@ -1032,43 +1090,42 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws) // descendant transaction of a direct conflict to pay a higher feerate than the transaction that // might replace them, under these rules. if (const auto err_string{PaysMoreThanConflicts(ws.m_iters_conflicting, newFeeRate, hash)}) { - // Even though this is a fee-related failure, this result is TX_MEMPOOL_POLICY, not - // TX_RECONSIDERABLE, because it cannot be bypassed using package validation. - // This must be changed if package RBF is enabled. - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, + // This fee-related failure is TX_RECONSIDERABLE because validating in a package may change + // the result. + return state.Invalid(TxValidationResult::TX_RECONSIDERABLE, strprintf("insufficient fee%s", ws.m_sibling_eviction ? " (including sibling eviction)" : ""), *err_string); } // Calculate all conflicting entries and enforce Rule #5. - if (const auto err_string{GetEntriesForConflicts(tx, m_pool, ws.m_iters_conflicting, ws.m_all_conflicting)}) { + if (const auto err_string{GetEntriesForConflicts(tx, m_pool, ws.m_iters_conflicting, m_subpackage.m_all_conflicts)}) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, strprintf("too many potential replacements%s", ws.m_sibling_eviction ? " (including sibling eviction)" : ""), *err_string); } // Enforce Rule #2. - if (const auto err_string{HasNoNewUnconfirmed(tx, m_pool, ws.m_iters_conflicting)}) { + if (const auto err_string{HasNoNewUnconfirmed(tx, m_pool, m_subpackage.m_all_conflicts)}) { // Sibling eviction is only done for v3 transactions, which cannot have multiple ancestors. Assume(!ws.m_sibling_eviction); return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, strprintf("replacement-adds-unconfirmed%s", ws.m_sibling_eviction ? " (including sibling eviction)" : ""), *err_string); } + // Check if it's economically rational to mine this transaction rather than the ones it // replaces and pays for its own relay fees. Enforce Rules #3 and #4. - for (CTxMemPool::txiter it : ws.m_all_conflicting) { - ws.m_conflicting_fees += it->GetModifiedFee(); - ws.m_conflicting_size += it->GetTxSize(); + for (CTxMemPool::txiter it : m_subpackage.m_all_conflicts) { + m_subpackage.m_conflicting_fees += it->GetModifiedFee(); + m_subpackage.m_conflicting_size += it->GetTxSize(); } - if (const auto err_string{PaysForRBF(ws.m_conflicting_fees, ws.m_modified_fees, ws.m_vsize, + if (const auto err_string{PaysForRBF(m_subpackage.m_conflicting_fees, ws.m_modified_fees, ws.m_vsize, m_pool.m_opts.incremental_relay_feerate, hash)}) { - // Even though this is a fee-related failure, this result is TX_MEMPOOL_POLICY, not - // TX_RECONSIDERABLE, because it cannot be bypassed using package validation. - // This must be changed if package RBF is enabled. - return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, + // Result may change in a package context + return state.Invalid(TxValidationResult::TX_RECONSIDERABLE, strprintf("insufficient fee%s", ws.m_sibling_eviction ? " (including sibling eviction)" : ""), *err_string); } return true; } bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txns, + std::vector<Workspace>& workspaces, const int64_t total_vsize, PackageValidationState& package_state) { @@ -1079,12 +1136,88 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txn assert(std::all_of(txns.cbegin(), txns.cend(), [this](const auto& tx) { return !m_pool.exists(GenTxid::Txid(tx->GetHash()));})); + assert(txns.size() == workspaces.size()); + auto result = m_pool.CheckPackageLimits(txns, total_vsize); if (!result) { // This is a package-wide error, separate from an individual transaction error. return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-mempool-limits", util::ErrorString(result).original); } - return true; + + // No conflicts means we're finished. Further checks are all RBF-only. + if (!m_subpackage.m_rbf) return true; + + // We're in package RBF context; replacement proposal must be size 2 + if (workspaces.size() != 2 || !Assume(IsChildWithParents(txns))) { + return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: package must be 1-parent-1-child"); + } + + // If the package has in-mempool ancestors, we won't consider a package RBF + // since it would result in a cluster larger than 2. + // N.B. To relax this constraint we will need to revisit how CCoinsViewMemPool::PackageAddTransaction + // is being used inside AcceptMultipleTransactions to track available inputs while processing a package. + for (const auto& ws : workspaces) { + if (!ws.m_ancestors.empty()) { + return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: new transaction cannot have mempool ancestors"); + } + } + + // Aggregate all conflicts into one set. + CTxMemPool::setEntries direct_conflict_iters; + for (Workspace& ws : workspaces) { + // Aggregate all conflicts into one set. + direct_conflict_iters.merge(ws.m_iters_conflicting); + } + + const auto& parent_ws = workspaces[0]; + const auto& child_ws = workspaces[1]; + + // Don't consider replacements that would cause us to remove a large number of mempool entries. + // This limit is not increased in a package RBF. Use the aggregate number of transactions. + if (const auto err_string{GetEntriesForConflicts(*child_ws.m_ptx, m_pool, direct_conflict_iters, + m_subpackage.m_all_conflicts)}) { + return package_state.Invalid(PackageValidationResult::PCKG_POLICY, + "package RBF failed: too many potential replacements", *err_string); + } + + for (CTxMemPool::txiter it : m_subpackage.m_all_conflicts) { + m_subpackage.m_conflicting_fees += it->GetModifiedFee(); + m_subpackage.m_conflicting_size += it->GetTxSize(); + } + + // Use the child as the transaction for attributing errors to. + const Txid& child_hash = child_ws.m_ptx->GetHash(); + if (const auto err_string{PaysForRBF(/*original_fees=*/m_subpackage.m_conflicting_fees, + /*replacement_fees=*/m_subpackage.m_total_modified_fees, + /*replacement_vsize=*/m_subpackage.m_total_vsize, + m_pool.m_opts.incremental_relay_feerate, child_hash)}) { + return package_state.Invalid(PackageValidationResult::PCKG_POLICY, + "package RBF failed: insufficient anti-DoS fees", *err_string); + } + + // Ensure this two transaction package is a "chunk" on its own; we don't want the child + // to be only paying anti-DoS fees + const CFeeRate parent_feerate(parent_ws.m_modified_fees, parent_ws.m_vsize); + const CFeeRate package_feerate(m_subpackage.m_total_modified_fees, m_subpackage.m_total_vsize); + if (package_feerate <= parent_feerate) { + return package_state.Invalid(PackageValidationResult::PCKG_POLICY, + "package RBF failed: package feerate is less than parent feerate", + strprintf("package feerate %s <= parent feerate is %s", package_feerate.ToString(), parent_feerate.ToString())); + } + + // Check if it's economically rational to mine this package rather than the ones it replaces. + // This takes the place of ReplacementChecks()'s PaysMoreThanConflicts() in the package RBF setting. + if (const auto err_tup{ImprovesFeerateDiagram(m_pool, direct_conflict_iters, m_subpackage.m_all_conflicts, m_subpackage.m_total_modified_fees, m_subpackage.m_total_vsize)}) { + return package_state.Invalid(PackageValidationResult::PCKG_POLICY, + "package RBF failed: " + err_tup.value().second, ""); + } + + LogPrint(BCLog::TXPACKAGES, "package RBF checks passed: parent %s (wtxid=%s), child %s (wtxid=%s)\n", + txns.front()->GetHash().ToString(), txns.front()->GetWitnessHash().ToString(), + txns.back()->GetHash().ToString(), txns.back()->GetWitnessHash().ToString()); + + + return true; } bool MemPoolAccept::PolicyScriptChecks(const ATMPArgs& args, Workspace& ws) @@ -1156,19 +1289,21 @@ bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws) const uint256& hash = ws.m_hash; TxValidationState& state = ws.m_state; const bool bypass_limits = args.m_bypass_limits; - std::unique_ptr<CTxMemPoolEntry>& entry = ws.m_entry; + if (!m_subpackage.m_all_conflicts.empty()) Assume(args.m_allow_replacement); // Remove conflicting transactions from the mempool - for (CTxMemPool::txiter it : ws.m_all_conflicting) + for (CTxMemPool::txiter it : m_subpackage.m_all_conflicts) { - LogPrint(BCLog::MEMPOOL, "replacing tx %s (wtxid=%s) with %s (wtxid=%s) for %s additional fees, %d delta bytes\n", + LogPrint(BCLog::MEMPOOL, "replacing mempool tx %s (wtxid=%s, fees=%s, vsize=%s). New tx %s (wtxid=%s, fees=%s, vsize=%s)\n", it->GetTx().GetHash().ToString(), it->GetTx().GetWitnessHash().ToString(), + it->GetFee(), + it->GetTxSize(), hash.ToString(), tx.GetWitnessHash().ToString(), - FormatMoney(ws.m_modified_fees - ws.m_conflicting_fees), - (int)entry->GetTxSize() - (int)ws.m_conflicting_size); + entry->GetFee(), + entry->GetTxSize()); TRACE7(mempool, replaced, it->GetTx().GetHash().data(), it->GetTxSize(), @@ -1178,9 +1313,12 @@ bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws) entry->GetTxSize(), entry->GetFee() ); - ws.m_replaced_transactions.push_back(it->GetSharedTx()); + m_subpackage.m_replaced_transactions.push_back(it->GetSharedTx()); } - m_pool.RemoveStaged(ws.m_all_conflicting, false, MemPoolRemovalReason::REPLACED); + m_pool.RemoveStaged(m_subpackage.m_all_conflicts, false, MemPoolRemovalReason::REPLACED); + // Don't attempt to process the same conflicts repeatedly during subpackage evaluation: + // they no longer exist on subsequent calls to Finalize() post-RemoveStaged + m_subpackage.m_all_conflicts.clear(); // Store transaction in memory m_pool.addUnchecked(*entry, ws.m_ancestors); @@ -1259,6 +1397,13 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>& std::transform(workspaces.cbegin(), workspaces.cend(), std::back_inserter(all_package_wtxids), [](const auto& ws) { return ws.m_ptx->GetWitnessHash(); }); + if (!m_subpackage.m_replaced_transactions.empty()) { + LogPrint(BCLog::MEMPOOL, "replaced %u mempool transactions with %u new one(s) for %s additional fees, %d delta bytes\n", + m_subpackage.m_replaced_transactions.size(), workspaces.size(), + m_subpackage.m_total_modified_fees - m_subpackage.m_conflicting_fees, + m_subpackage.m_total_vsize - static_cast<int>(m_subpackage.m_conflicting_size)); + } + // Add successful results. The returned results may change later if LimitMempoolSize() evicts them. for (Workspace& ws : workspaces) { const auto effective_feerate = args.m_package_feerates ? ws.m_package_feerate : @@ -1266,7 +1411,7 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>& const auto effective_feerate_wtxids = args.m_package_feerates ? all_package_wtxids : std::vector<Wtxid>{ws.m_ptx->GetWitnessHash()}; results.emplace(ws.m_ptx->GetWitnessHash(), - MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, + MempoolAcceptResult::Success(std::move(m_subpackage.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, effective_feerate, effective_feerate_wtxids)); if (!m_pool.m_opts.signals) continue; const CTransaction& tx = *ws.m_ptx; @@ -1302,7 +1447,13 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef return MempoolAcceptResult::Failure(ws.m_state); } - if (m_rbf && !ReplacementChecks(ws)) return MempoolAcceptResult::Failure(ws.m_state); + if (m_subpackage.m_rbf && !ReplacementChecks(ws)) { + if (ws.m_state.GetResult() == TxValidationResult::TX_RECONSIDERABLE) { + // Failed for incentives-based fee reasons. Provide the effective feerate and which tx was included. + return MempoolAcceptResult::FeeFailure(ws.m_state, CFeeRate(ws.m_modified_fees, ws.m_vsize), single_wtxid); + } + return MempoolAcceptResult::Failure(ws.m_state); + } // Perform the inexpensive checks first and avoid hashing and signature verification unless // those checks pass, to mitigate CPU exhaustion denial-of-service attacks. @@ -1313,7 +1464,7 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef const CFeeRate effective_feerate{ws.m_modified_fees, static_cast<uint32_t>(ws.m_vsize)}; // Tx was accepted, but not added if (args.m_test_accept) { - return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, + return MempoolAcceptResult::Success(std::move(m_subpackage.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, effective_feerate, single_wtxid); } @@ -1334,7 +1485,14 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef m_pool.m_opts.signals->TransactionAddedToMempool(tx_info, m_pool.GetAndIncrementSequence()); } - return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, + if (!m_subpackage.m_replaced_transactions.empty()) { + LogPrint(BCLog::MEMPOOL, "replaced %u mempool transactions with 1 new transaction for %s additional fees, %d delta bytes\n", + m_subpackage.m_replaced_transactions.size(), + ws.m_modified_fees - m_subpackage.m_conflicting_fees, + ws.m_vsize - static_cast<int>(m_subpackage.m_conflicting_size)); + } + + return MempoolAcceptResult::Success(std::move(m_subpackage.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, effective_feerate, single_wtxid); } @@ -1375,10 +1533,14 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: } // Make the coins created by this transaction available for subsequent transactions in the - // package to spend. Since we already checked conflicts in the package and we don't allow - // replacements, we don't need to track the coins spent. Note that this logic will need to be - // updated if package replace-by-fee is allowed in the future. - assert(!args.m_allow_replacement); + // package to spend. If there are no conflicts within the package, no transaction can spend a coin + // needed by another transaction in the package. We also need to make sure that no package + // tx replaces (or replaces the ancestor of) the parent of another package tx. As long as we + // check these two things, we don't need to track the coins spent. + // If a package tx conflicts with a mempool tx, PackageMempoolChecks() ensures later that any package RBF attempt + // has *no* in-mempool ancestors, so we don't have to worry about subsequent transactions in + // same package spending the same in-mempool outpoints. This needs to be revisited for general + // package RBF. m_viewmempool.PackageAddTransaction(ws.m_ptx); } @@ -1400,28 +1562,26 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: // a child that is below mempool minimum feerate. To avoid these behaviors, callers of // AcceptMultipleTransactions need to restrict txns topology (e.g. to ancestor sets) and check // the feerates of individuals and subsets. - const auto m_total_vsize = std::accumulate(workspaces.cbegin(), workspaces.cend(), int64_t{0}, + m_subpackage.m_total_vsize = std::accumulate(workspaces.cbegin(), workspaces.cend(), int64_t{0}, [](int64_t sum, auto& ws) { return sum + ws.m_vsize; }); - const auto m_total_modified_fees = std::accumulate(workspaces.cbegin(), workspaces.cend(), CAmount{0}, + m_subpackage.m_total_modified_fees = std::accumulate(workspaces.cbegin(), workspaces.cend(), CAmount{0}, [](CAmount sum, auto& ws) { return sum + ws.m_modified_fees; }); - const CFeeRate package_feerate(m_total_modified_fees, m_total_vsize); + const CFeeRate package_feerate(m_subpackage.m_total_modified_fees, m_subpackage.m_total_vsize); std::vector<Wtxid> all_package_wtxids; all_package_wtxids.reserve(workspaces.size()); std::transform(workspaces.cbegin(), workspaces.cend(), std::back_inserter(all_package_wtxids), [](const auto& ws) { return ws.m_ptx->GetWitnessHash(); }); TxValidationState placeholder_state; if (args.m_package_feerates && - !CheckFeeRate(m_total_vsize, m_total_modified_fees, placeholder_state)) { + !CheckFeeRate(m_subpackage.m_total_vsize, m_subpackage.m_total_modified_fees, placeholder_state)) { package_state.Invalid(PackageValidationResult::PCKG_TX, "transaction failed"); return PackageMempoolAcceptResult(package_state, {{workspaces.back().m_ptx->GetWitnessHash(), - MempoolAcceptResult::FeeFailure(placeholder_state, CFeeRate(m_total_modified_fees, m_total_vsize), all_package_wtxids)}}); + MempoolAcceptResult::FeeFailure(placeholder_state, CFeeRate(m_subpackage.m_total_modified_fees, m_subpackage.m_total_vsize), all_package_wtxids)}}); } // Apply package mempool ancestor/descendant limits. Skip if there is only one transaction, - // because it's unnecessary. Also, CPFP carve out can increase the limit for individual - // transactions, but this exemption is not extended to packages in CheckPackageLimits(). - std::string err_string; - if (txns.size() > 1 && !PackageMempoolChecks(txns, m_total_vsize, package_state)) { + // because it's unnecessary. + if (txns.size() > 1 && !PackageMempoolChecks(txns, workspaces, m_subpackage.m_total_vsize, package_state)) { return PackageMempoolAcceptResult(package_state, std::move(results)); } @@ -1439,7 +1599,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: const auto effective_feerate_wtxids = args.m_package_feerates ? all_package_wtxids : std::vector<Wtxid>{ws.m_ptx->GetWitnessHash()}; results.emplace(ws.m_ptx->GetWitnessHash(), - MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), + MempoolAcceptResult::Success(std::move(m_subpackage.m_replaced_transactions), ws.m_vsize, ws.m_base_fees, effective_feerate, effective_feerate_wtxids)); } @@ -1504,7 +1664,8 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptSubPackage(const std::vector<CTr // Clean up m_view and m_viewmempool so that other subpackage evaluations don't have access to // coins they shouldn't. Keep some coins in order to minimize re-fetching coins from the UTXO set. - CleanupTemporaryCoins(); + // Clean up package feerate and rbf calculations + ClearSubPackageState(); return result; } @@ -1688,7 +1849,6 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, MempoolAcceptResult AcceptToMemoryPool(Chainstate& active_chainstate, const CTransactionRef& tx, int64_t accept_time, bool bypass_limits, bool test_accept) - EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main); const CChainParams& chainparams{active_chainstate.m_chainman.GetParams()}; @@ -1862,9 +2022,11 @@ void Chainstate::CheckForkWarningConditions() if (m_chainman.m_best_invalid && m_chainman.m_best_invalid->nChainWork > m_chain.Tip()->nChainWork + (GetBlockProof(*m_chain.Tip()) * 6)) { LogPrintf("%s: Warning: Found invalid chain at least ~6 blocks longer than our best chain.\nChain state database corruption likely.\n", __func__); - SetfLargeWorkInvalidChainFound(true); + m_chainman.GetNotifications().warningSet( + kernel::Warning::LARGE_WORK_INVALID_CHAIN, + _("Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade.")); } else { - SetfLargeWorkInvalidChainFound(false); + m_chainman.GetNotifications().warningUnset(kernel::Warning::LARGE_WORK_INVALID_CHAIN); } } @@ -2641,7 +2803,7 @@ bool Chainstate::FlushStateToDisk( CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(); LOCK(m_blockman.cs_LastBlockFile); - if (m_blockman.IsPruneMode() && (m_blockman.m_check_for_pruning || nManualPruneHeight > 0) && !m_chainman.m_blockman.m_reindexing) { + if (m_blockman.IsPruneMode() && (m_blockman.m_check_for_pruning || nManualPruneHeight > 0) && m_chainman.m_blockman.m_blockfiles_indexed) { // make sure we don't prune above any of the prune locks bestblocks // pruning is height-based int last_prune{m_chain.Height()}; // last height we can prune @@ -2788,13 +2950,6 @@ void Chainstate::PruneAndFlush() } } -/** Private helper function that concatenates warning messages. */ -static void AppendWarning(bilingual_str& res, const bilingual_str& warn) -{ - if (!res.empty()) res += Untranslated(", "); - res += warn; -} - static void UpdateTipLog( const CCoinsViewCache& coins_tip, const CBlockIndex* tip, @@ -2845,7 +3000,7 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew) g_best_block_cv.notify_all(); } - bilingual_str warning_messages; + std::vector<bilingual_str> warning_messages; if (!m_chainman.IsInitialBlockDownload()) { const CBlockIndex* pindex = pindexNew; for (int bit = 0; bit < VERSIONBITS_NUM_BITS; bit++) { @@ -2854,14 +3009,15 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew) if (state == ThresholdState::ACTIVE || state == ThresholdState::LOCKED_IN) { const bilingual_str warning = strprintf(_("Unknown new rules activated (versionbit %i)"), bit); if (state == ThresholdState::ACTIVE) { - m_chainman.GetNotifications().warning(warning); + m_chainman.GetNotifications().warningSet(kernel::Warning::UNKNOWN_NEW_RULES_ACTIVATED, warning); } else { - AppendWarning(warning_messages, warning); + warning_messages.push_back(warning); } } } } - UpdateTipLog(coins_tip, pindexNew, params, __func__, "", warning_messages.original); + UpdateTipLog(coins_tip, pindexNew, params, __func__, "", + util::Join(warning_messages, Untranslated(", ")).original); } /** Disconnect m_chain's tip. @@ -3254,10 +3410,10 @@ bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex* return true; } -static SynchronizationState GetSynchronizationState(bool init, bool reindexing) +static SynchronizationState GetSynchronizationState(bool init, bool blockfiles_indexed) { if (!init) return SynchronizationState::POST_INIT; - if (reindexing) return SynchronizationState::INIT_REINDEX; + if (!blockfiles_indexed) return SynchronizationState::INIT_REINDEX; return SynchronizationState::INIT_DOWNLOAD; } @@ -3279,7 +3435,7 @@ static bool NotifyHeaderTip(ChainstateManager& chainman) LOCKS_EXCLUDED(cs_main) } // Send block tip changed notifications without cs_main if (fNotify) { - chainman.GetNotifications().headerTip(GetSynchronizationState(fInitialBlockDownload, chainman.m_blockman.m_reindexing), pindexHeader->nHeight, pindexHeader->nTime, false); + chainman.GetNotifications().headerTip(GetSynchronizationState(fInitialBlockDownload, chainman.m_blockman.m_blockfiles_indexed), pindexHeader->nHeight, pindexHeader->nTime, false); } return fNotify; } @@ -3398,7 +3554,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< } // Always notify the UI if a new block tip was connected - if (kernel::IsInterrupted(m_chainman.GetNotifications().blockTip(GetSynchronizationState(still_in_ibd, m_chainman.m_blockman.m_reindexing), *pindexNewTip))) { + if (kernel::IsInterrupted(m_chainman.GetNotifications().blockTip(GetSynchronizationState(still_in_ibd, m_chainman.m_blockman.m_blockfiles_indexed), *pindexNewTip))) { // Just breaking and returning success for now. This could // be changed to bubble up the kernel::Interrupted value to // the caller so the caller could distinguish between @@ -3624,7 +3780,7 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde // parameter indicating the source of the tip change so hooks can // distinguish user-initiated invalidateblock changes from other // changes. - (void)m_chainman.GetNotifications().blockTip(GetSynchronizationState(m_chainman.IsInitialBlockDownload(), m_chainman.m_blockman.m_reindexing), *to_mark_failed->pprev); + (void)m_chainman.GetNotifications().blockTip(GetSynchronizationState(m_chainman.IsInitialBlockDownload(), m_chainman.m_blockman.m_blockfiles_indexed), *to_mark_failed->pprev); } return true; } @@ -4263,7 +4419,7 @@ void ChainstateManager::ReportHeadersPresync(const arith_uint256& work, int64_t m_last_presync_update = now; } bool initial_download = IsInitialBlockDownload(); - GetNotifications().headerTip(GetSynchronizationState(initial_download, m_blockman.m_reindexing), height, timestamp, /*presync=*/true); + GetNotifications().headerTip(GetSynchronizationState(initial_download, m_blockman.m_blockfiles_indexed), height, timestamp, /*presync=*/true); if (initial_download) { int64_t blocks_left{(NodeClock::now() - NodeSeconds{std::chrono::seconds{timestamp}}) / GetConsensus().PowTargetSpacing()}; blocks_left = std::max<int64_t>(0, blocks_left); @@ -4790,8 +4946,7 @@ bool ChainstateManager::LoadBlockIndex() { AssertLockHeld(cs_main); // Load block index from databases - bool needs_init = m_blockman.m_reindexing; - if (!m_blockman.m_reindexing) { + if (m_blockman.m_blockfiles_indexed) { bool ret{m_blockman.LoadBlockIndexDB(SnapshotBlockhash())}; if (!ret) return false; @@ -4822,18 +4977,6 @@ bool ChainstateManager::LoadBlockIndex() if (pindex->IsValid(BLOCK_VALID_TREE) && (m_best_header == nullptr || CBlockIndexWorkComparator()(m_best_header, pindex))) m_best_header = pindex; } - - needs_init = m_blockman.m_block_index.empty(); - } - - if (needs_init) { - // Everything here is for *new* reindex/DBs. Thus, though - // LoadBlockIndexDB may have set m_reindexing if we shut down - // mid-reindex previously, we don't check m_reindexing and - // instead only check it prior to LoadBlockIndexDB to set - // needs_init. - - LogPrintf("Initializing databases...\n"); } return true; } @@ -4974,7 +5117,7 @@ void ChainstateManager::LoadExternalBlockFile( } } - if (m_blockman.IsPruneMode() && !m_blockman.m_reindexing && pblock) { + if (m_blockman.IsPruneMode() && m_blockman.m_blockfiles_indexed && pblock) { // must update the tip for pruning to work while importing with -loadblock. // this is a tradeoff to conserve disk space at the expense of time // spent updating the tip to be able to prune. @@ -5046,6 +5189,14 @@ void ChainstateManager::LoadExternalBlockFile( LogPrintf("Loaded %i blocks from external file in %dms\n", nLoaded, Ticks<std::chrono::milliseconds>(SteadyClock::now() - start)); } +bool ChainstateManager::ShouldCheckBlockIndex() const +{ + // Assert to verify Flatten() has been called. + if (!*Assert(m_options.check_block_index)) return false; + if (GetRand(*m_options.check_block_index) >= 1) return false; + return true; +} + void ChainstateManager::CheckBlockIndex() { if (!ShouldCheckBlockIndex()) { @@ -5062,19 +5213,28 @@ void ChainstateManager::CheckBlockIndex() return; } - // Build forward-pointing map of the entire block tree. + // Build forward-pointing data structure for the entire block tree. + // For performance reasons, indexes of the best header chain are stored in a vector (within CChain). + // All remaining blocks are stored in a multimap. + // The best header chain can differ from the active chain: E.g. its entries may belong to blocks that + // are not yet validated. + CChain best_hdr_chain; + assert(m_best_header); + best_hdr_chain.SetTip(*m_best_header); + std::multimap<CBlockIndex*,CBlockIndex*> forward; for (auto& [_, block_index] : m_blockman.m_block_index) { - forward.emplace(block_index.pprev, &block_index); + // Only save indexes in forward that are not part of the best header chain. + if (!best_hdr_chain.Contains(&block_index)) { + // Only genesis, which must be part of the best header chain, can have a nullptr parent. + assert(block_index.pprev); + forward.emplace(block_index.pprev, &block_index); + } } + assert(forward.size() + best_hdr_chain.Height() + 1 == m_blockman.m_block_index.size()); - assert(forward.size() == m_blockman.m_block_index.size()); - - std::pair<std::multimap<CBlockIndex*,CBlockIndex*>::iterator,std::multimap<CBlockIndex*,CBlockIndex*>::iterator> rangeGenesis = forward.equal_range(nullptr); - CBlockIndex *pindex = rangeGenesis.first->second; - rangeGenesis.first++; - assert(rangeGenesis.first == rangeGenesis.second); // There is only one index entry with parent nullptr. - + CBlockIndex* pindex = best_hdr_chain[0]; + assert(pindex); // Iterate over the entire block tree, using depth-first search. // Along the way, remember whether there are blocks on the path from genesis // block being explored which are the first to have certain properties. @@ -5286,14 +5446,21 @@ void ChainstateManager::CheckBlockIndex() // assert(pindex->GetBlockHash() == pindex->GetBlockHeader().GetHash()); // Perhaps too slow // End: actual consistency checks. - // Try descending into the first subnode. + + // Try descending into the first subnode. Always process forks first and the best header chain after. snap_update_firsts(); std::pair<std::multimap<CBlockIndex*,CBlockIndex*>::iterator,std::multimap<CBlockIndex*,CBlockIndex*>::iterator> range = forward.equal_range(pindex); if (range.first != range.second) { - // A subnode was found. + // A subnode not part of the best header chain was found. pindex = range.first->second; nHeight++; continue; + } else if (best_hdr_chain.Contains(pindex)) { + // Descend further into best header chain. + nHeight++; + pindex = best_hdr_chain[nHeight]; + if (!pindex) break; // we are finished, since the best header chain is always processed last + continue; } // This is a leaf node. // Move upwards until we reach a node of which we have not yet visited the last child. @@ -5319,9 +5486,15 @@ void ChainstateManager::CheckBlockIndex() // Proceed to the next one. rangePar.first++; if (rangePar.first != rangePar.second) { - // Move to the sibling. + // Move to a sibling not part of the best header chain. pindex = rangePar.first->second; break; + } else if (pindexPar == best_hdr_chain[nHeight - 1]) { + // Move to pindex's sibling on the best-chain, if it has one. + pindex = best_hdr_chain[nHeight]; + // There will not be a next block if (and only if) parent block is the best header. + assert((pindex == nullptr) == (pindexPar == best_hdr_chain.Tip())); + break; } else { // Move up further. pindex = pindexPar; @@ -5331,8 +5504,8 @@ void ChainstateManager::CheckBlockIndex() } } - // Check that we actually traversed the entire map. - assert(nNodes == forward.size()); + // Check that we actually traversed the entire block index. + assert(nNodes == forward.size() + best_hdr_chain.Height() + 1); } std::string Chainstate::ToString() @@ -5665,69 +5838,81 @@ bool ChainstateManager::PopulateAndValidateSnapshot( return false; } - COutPoint outpoint; - Coin coin; const uint64_t coins_count = metadata.m_coins_count; uint64_t coins_left = metadata.m_coins_count; - LogPrintf("[snapshot] loading coins from snapshot %s\n", base_blockhash.ToString()); + LogPrintf("[snapshot] loading %d coins from snapshot %s\n", coins_left, base_blockhash.ToString()); int64_t coins_processed{0}; while (coins_left > 0) { try { - coins_file >> outpoint; - coins_file >> coin; - } catch (const std::ios_base::failure&) { - LogPrintf("[snapshot] bad snapshot format or truncated snapshot after deserializing %d coins\n", - coins_count - coins_left); - return false; - } - if (coin.nHeight > base_height || - outpoint.n >= std::numeric_limits<decltype(outpoint.n)>::max() // Avoid integer wrap-around in coinstats.cpp:ApplyHash - ) { - LogPrintf("[snapshot] bad snapshot data after deserializing %d coins\n", - coins_count - coins_left); - return false; - } - if (!MoneyRange(coin.out.nValue)) { - LogPrintf("[snapshot] bad snapshot data after deserializing %d coins - bad tx out value\n", - coins_count - coins_left); - return false; - } + Txid txid; + coins_file >> txid; + size_t coins_per_txid{0}; + coins_per_txid = ReadCompactSize(coins_file); - coins_cache.EmplaceCoinInternalDANGER(std::move(outpoint), std::move(coin)); + if (coins_per_txid > coins_left) { + LogPrintf("[snapshot] mismatch in coins count in snapshot metadata and actual snapshot data\n"); + return false; + } - --coins_left; - ++coins_processed; + for (size_t i = 0; i < coins_per_txid; i++) { + COutPoint outpoint; + Coin coin; + outpoint.n = static_cast<uint32_t>(ReadCompactSize(coins_file)); + outpoint.hash = txid; + coins_file >> coin; + if (coin.nHeight > base_height || + outpoint.n >= std::numeric_limits<decltype(outpoint.n)>::max() // Avoid integer wrap-around in coinstats.cpp:ApplyHash + ) { + LogPrintf("[snapshot] bad snapshot data after deserializing %d coins\n", + coins_count - coins_left); + return false; + } + if (!MoneyRange(coin.out.nValue)) { + LogPrintf("[snapshot] bad snapshot data after deserializing %d coins - bad tx out value\n", + coins_count - coins_left); + return false; + } + coins_cache.EmplaceCoinInternalDANGER(std::move(outpoint), std::move(coin)); - if (coins_processed % 1000000 == 0) { - LogPrintf("[snapshot] %d coins loaded (%.2f%%, %.2f MB)\n", - coins_processed, - static_cast<float>(coins_processed) * 100 / static_cast<float>(coins_count), - coins_cache.DynamicMemoryUsage() / (1000 * 1000)); - } + --coins_left; + ++coins_processed; - // Batch write and flush (if we need to) every so often. - // - // If our average Coin size is roughly 41 bytes, checking every 120,000 coins - // means <5MB of memory imprecision. - if (coins_processed % 120000 == 0) { - if (m_interrupt) { - return false; - } + if (coins_processed % 1000000 == 0) { + LogPrintf("[snapshot] %d coins loaded (%.2f%%, %.2f MB)\n", + coins_processed, + static_cast<float>(coins_processed) * 100 / static_cast<float>(coins_count), + coins_cache.DynamicMemoryUsage() / (1000 * 1000)); + } + + // Batch write and flush (if we need to) every so often. + // + // If our average Coin size is roughly 41 bytes, checking every 120,000 coins + // means <5MB of memory imprecision. + if (coins_processed % 120000 == 0) { + if (m_interrupt) { + return false; + } - const auto snapshot_cache_state = WITH_LOCK(::cs_main, - return snapshot_chainstate.GetCoinsCacheSizeState()); + const auto snapshot_cache_state = WITH_LOCK(::cs_main, + return snapshot_chainstate.GetCoinsCacheSizeState()); - if (snapshot_cache_state >= CoinsCacheSizeState::CRITICAL) { - // This is a hack - we don't know what the actual best block is, but that - // doesn't matter for the purposes of flushing the cache here. We'll set this - // to its correct value (`base_blockhash`) below after the coins are loaded. - coins_cache.SetBestBlock(GetRandHash()); + if (snapshot_cache_state >= CoinsCacheSizeState::CRITICAL) { + // This is a hack - we don't know what the actual best block is, but that + // doesn't matter for the purposes of flushing the cache here. We'll set this + // to its correct value (`base_blockhash`) below after the coins are loaded. + coins_cache.SetBestBlock(GetRandHash()); - // No need to acquire cs_main since this chainstate isn't being used yet. - FlushSnapshotToDisk(coins_cache, /*snapshot_loaded=*/false); + // No need to acquire cs_main since this chainstate isn't being used yet. + FlushSnapshotToDisk(coins_cache, /*snapshot_loaded=*/false); + } + } } + } catch (const std::ios_base::failure&) { + LogPrintf("[snapshot] bad snapshot format or truncated snapshot after deserializing %d coins\n", + coins_processed); + return false; } } @@ -5740,7 +5925,8 @@ bool ChainstateManager::PopulateAndValidateSnapshot( bool out_of_coins{false}; try { - coins_file >> outpoint; + std::byte left_over_byte; + coins_file >> left_over_byte; } catch (const std::ios_base::failure&) { // We expect an exception since we should be out of coins. out_of_coins = true; @@ -5878,8 +6064,8 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation() PACKAGE_NAME, snapshot_tip_height, snapshot_base_height, snapshot_base_height, PACKAGE_BUGREPORT ); - LogPrintf("[snapshot] !!! %s\n", user_error.original); - LogPrintf("[snapshot] deleting snapshot, reverting to validated chain, and stopping node\n"); + LogError("[snapshot] !!! %s\n", user_error.original); + LogError("[snapshot] deleting snapshot, reverting to validated chain, and stopping node\n"); m_active_chainstate = m_ibd_chainstate.get(); m_snapshot_chainstate->m_disabled = true; @@ -6231,7 +6417,7 @@ bool ChainstateManager::ValidatedSnapshotCleanup() fs::path p_old, fs::path p_new, const fs::filesystem_error& err) { - LogPrintf("Error renaming path (%s) -> (%s): %s\n", + LogError("[snapshot] Error renaming path (%s) -> (%s): %s\n", fs::PathToString(p_old), fs::PathToString(p_new), err.what()); GetNotifications().fatalError(strprintf(_( "Rename of '%s' -> '%s' failed. " |