diff --git a/src/coins.cpp b/src/coins.cpp index 30cd532262..6efeba9b76 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -9,7 +9,7 @@ #include #include -bool CCoinsView::GetCoin(const COutPoint &outpoint, Coin &coin) const { return false; } +std::optional CCoinsView::GetCoin(const COutPoint& outpoint) const { return std::nullopt; } uint256 CCoinsView::GetBestBlock() const { return uint256(); } std::vector CCoinsView::GetHeadBlocks() const { return std::vector(); } bool CCoinsView::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &hashBlock) { return false; } @@ -19,12 +19,11 @@ bool CCoinsView::IsPeginSpent(const std::pair &outpoint) con bool CCoinsView::HaveCoin(const COutPoint &outpoint) const { - Coin coin; - return GetCoin(outpoint, coin); + return GetCoin(outpoint).has_value(); } CCoinsViewBacked::CCoinsViewBacked(CCoinsView *viewIn) : base(viewIn) { } -bool CCoinsViewBacked::GetCoin(const COutPoint &outpoint, Coin &coin) const { return base->GetCoin(outpoint, coin); } +std::optional CCoinsViewBacked::GetCoin(const COutPoint& outpoint) const { return base->GetCoin(outpoint); } bool CCoinsViewBacked::HaveCoin(const COutPoint &outpoint) const { return base->HaveCoin(outpoint); } uint256 CCoinsViewBacked::GetBestBlock() const { return base->GetBestBlock(); } std::vector CCoinsViewBacked::GetHeadBlocks() const { return base->GetHeadBlocks(); } @@ -55,26 +54,25 @@ static inline CCoinsMapKey native_key(const COutPoint& outpoint) { CCoinsMap::iterator CCoinsViewCache::FetchCoin(const COutPoint &outpoint) const { const auto [ret, inserted] = cacheCoins.try_emplace(native_key(outpoint)); if (inserted) { - if (!base->GetCoin(outpoint, ret->second.coin)) { + if (auto coin{base->GetCoin(outpoint)}) { + ret->second.coin = std::move(*coin); + cachedCoinsUsage += ret->second.coin.DynamicMemoryUsage(); + if (ret->second.coin.IsSpent()) { // TODO GetCoin cannot return spent coins + // The parent only has an empty entry for this outpoint; we can consider our version as fresh. + ret->second.AddFlags(CCoinsCacheEntry::FRESH, *ret, m_sentinel); + } + } else { cacheCoins.erase(ret); return cacheCoins.end(); } - if (ret->second.coin.IsSpent()) { - // The parent only has an empty entry for this outpoint; we can consider our version as fresh. - ret->second.AddFlags(CCoinsCacheEntry::FRESH, *ret, m_sentinel); - } - cachedCoinsUsage += ret->second.coin.DynamicMemoryUsage(); } return ret; } -bool CCoinsViewCache::GetCoin(const COutPoint &outpoint, Coin &coin) const { - CCoinsMap::const_iterator it = FetchCoin(outpoint); - if (it != cacheCoins.end()) { - coin = it->second.coin; - return !coin.IsSpent(); - } - return false; +std::optional CCoinsViewCache::GetCoin(const COutPoint& outpoint) const +{ + if (auto it{FetchCoin(outpoint)}; it != cacheCoins.end() && !it->second.coin.IsSpent()) return it->second.coin; + return std::nullopt; } void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possible_overwrite) { @@ -447,8 +445,8 @@ const Coin& AccessByTxid(const CCoinsViewCache& view, const Txid& txid) return coinEmpty; } -template -static bool ExecuteBackedWrapper(Func func, const std::vector>& err_callbacks) +template +static ReturnType ExecuteBackedWrapper(Func func, const std::vector>& err_callbacks) { try { return func(); @@ -465,10 +463,12 @@ static bool ExecuteBackedWrapper(Func func, const std::vector CCoinsViewErrorCatcher::GetCoin(const COutPoint& outpoint) const +{ + return ExecuteBackedWrapper>([&]() { return CCoinsViewBacked::GetCoin(outpoint); }, m_err_callbacks); } -bool CCoinsViewErrorCatcher::HaveCoin(const COutPoint &outpoint) const { - return ExecuteBackedWrapper([&]() { return CCoinsViewBacked::HaveCoin(outpoint); }, m_err_callbacks); +bool CCoinsViewErrorCatcher::HaveCoin(const COutPoint& outpoint) const +{ + return ExecuteBackedWrapper([&]() { return CCoinsViewBacked::HaveCoin(outpoint); }, m_err_callbacks); } diff --git a/src/coins.h b/src/coins.h index f188d305a4..28b9d7104c 100644 --- a/src/coins.h +++ b/src/coins.h @@ -311,11 +311,8 @@ private: class CCoinsView { public: - /** Retrieve the Coin (unspent transaction output) for a given outpoint. - * Returns true only when an unspent coin was found, which is returned in coin. - * When false is returned, coin's value is unspecified. - */ - virtual bool GetCoin(const COutPoint &outpoint, Coin &coin) const; + //! Retrieve the Coin (unspent transaction output) for a given outpoint. + virtual std::optional GetCoin(const COutPoint& outpoint) const; //! Just check whether a given outpoint is unspent. virtual bool HaveCoin(const COutPoint &outpoint) const; @@ -356,7 +353,7 @@ protected: public: CCoinsViewBacked(CCoinsView *viewIn); - bool GetCoin(const COutPoint &outpoint, Coin &coin) const override; + std::optional GetCoin(const COutPoint& outpoint) const override; bool HaveCoin(const COutPoint &outpoint) const override; uint256 GetBestBlock() const override; std::vector GetHeadBlocks() const override; @@ -398,7 +395,7 @@ public: CCoinsViewCache(const CCoinsViewCache &) = delete; // Standard CCoinsView methods - bool GetCoin(const COutPoint &outpoint, Coin &coin) const override; + std::optional GetCoin(const COutPoint& outpoint) const override; bool HaveCoin(const COutPoint &outpoint) const override; uint256 GetBestBlock() const override; void SetBestBlock(const uint256 &hashBlock); @@ -531,7 +528,7 @@ public: m_err_callbacks.emplace_back(std::move(f)); } - bool GetCoin(const COutPoint &outpoint, Coin &coin) const override; + std::optional GetCoin(const COutPoint& outpoint) const override; bool HaveCoin(const COutPoint &outpoint) const override; private: diff --git a/src/node/coin.cpp b/src/node/coin.cpp index 221854c5f6..eba67f4cb5 100644 --- a/src/node/coin.cpp +++ b/src/node/coin.cpp @@ -16,10 +16,11 @@ void FindCoins(const NodeContext& node, std::map& coins) LOCK2(cs_main, node.mempool->cs); CCoinsViewCache& chain_view = node.chainman->ActiveChainstate().CoinsTip(); CCoinsViewMemPool mempool_view(&chain_view, *node.mempool); - for (auto& coin : coins) { - if (!mempool_view.GetCoin(coin.first, coin.second)) { - // Either the coin is not in the CCoinsViewCache or is spent. Clear it. - coin.second.Clear(); + for (auto& [outpoint, coin] : coins) { + if (auto c{mempool_view.GetCoin(outpoint)}) { + coin = std::move(*c); + } else { + coin.Clear(); // Either the coin is not in the CCoinsViewCache or is spent } } } diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 816d07d37a..bee744f569 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -358,9 +358,7 @@ public: std::optional getUnspentOutput(const COutPoint& output) override { LOCK(::cs_main); - Coin coin; - if (chainman().ActiveChainstate().CoinsTip().GetCoin(output, coin)) return coin; - return {}; + return chainman().ActiveChainstate().CoinsTip().GetCoin(output); } TransactionError broadcastTransaction(CTransactionRef tx, CAmount max_tx_fee, std::string& err_string) override { diff --git a/src/rest.cpp b/src/rest.cpp index 8e0d34e69c..c35f320a9c 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -876,10 +876,9 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std:: { auto process_utxos = [&vOutPoints, &outs, &hits, &active_height, &active_hash, &chainman](const CCoinsView& view, const CTxMemPool* mempool) EXCLUSIVE_LOCKS_REQUIRED(chainman.GetMutex()) { for (const COutPoint& vOutPoint : vOutPoints) { - Coin coin; - bool hit = (!mempool || !mempool->isSpent(vOutPoint)) && view.GetCoin(vOutPoint, coin); - hits.push_back(hit); - if (hit) outs.emplace_back(std::move(coin)); + auto coin = !mempool || !mempool->isSpent(vOutPoint) ? view.GetCoin(vOutPoint) : std::nullopt; + hits.push_back(coin.has_value()); + if (coin) outs.emplace_back(std::move(*coin)); } active_height = chainman.ActiveHeight(); active_hash = chainman.ActiveTip()->GetBlockHash(); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index e1ec62f354..65d1e18b03 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1270,48 +1270,45 @@ static RPCHelpMan gettxout() if (!request.params[2].isNull()) fMempool = request.params[2].get_bool(); - Coin coin; Chainstate& active_chainstate = chainman.ActiveChainstate(); CCoinsViewCache* coins_view = &active_chainstate.CoinsTip(); + std::optional coin; if (fMempool) { const CTxMemPool& mempool = EnsureMemPool(node); LOCK(mempool.cs); CCoinsViewMemPool view(coins_view, mempool); - if (!view.GetCoin(out, coin) || mempool.isSpent(out)) { - return UniValue::VNULL; - } + if (!mempool.isSpent(out)) coin = view.GetCoin(out); } else { - if (!coins_view->GetCoin(out, coin)) { - return UniValue::VNULL; - } + coin = coins_view->GetCoin(out); } + if (!coin) return UniValue::VNULL; const CBlockIndex* pindex = active_chainstate.m_blockman.LookupBlockIndex(coins_view->GetBestBlock()); ret.pushKV("bestblock", pindex->GetBlockHash().GetHex()); - if (coin.nHeight == MEMPOOL_HEIGHT) { + if (coin->nHeight == MEMPOOL_HEIGHT) { ret.pushKV("confirmations", 0); } else { - ret.pushKV("confirmations", (int64_t)(pindex->nHeight - coin.nHeight + 1)); + ret.pushKV("confirmations", (int64_t)(pindex->nHeight - coin->nHeight + 1)); } - if (coin.out.nValue.IsExplicit()) { - ret.pushKV("value", ValueFromAmount(coin.out.nValue.GetAmount())); + if (coin->out.nValue.IsExplicit()) { + ret.pushKV("value", ValueFromAmount(coin->out.nValue.GetAmount())); } else { - ret.pushKV("valuecommitment", coin.out.nValue.GetHex()); + ret.pushKV("valuecommitment", coin->out.nValue.GetHex()); } if (g_con_elementsmode) { - if (coin.out.nAsset.IsExplicit()) { - ret.pushKV("asset", coin.out.nAsset.GetAsset().GetHex()); + if (coin->out.nAsset.IsExplicit()) { + ret.pushKV("asset", coin->out.nAsset.GetAsset().GetHex()); } else { - ret.pushKV("assetcommitment", coin.out.nAsset.GetHex()); + ret.pushKV("assetcommitment", coin->out.nAsset.GetHex()); } - ret.pushKV("commitmentnonce", coin.out.nNonce.GetHex()); + ret.pushKV("commitmentnonce", coin->out.nNonce.GetHex()); } UniValue o(UniValue::VOBJ); - ScriptToUniv(coin.out.scriptPubKey, /*out=*/o, /*include_hex=*/true, /*include_address=*/true); + ScriptToUniv(coin->out.scriptPubKey, /*out=*/o, /*include_hex=*/true, /*include_address=*/true); ret.pushKV("scriptPubKey", std::move(o)); - ret.pushKV("coinbase", (bool)coin.fCoinBase); + ret.pushKV("coinbase", (bool)coin->fCoinBase); return ret; }, diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 6385e708e6..b700ddbef3 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -44,18 +44,14 @@ class CCoinsViewTest : public CCoinsView public: CCoinsViewTest(FastRandomContext& rng) : m_rng{rng} {} - [[nodiscard]] bool GetCoin(const COutPoint& outpoint, Coin& coin) const override + std::optional GetCoin(const COutPoint& outpoint) const override { - std::map::const_iterator it = map_.find(outpoint); - if (it == map_.end()) { - return false; + if (auto it{map_.find(outpoint)}; it != map_.end()) { + if (!it->second.IsSpent() || m_rng.randbool()) { + return it->second; // TODO spent coins shouldn't be returned + } } - coin = it->second; - if (coin.IsSpent() && m_rng.randbool() == 0) { - // Randomly return false in case of an empty entry. - return false; - } - return true; + return std::nullopt; } uint256 GetBestBlock() const override { return hashBestBlock_; } diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index eeba88da87..ce6ad6d4ee 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -173,22 +173,20 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view) const bool exists_using_access_coin = !(coin_using_access_coin == EMPTY_COIN); const bool exists_using_have_coin = coins_view_cache.HaveCoin(random_out_point); const bool exists_using_have_coin_in_cache = coins_view_cache.HaveCoinInCache(random_out_point); - Coin coin_using_get_coin; - const bool exists_using_get_coin = coins_view_cache.GetCoin(random_out_point, coin_using_get_coin); - if (exists_using_get_coin) { - assert(coin_using_get_coin == coin_using_access_coin); + if (auto coin{coins_view_cache.GetCoin(random_out_point)}) { + assert(*coin == coin_using_access_coin); + assert(exists_using_access_coin && exists_using_have_coin_in_cache && exists_using_have_coin); + } else { + assert(!exists_using_access_coin && !exists_using_have_coin_in_cache && !exists_using_have_coin); } - assert((exists_using_access_coin && exists_using_have_coin_in_cache && exists_using_have_coin && exists_using_get_coin) || - (!exists_using_access_coin && !exists_using_have_coin_in_cache && !exists_using_have_coin && !exists_using_get_coin)); // If HaveCoin on the backend is true, it must also be on the cache if the coin wasn't spent. const bool exists_using_have_coin_in_backend = backend_coins_view.HaveCoin(random_out_point); if (!coin_using_access_coin.IsSpent() && exists_using_have_coin_in_backend) { assert(exists_using_have_coin); } - Coin coin_using_backend_get_coin; - if (backend_coins_view.GetCoin(random_out_point, coin_using_backend_get_coin)) { + if (auto coin{backend_coins_view.GetCoin(random_out_point)}) { assert(exists_using_have_coin_in_backend); - // Note we can't assert that `coin_using_get_coin == coin_using_backend_get_coin` because the coin in + // Note we can't assert that `coin_using_get_coin == *coin` because the coin in // the cache may have been modified but not yet flushed. } else { assert(!exists_using_have_coin_in_backend); diff --git a/src/test/fuzz/coinscache_sim.cpp b/src/test/fuzz/coinscache_sim.cpp index 8d257a88ab..5716cfadda 100644 --- a/src/test/fuzz/coinscache_sim.cpp +++ b/src/test/fuzz/coinscache_sim.cpp @@ -137,9 +137,8 @@ struct CacheLevel /** Class for the base of the hierarchy (roughly simulating a memory-backed CCoinsViewDB). * - * The initial state consists of the empty UTXO set, though coins whose output index - * is 3 (mod 5) always have GetCoin() succeed (but returning an IsSpent() coin unless a UTXO - * exists). Coins whose output index is 4 (mod 5) have GetCoin() always succeed after being spent. + * The initial state consists of the empty UTXO set. + * Coins whose output index is 4 (mod 5) have GetCoin() always succeed after being spent. * This exercises code paths with spent, non-DIRTY cache entries. */ class CoinsViewBottom final : public CCoinsView @@ -147,19 +146,11 @@ class CoinsViewBottom final : public CCoinsView std::map m_data; public: - bool GetCoin(const COutPoint& outpoint, Coin& coin) const final + std::optional GetCoin(const COutPoint& outpoint) const final { - auto it = m_data.find(outpoint); - if (it == m_data.end()) { - if ((outpoint.n % 5) == 3) { - coin.Clear(); - return true; - } - return false; - } else { - coin = it->second; - return true; - } + // TODO GetCoin shouldn't return spent coins + if (auto it = m_data.find(outpoint); it != m_data.end()) return it->second; + return std::nullopt; } bool HaveCoin(const COutPoint& outpoint) const final @@ -270,17 +261,16 @@ FUZZ_TARGET(coinscache_sim) // Look up in simulation data. auto sim = lookup(outpointidx); // Look up in real caches. - Coin realcoin; - auto real = caches.back()->GetCoin(data.outpoints[outpointidx], realcoin); + auto realcoin = caches.back()->GetCoin(data.outpoints[outpointidx]); // Compare results. if (!sim.has_value()) { - assert(!real || realcoin.IsSpent()); + assert(!realcoin || realcoin->IsSpent()); } else { - assert(real && !realcoin.IsSpent()); + assert(realcoin && !realcoin->IsSpent()); const auto& simcoin = data.coins[sim->first]; - assert(realcoin.out == simcoin.out); - assert(realcoin.fCoinBase == simcoin.fCoinBase); - assert(realcoin.nHeight == sim->second); + assert(realcoin->out == simcoin.out); + assert(realcoin->fCoinBase == simcoin.fCoinBase); + assert(realcoin->nHeight == sim->second); } }, @@ -465,16 +455,15 @@ FUZZ_TARGET(coinscache_sim) // Compare the bottom coinsview (not a CCoinsViewCache) with sim_cache[0]. for (uint32_t outpointidx = 0; outpointidx < NUM_OUTPOINTS; ++outpointidx) { - Coin realcoin; - bool real = bottom.GetCoin(data.outpoints[outpointidx], realcoin); + auto realcoin = bottom.GetCoin(data.outpoints[outpointidx]); auto sim = lookup(outpointidx, 0); if (!sim.has_value()) { - assert(!real || realcoin.IsSpent()); + assert(!realcoin || realcoin->IsSpent()); } else { - assert(real && !realcoin.IsSpent()); - assert(realcoin.out == data.coins[sim->first].out); - assert(realcoin.fCoinBase == data.coins[sim->first].fCoinBase); - assert(realcoin.nHeight == sim->second); + assert(realcoin && !realcoin->IsSpent()); + assert(realcoin->out == data.coins[sim->first].out); + assert(realcoin->fCoinBase == data.coins[sim->first].fCoinBase); + assert(realcoin->nHeight == sim->second); } } } diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 1e0c073562..394b54caf0 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -214,9 +214,8 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool) // Helper to query an amount const CCoinsViewMemPool amount_view{WITH_LOCK(::cs_main, return &chainstate.CoinsTip()), tx_pool}; const auto GetAmount = [&](const COutPoint& outpoint) { - Coin c; - Assert(amount_view.GetCoin(outpoint, c)); - return c.out.nValue.GetAmount(); + auto coin{amount_view.GetCoin(outpoint).value()}; + return coin.out.nValue.GetAmount(); }; LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 300) diff --git a/src/test/pegin_spent_tests.cpp b/src/test/pegin_spent_tests.cpp index 1bbc2a93b0..141f98b9fd 100644 --- a/src/test/pegin_spent_tests.cpp +++ b/src/test/pegin_spent_tests.cpp @@ -60,7 +60,7 @@ BOOST_AUTO_TEST_CASE(PeginSpent_validity) //Basic insert of blank outpoint pair, blank COutPoint allows for checking coinsCache std::pair outpoint = std::make_pair(GetRandHash(), COutPoint(Txid::FromUint256(GetRandHash()), 42)); - BOOST_CHECK(!coinsCache.GetCoin(outpoint.second, ret)); + BOOST_CHECK(!coinsCache.GetCoin(outpoint.second)); //Checking for pegin spentness should not create an entry BOOST_CHECK(!coinsCache.IsPeginSpent(outpoint)); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index b4dd1ad5c3..cd50f1a77b 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -455,9 +455,8 @@ std::pair TestChain100Setup::CreateValidTransactio std::map input_coins; CAmount inputs_amount{0}; for (const auto& outpoint_to_spend : inputs) { - // - Use GetCoin to properly populate utxo_to_spend, - Coin utxo_to_spend; - assert(coins_cache.GetCoin(outpoint_to_spend, utxo_to_spend)); + // Use GetCoin to properly populate utxo_to_spend + auto utxo_to_spend{coins_cache.GetCoin(outpoint_to_spend).value()}; input_coins.insert({outpoint_to_spend, utxo_to_spend}); inputs_amount += utxo_to_spend.out.nValue.GetAmount(); } diff --git a/src/txdb.cpp b/src/txdb.cpp index 8368c688a3..61295da801 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -71,8 +71,10 @@ void CCoinsViewDB::ResizeCache(size_t new_cache_size) } } -bool CCoinsViewDB::GetCoin(const COutPoint &outpoint, Coin &coin) const { - return m_db->Read(CoinEntry(&outpoint), coin); +std::optional CCoinsViewDB::GetCoin(const COutPoint& outpoint) const +{ + if (Coin coin; m_db->Read(CoinEntry(&outpoint), coin)) return coin; + return std::nullopt; } bool CCoinsViewDB::HaveCoin(const COutPoint &outpoint) const { diff --git a/src/txdb.h b/src/txdb.h index ec724954eb..0a7e847fde 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -57,7 +57,7 @@ protected: public: explicit CCoinsViewDB(DBParams db_params, CoinsViewOptions options); - bool GetCoin(const COutPoint &outpoint, Coin &coin) const override; + std::optional GetCoin(const COutPoint& outpoint) const override; bool HaveCoin(const COutPoint &outpoint) const override; uint256 GetBestBlock() const override; std::vector GetHeadBlocks() const override; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 019f3ca4d3..1180c3fb80 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1069,12 +1069,12 @@ bool CTxMemPool::HasNoInputsOf(const CTransaction &tx) const CCoinsViewMemPool::CCoinsViewMemPool(CCoinsView* baseIn, const CTxMemPool& mempoolIn) : CCoinsViewBacked(baseIn), mempool(mempoolIn) { } -bool CCoinsViewMemPool::GetCoin(const COutPoint &outpoint, Coin &coin) const { +std::optional CCoinsViewMemPool::GetCoin(const COutPoint& outpoint) const +{ // Check to see if the inputs are made available by another tx in the package. // These Coins would not be available in the underlying CoinsView. if (auto it = m_temp_added.find(outpoint); it != m_temp_added.end()) { - coin = it->second; - return true; + return it->second; } // If an entry in the mempool exists, always return that one, as it's guaranteed to never @@ -1083,14 +1083,13 @@ bool CCoinsViewMemPool::GetCoin(const COutPoint &outpoint, Coin &coin) const { CTransactionRef ptx = mempool.get(outpoint.hash); if (ptx) { if (outpoint.n < ptx->vout.size()) { - coin = Coin(ptx->vout[outpoint.n], MEMPOOL_HEIGHT, false); + Coin coin(ptx->vout[outpoint.n], MEMPOOL_HEIGHT, false); m_non_base_coins.emplace(outpoint); - return true; - } else { - return false; + return coin; } + return std::nullopt; } - return base->GetCoin(outpoint, coin); + return base->GetCoin(outpoint); } void CCoinsViewMemPool::PackageAddTransaction(const CTransactionRef& tx) diff --git a/src/txmempool.h b/src/txmempool.h index 9f826d8310..fe220546b5 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -908,7 +908,7 @@ public: CCoinsViewMemPool(CCoinsView* baseIn, const CTxMemPool& mempoolIn); /** GetCoin, returning whether it exists and is not spent. Also updates m_non_base_coins if the * coin is not fetched from base. */ - bool GetCoin(const COutPoint &outpoint, Coin &coin) const override; + std::optional GetCoin(const COutPoint& outpoint) const override; /** Add the coins created by this transaction. These coins are only temporarily stored in * m_temp_added and cannot be flushed to the back end. Only used for package validation. */ void PackageAddTransaction(const CTransactionRef& tx); diff --git a/src/validation.cpp b/src/validation.cpp index 0b32d0d1ce..7887e98d56 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -189,17 +189,14 @@ std::optional> CalculatePrevHeights( prev_heights[i] = -1; continue; } - Coin coin; - if (!coins.GetCoin(txin.prevout, coin)) { + if (auto coin{coins.GetCoin(txin.prevout)}) { + prev_heights[i] = coin->nHeight == MEMPOOL_HEIGHT + ? tip.nHeight + 1 // Assume all mempool transaction confirm in the next block. + : coin->nHeight; + } else { 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; }