diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp index ae96e3d6e7..7c131e3b42 100644 --- a/src/index/coinstatsindex.cpp +++ b/src/index/coinstatsindex.cpp @@ -39,7 +39,6 @@ struct DBVal { CAmount total_new_outputs_ex_coinbase_amount; CAmount total_coinbase_amount; CAmount total_unspendables_genesis_block; - CAmount total_unspendables_bip30; CAmount total_unspendables_scripts; CAmount total_unspendables_unclaimed_rewards; @@ -55,7 +54,6 @@ struct DBVal { READWRITE(obj.total_new_outputs_ex_coinbase_amount); READWRITE(obj.total_coinbase_amount); READWRITE(obj.total_unspendables_genesis_block); - READWRITE(obj.total_unspendables_bip30); READWRITE(obj.total_unspendables_scripts); READWRITE(obj.total_unspendables_unclaimed_rewards); } @@ -150,13 +148,6 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block) for (size_t i = 0; i < block.data->vtx.size(); ++i) { const auto& tx{block.data->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, block.height, tx->IsCoinBase(), tx->IsCoinStake(), (int)tx->nTime}; @@ -225,7 +216,6 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block) value.second.total_new_outputs_ex_coinbase_amount = m_total_new_outputs_ex_coinbase_amount; value.second.total_coinbase_amount = m_total_coinbase_amount; value.second.total_unspendables_genesis_block = m_total_unspendables_genesis_block; - value.second.total_unspendables_bip30 = m_total_unspendables_bip30; value.second.total_unspendables_scripts = m_total_unspendables_scripts; value.second.total_unspendables_unclaimed_rewards = m_total_unspendables_unclaimed_rewards; @@ -340,7 +330,6 @@ std::optional CoinStatsIndex::LookUpStats(const CBlockIndex& block_ stats.total_new_outputs_ex_coinbase_amount = entry.total_new_outputs_ex_coinbase_amount; stats.total_coinbase_amount = entry.total_coinbase_amount; stats.total_unspendables_genesis_block = entry.total_unspendables_genesis_block; - stats.total_unspendables_bip30 = entry.total_unspendables_bip30; stats.total_unspendables_scripts = entry.total_unspendables_scripts; stats.total_unspendables_unclaimed_rewards = entry.total_unspendables_unclaimed_rewards; @@ -382,7 +371,6 @@ bool CoinStatsIndex::CustomInit(const std::optional& block m_total_new_outputs_ex_coinbase_amount = entry.total_new_outputs_ex_coinbase_amount; m_total_coinbase_amount = entry.total_coinbase_amount; m_total_unspendables_genesis_block = entry.total_unspendables_genesis_block; - m_total_unspendables_bip30 = entry.total_unspendables_bip30; m_total_unspendables_scripts = entry.total_unspendables_scripts; m_total_unspendables_unclaimed_rewards = entry.total_unspendables_unclaimed_rewards; } @@ -495,7 +483,6 @@ bool CoinStatsIndex::ReverseBlock(const CBlock& block, const CBlockIndex* pindex Assert(m_total_new_outputs_ex_coinbase_amount == read_out.second.total_new_outputs_ex_coinbase_amount); Assert(m_total_coinbase_amount == read_out.second.total_coinbase_amount); Assert(m_total_unspendables_genesis_block == read_out.second.total_unspendables_genesis_block); - Assert(m_total_unspendables_bip30 == read_out.second.total_unspendables_bip30); Assert(m_total_unspendables_scripts == read_out.second.total_unspendables_scripts); Assert(m_total_unspendables_unclaimed_rewards == read_out.second.total_unspendables_unclaimed_rewards); diff --git a/src/index/coinstatsindex.h b/src/index/coinstatsindex.h index acb065ac15..9c114bf85c 100644 --- a/src/index/coinstatsindex.h +++ b/src/index/coinstatsindex.h @@ -34,7 +34,6 @@ 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}; - 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 54d0e4f664..8e66ad3289 100644 --- a/src/kernel/coinstats.h +++ b/src/kernel/coinstats.h @@ -59,8 +59,6 @@ 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 - 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}; //! Total cumulative amount of coins lost due to unclaimed miner rewards up to and including this block diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 758ae08e5d..b1496685d0 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -872,7 +872,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"}, }} @@ -976,7 +975,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); @@ -1813,9 +1811,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 doesn'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 ece854b5e1..348df95d27 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1942,22 +1942,12 @@ DisconnectResult Chainstate::DisconnectBlock(const CBlock& block, const CBlockIn 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_coinstake = tx.IsCoinStake(); - bool is_bip30_exception = (is_coinbase && !fEnforceBIP30); // Check that all outputs are available and match the outputs in the block itself // exactly. @@ -1967,9 +1957,7 @@ DisconnectResult Chainstate::DisconnectBlock(const CBlock& block, const CBlockIn 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 || is_coinstake != coin.fCoinStake) { - if (!is_bip30_exception) { fClean = false; // transaction output mismatch - } } } } @@ -2203,94 +2191,6 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, Ticks(time_check), Ticks(time_check) / num_blocks_total); - /* - // Blackcoin - // Do not allow blocks that contain transactions which 'overwrite' older transactions, - // unless those are already completely spent. - // If such overwrites are allowed, coinbases and transactions depending upon those - // 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 - // 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. - // 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. - - // BIP34 requires that a block at height X (block X) has its coinbase - // scriptSig start with a CScriptNum of X (indicated height X). The above - // logic of no longer requiring BIP30 once BIP34 activates is flawed in the - // case that there is a block X before the BIP34 height of 227,931 which has - // an indicated height Y where Y is greater than X. The coinbase for block - // X would also be a valid coinbase for block Y, which could be a BIP30 - // violation. An exhaustive search of all mainnet coinbases before the - // BIP34 height which have an indicated height greater than the block height - // reveals many occurrences. The 3 lowest indicated heights found are - // 209,921, 490,897, and 1,983,702 and thus coinbases for blocks at these 3 - // heights would be the first opportunity for BIP30 to be violated. - - // The search reveals a great many blocks which have an indicated height - // greater than 1,983,702, so we simply remove the optimization to skip - // BIP30 checking for blocks at height 1,983,702 or higher. Before we reach - // that block in another 25 years or so, we should take advantage of a - // future consensus change to do a new and improved version of BIP34 that - // will actually prevent ever creating any duplicate coinbases in the - // future. - static constexpr int BIP34_IMPLIES_BIP30_LIMIT = 1983702; - - // There is no potential to create a duplicate coinbase at block 209,921 - // because this is still before the BIP34 height and so explicit BIP30 - // checking is still active. - - // The final case is block 176,684 which has an indicated height of - // 490,897. Unfortunately, this issue was not discovered until about 2 weeks - // before block 490,897 so there was not much opportunity to address this - // case other than to carefully analyze it and determine it would not be a - // problem. Block 490,897 was, in fact, mined with a different coinbase than - // block 176,684, but it is important to note that even if it hadn't been or - // is remined on an alternate fork with a duplicate coinbase, we would still - // not run into a BIP30 violation. This is because the coinbase for 176,684 - // is spent in block 185,956 in transaction - // d4f7fbbf92f4a3014a230b2dc70b8058d02eb36ac06b4a0736d9d60eaa9e8781. This - // spending transaction can't be duplicated because it also spends coinbase - // 0328dd85c331237f18e781d692c92de57649529bd5edf1d01036daea32ffde29. This - // coinbase has an indicated height of over 4.2 billion, and wouldn't be - // duplicatable until that height, and it's currently impossible to create a - // chain that long. Nevertheless we may wish to consider a future soft fork - // which retroactively prevents block 490,897 from creating a duplicate - // coinbase. The two historical BIP30 violations often provide a confusing - // edge case when manipulating the UTXO and it would be simpler not to have - // another edge case to deal with. - - // testnet3 has no blocks before the BIP34 height with indicated heights - // post BIP34 before approximately height 486,000,000. After block - // 1,983,702 testnet3 starts doing unnecessary BIP30 checking again. - assert(pindex->pprev); - CBlockIndex* pindexBIP34height = pindex->pprev->GetAncestor(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() == params.GetConsensus().BIP34Hash)); - - // TODO: Remove BIP30 checking from block height 1,983,702 on, once we have a - // consensus change that ensures coinbases at those heights cannot - // duplicate earlier coinbases. - if (fEnforceBIP30 || pindex->nHeight >= BIP34_IMPLIES_BIP30_LIMIT) { - for (const auto& tx : block.vtx) { - for (size_t o = 0; o < tx->vout.size(); o++) { - if (view.HaveCoin(COutPoint(tx->GetHash(), o))) { - LogPrintf("ERROR: ConnectBlock(): tried to overwrite transaction\n"); - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-txns-BIP30"); - } - } - } - */ - // BIP30 and BIP34 are always active for (const auto& tx : block.vtx) { for (size_t o = 0; o < tx->vout.size(); o++) { @@ -2301,7 +2201,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, } } - // Enforce BIP68 (sequence locks) + // Enforce BIP68 (sequence locks) and BIP112 (CHECKSEQUENCEVERIFY) int nLockTimeFlags = 0; // if (DeploymentActiveAt(*pindex, m_chainman, Consensus::DEPLOYMENT_CSV)) { if (params.GetConsensus().IsProtocolV3_1(pindex->GetBlockTime())) { @@ -5823,18 +5723,6 @@ Chainstate& ChainstateManager::ActivateExistingSnapshot(CTxMemPool* mempool, uin return *m_snapshot_chainstate; } -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")); -} - void Chainstate::InvalidateCoinsDBOnDisk() { AssertLockHeld(::cs_main); diff --git a/src/validation.h b/src/validation.h index 9fce283f85..45c8df422f 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1203,10 +1203,4 @@ bool DeploymentEnabled(const ChainstateManager& chainman, DEP dep) */ 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