diff --git a/src/chain.h b/src/chain.h index 4236e5a7adff..01efbef4ff08 100644 --- a/src/chain.h +++ b/src/chain.h @@ -96,7 +96,7 @@ enum BlockStatus : uint32_t { */ BLOCK_VALID_TRANSACTIONS = 3, - //! Outputs do not overspend inputs, no double spends, coinbase output ok, no immature coinbase spends, BIP30. + //! Outputs do not overspend inputs, no double spends, coinbase output ok, no immature coinbase spends. //! Implies all parents are also at least CHAIN. BLOCK_VALID_CHAIN = 4, diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp index 49c81ba1efbc..479198ed8629 100644 --- a/src/index/coinstatsindex.cpp +++ b/src/index/coinstatsindex.cpp @@ -144,13 +144,6 @@ bool CoinStatsIndex::WriteBlock(const CBlock& block, const CBlockIndex* pindex) for (size_t i = 0; i < block.vtx.size(); ++i) { const auto& tx{block.vtx.at(i)}; - // Skip duplicate txid coinbase transactions (BIP30). - if (IsBIP30Unspendable(*pindex) && tx->IsCoinBase()) { - m_total_unspendable_amount += block_subsidy; - m_total_unspendables_bip30 += block_subsidy; - continue; - } - for (uint32_t j = 0; j < tx->vout.size(); ++j) { const CTxOut& out{tx->vout[j]}; Coin coin{out, pindex->nHeight, tx->IsCoinBase()}; diff --git a/src/index/coinstatsindex.h b/src/index/coinstatsindex.h index b7ad12b76082..dbe2e3d87f0d 100644 --- a/src/index/coinstatsindex.h +++ b/src/index/coinstatsindex.h @@ -32,6 +32,7 @@ class CoinStatsIndex final : public BaseIndex CAmount m_total_new_outputs_ex_coinbase_amount{0}; CAmount m_total_coinbase_amount{0}; CAmount m_total_unspendables_genesis_block{0}; + //! There's no unspendable coinbase outputs in dash core. TODO: remove it with a version bump CAmount m_total_unspendables_bip30{0}; CAmount m_total_unspendables_scripts{0}; CAmount m_total_unspendables_unclaimed_rewards{0}; diff --git a/src/kernel/coinstats.h b/src/kernel/coinstats.h index c103966568f2..f1d7c118f59c 100644 --- a/src/kernel/coinstats.h +++ b/src/kernel/coinstats.h @@ -58,7 +58,7 @@ struct CCoinsStats { CAmount total_coinbase_amount{0}; //! The unspendable coinbase amount from the genesis block CAmount total_unspendables_genesis_block{0}; - //! The two unspendable coinbase outputs total amount caused by BIP30 + //! There's no unspendable coinbase outputs in dash core. TODO: remove it with a version bump CAmount total_unspendables_bip30{0}; //! Total cumulative amount of outputs sent to unspendable scripts (OP_RETURN for example) up to and including this block CAmount total_unspendables_scripts{0}; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 1803294f7b3c..4abf27d57e5c 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1171,7 +1171,6 @@ static RPCHelpMan gettxoutsetinfo() {RPCResult::Type::OBJ, "unspendables", "Detailed view of the unspendable categories", { {RPCResult::Type::STR_AMOUNT, "genesis_block", "The unspendable amount of the Genesis block subsidy"}, - {RPCResult::Type::STR_AMOUNT, "bip30", "Transactions overridden by duplicates (no longer possible with BIP30)"}, {RPCResult::Type::STR_AMOUNT, "scripts", "Amounts sent to scripts that are unspendable (for example OP_RETURN outputs)"}, {RPCResult::Type::STR_AMOUNT, "unclaimed_rewards", "Fee rewards that miners did not claim in their coinbase transaction"}, }} @@ -1275,7 +1274,6 @@ static RPCHelpMan gettxoutsetinfo() UniValue unspendables(UniValue::VOBJ); unspendables.pushKV("genesis_block", ValueFromAmount(stats.total_unspendables_genesis_block - prev_stats.total_unspendables_genesis_block)); - unspendables.pushKV("bip30", ValueFromAmount(stats.total_unspendables_bip30 - prev_stats.total_unspendables_bip30)); unspendables.pushKV("scripts", ValueFromAmount(stats.total_unspendables_scripts - prev_stats.total_unspendables_scripts)); unspendables.pushKV("unclaimed_rewards", ValueFromAmount(stats.total_unspendables_unclaimed_rewards - prev_stats.total_unspendables_unclaimed_rewards)); block_info.pushKV("unspendables", unspendables); @@ -2132,9 +2130,9 @@ static RPCHelpMan getblockstats() size_t out_size = GetSerializeSize(out, PROTOCOL_VERSION) + PER_UTXO_OVERHEAD; utxo_size_inc += out_size; - // The Genesis block and the repeated BIP30 block coinbases don't change the UTXO + // The Genesis block coinbase don't change the UTXO // set counts, so they have to be excluded from the statistics - if (pindex.nHeight == 0 || (IsBIP30Repeat(pindex) && tx->IsCoinBase())) continue; + if (pindex.nHeight == 0) continue; // Skip unspendable outputs since they are not included in the UTXO set if (out.scriptPubKey.IsUnspendable()) continue; diff --git a/src/validation.cpp b/src/validation.cpp index 89dfdaa75166..01e258225758 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2038,21 +2038,11 @@ 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;) { @@ -2081,9 +2071,7 @@ 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) { - if (!is_bip30_exception) { - fClean = false; // transaction output mismatch - } + fClean = false; // transaction output mismatch } } } @@ -2382,24 +2370,17 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, // can be duplicated to remove the ability to spend the first instance -- even after // being sent to another address. // See BIP30, CVE-2012-1909, and http://r6.ca/blog/20120206T005236Z.html for more information. - // This rule was originally applied to all blocks with a timestamp after March 15, 2012, 0:00 UTC. - // Now that the whole chain is irreversibly beyond that time it is applied to all blocks except the - // two in the chain that violate it. This prevents exploiting the issue against nodes during their + // This prevents exploiting the issue against nodes during their // initial block download. - bool fEnforceBIP30 = !IsBIP30Repeat(*pindex); // Once BIP34 activated it was not possible to create new duplicate coinbases and thus other than starting - // with the 2 existing duplicate coinbase pairs, not possible to create overwriting txs. But by the - // time BIP34 activated, in each of the existing pairs the duplicate coinbase had overwritten the first - // before the first had been spent. Since those coinbases are sufficiently buried it's no longer possible to create further - // duplicate transactions descending from the known pairs either. + // with the 2 existing duplicate coinbase pairs, not possible to create overwriting txs. // If we're on the known chain at height greater than where BIP34 activated, we can save the db accesses needed for the BIP30 check. assert(pindex->pprev); CBlockIndex* pindexBIP34height = pindex->pprev->GetAncestor(m_params.GetConsensus().BIP34Height); //Only continue to enforce if we're below BIP34 activation height or the block hash at that height doesn't correspond. - fEnforceBIP30 = fEnforceBIP30 && (!pindexBIP34height || !(pindexBIP34height->GetBlockHash() == m_params.GetConsensus().BIP34Hash)); - - if (fEnforceBIP30) { + bool fEnforceBIP30 = !pindexBIP34height || !(pindexBIP34height->GetBlockHash() == m_params.GetConsensus().BIP34Hash); + if (fEnforceBIP30 && pindex->nHeight <= m_params.GetConsensus().BIP34Height) { for (const auto& tx : block.vtx) { for (size_t o = 0; o < tx->vout.size(); o++) { if (view.HaveCoin(COutPoint(tx->GetHash(), o))) { @@ -6157,15 +6138,3 @@ ChainstateManager::~ChainstateManager() i.clear(); } } - -bool IsBIP30Repeat(const CBlockIndex& block_index) -{ - return (block_index.nHeight==91842 && block_index.GetBlockHash() == uint256S("0x00000000000a4d0a398161ffc163c503763b1f4360639393e0e4c8e300e0caec")) || - (block_index.nHeight==91880 && block_index.GetBlockHash() == uint256S("0x00000000000743f190a18c5577a3c2d2a1f610ae9601ac046a38084ccb7cd721")); -} - -bool IsBIP30Unspendable(const CBlockIndex& block_index) -{ - return (block_index.nHeight==91722 && block_index.GetBlockHash() == uint256S("0x00000000000271a2dc26e7667f8419f2e15416dc6955e5a6c6cdf3f2574dd08e")) || - (block_index.nHeight==91812 && block_index.GetBlockHash() == uint256S("0x00000000000af0aed4792b1acee3d966af36cf5def14935db8de83d6f9306f2f")); -} diff --git a/src/validation.h b/src/validation.h index 666773994265..ff36f0aeaf47 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1126,10 +1126,4 @@ bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mocka */ const AssumeutxoData* ExpectedAssumeutxo(const int height, const CChainParams& params); -/** Identifies blocks that overwrote an existing coinbase output in the UTXO set (see BIP30) */ -bool IsBIP30Repeat(const CBlockIndex& block_index); - -/** Identifies blocks which coinbase output was subsequently overwritten in the UTXO set (see BIP30) */ -bool IsBIP30Unspendable(const CBlockIndex& block_index); - #endif // BITCOIN_VALIDATION_H diff --git a/test/functional/feature_coinstatsindex.py b/test/functional/feature_coinstatsindex.py index cad3bc886a2a..b27f1b0b4759 100755 --- a/test/functional/feature_coinstatsindex.py +++ b/test/functional/feature_coinstatsindex.py @@ -135,7 +135,6 @@ def _test_coin_stats_index(self): 'coinbase': 0, 'unspendables': { 'genesis_block': 50, - 'bip30': 0, 'scripts': 0, 'unclaimed_rewards': 0 } @@ -152,7 +151,6 @@ def _test_coin_stats_index(self): 'coinbase': Decimal('500.00025500'), 'unspendables': { 'genesis_block': 0, - 'bip30': 0, 'scripts': 0, 'unclaimed_rewards': 0, } @@ -189,7 +187,6 @@ def _test_coin_stats_index(self): 'coinbase': Decimal('500.00101000'), 'unspendables': { 'genesis_block': 0, - 'bip30': 0, 'scripts': Decimal('20.99900000'), 'unclaimed_rewards': 0, } @@ -220,7 +217,6 @@ def _test_coin_stats_index(self): 'coinbase': 40, 'unspendables': { 'genesis_block': 0, - 'bip30': 0, 'scripts': 0, 'unclaimed_rewards': 460 }