diff --git a/src/consensus/params.h b/src/consensus/params.h index d104fa6aed828c..30f2de543c898b 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -35,6 +35,7 @@ enum DeploymentPos : uint16_t { DEPLOYMENT_CHECKTEMPLATEVERIFY, // Deployment of CTV (BIP 119) DEPLOYMENT_ANYPREVOUT, DEPLOYMENT_OP_CAT, + DEPLOYMENT_64BYTETX, // NOTE: Also add new deployments to VersionBitsDeploymentInfo in deploymentinfo.cpp MAX_VERSION_BITS_DEPLOYMENTS }; diff --git a/src/deploymentinfo.cpp b/src/deploymentinfo.cpp index 329c444977e6df..2340c4a2f0dba2 100644 --- a/src/deploymentinfo.cpp +++ b/src/deploymentinfo.cpp @@ -27,6 +27,10 @@ const struct VBDeploymentInfo VersionBitsDeploymentInfo[Consensus::MAX_VERSION_B /*.name =*/ "opcat", /*.gbt_force =*/ true, }, + { + /*.name =*/ "64bytetx", + /*.gbt_force =*/ true, + }, }; std::string DeploymentName(Consensus::BuriedDeployment dep) diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index 0970c084648420..c7597c1bdba472 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -472,6 +472,8 @@ class CRegTestParams : public CChainParams consensus.vDeployments[Consensus::DEPLOYMENT_CHECKTEMPLATEVERIFY] = SetupDeployment{.activate = 0x60007700, .abandon = 0x40007700, .always = true}; consensus.vDeployments[Consensus::DEPLOYMENT_ANYPREVOUT] = SetupDeployment{.activate = 0x60007600, .abandon = 0x40007600, .always = true}; consensus.vDeployments[Consensus::DEPLOYMENT_OP_CAT] = SetupDeployment{.activate = 0x62000100, .abandon = 0x42000100, .always = true}; + consensus.vDeployments[Consensus::DEPLOYMENT_64BYTETX] = SetupDeployment{.activate = 0x60000001, .abandon = 0x40000001, .always = true}; // needs BIN assigned + consensus.nMinimumChainWork = uint256{}; consensus.defaultAssumeValid = uint256{}; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 66c91237633417..6080e5939cd305 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1334,6 +1334,7 @@ UniValue DeploymentInfo(const CBlockIndex* blockindex, const ChainstateManager& SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_CHECKTEMPLATEVERIFY); SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_ANYPREVOUT); SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_OP_CAT); + SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_64BYTETX); return softforks; } } // anon namespace diff --git a/src/validation.cpp b/src/validation.cpp index c71c860b3dddbe..9ba7a338d92b75 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2175,6 +2175,7 @@ unsigned int GetBlockScriptFlags(const CBlockIndex& block_index, const Chainstat return flags; } +static bool ContextualBlockPreCheck(const CBlock& block, BlockValidationState& state, const ChainstateManager& chainman, const CBlockIndex* pindexPrev); static SteadyClock::duration time_check{}; static SteadyClock::duration time_forks{}; @@ -2201,6 +2202,10 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, const auto time_start{SteadyClock::now()}; const CChainParams& params{m_chainman.GetParams()}; + if (!ContextualBlockPreCheck(block, state, m_chainman, pindex->pprev)) { + return error("%s: Consensus::ContextualBlockPreCheck: %s", __func__, state.ToString()); + } + // Check it again in case a previous version let a bad block in // NOTE: We don't currently (re-)invoke ContextualCheckBlock() or // ContextualCheckBlockHeader() here. This means that if we add a new @@ -3925,6 +3930,28 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio return true; } +/** + * We want to enforce certain rules (specifically the 64-byte transaction check) + * before we call CheckBlock to check the merkle root. This allows us to enforce + * malleability checks which may interact with other CheckBlock checks. + * This is currently called both in AcceptBlock prior to writing the block to + * disk and in ConnectBlock. + * Note that as this is called before merkle-tree checks so must never return a + * non-malleable error condition. + */ +static bool ContextualBlockPreCheck(const CBlock& block, BlockValidationState& state, const ChainstateManager& chainman, const CBlockIndex* pindexPrev) +{ + if (DeploymentActiveAfter(pindexPrev, chainman, Consensus::DEPLOYMENT_64BYTETX)) { + for (const auto& tx : block.vtx) { + if (::GetSerializeSize(TX_NO_WITNESS(tx)) == 64) { + return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "64-byte-transaction", strprintf("size of tx %s without witness is 64 bytes", tx->GetHash().ToString())); + } + } + } + + return true; +} + /** NOTE: This function is not currently invoked by ConnectBlock(), so we * should consider upgrade issues if we change which consensus rules are * enforced in this function (eg by adding a new consensus rule). See comment @@ -4205,7 +4232,8 @@ bool ChainstateManager::AcceptBlock(const std::shared_ptr& pblock, const CChainParams& params{GetParams()}; - if (!CheckBlock(block, state, params.GetConsensus()) || + if (!ContextualBlockPreCheck(block, state, *this, pindex->pprev) || + !CheckBlock(block, state, params.GetConsensus()) || !ContextualCheckBlock(block, state, *this, pindex->pprev)) { if (state.IsInvalid() && state.GetResult() != BlockValidationResult::BLOCK_MUTATED) { pindex->nStatus |= BLOCK_FAILED_VALID; @@ -4325,6 +4353,8 @@ bool TestBlockValidity(BlockValidationState& state, // NOTE: CheckBlockHeader is called by CheckBlock if (!ContextualCheckBlockHeader(block, state, chainstate.m_blockman, chainstate.m_chainman, pindexPrev)) return error("%s: Consensus::ContextualCheckBlockHeader: %s", __func__, state.ToString()); + if (!ContextualBlockPreCheck(block, state, chainstate.m_chainman, pindexPrev)) + return error("%s: Consensus::ContextualBlockPreCheck: %s", __func__, state.ToString()); if (!CheckBlock(block, state, chainparams.GetConsensus(), fCheckPOW, fCheckMerkleRoot)) return error("%s: Consensus::CheckBlock: %s", __func__, state.ToString()); if (!ContextualCheckBlock(block, state, chainstate.m_chainman, pindexPrev)) @@ -4441,6 +4471,11 @@ VerifyDBResult CVerifyDB::VerifyDB( return VerifyDBResult::CORRUPTED_BLOCK_DB; } // check level 1: verify block validity + if (nCheckLevel >= 1 && !ContextualBlockPreCheck(block, state, chainstate.m_chainman, pindex->pprev)) { + LogPrintf("Verification error: found bad block at %d due to soft-fork, hash=%s (%s)\n", + pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString()); + return VerifyDBResult::CORRUPTED_BLOCK_DB; + } if (nCheckLevel >= 1 && !CheckBlock(block, state, consensus_params)) { LogPrintf("Verification error: found bad block at %d, hash=%s (%s)\n", pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString()); diff --git a/test/functional/data/invalid_txs.py b/test/functional/data/invalid_txs.py index 457f3f932f728e..111cf8e324da44 100755 --- a/test/functional/data/invalid_txs.py +++ b/test/functional/data/invalid_txs.py @@ -76,6 +76,9 @@ class BadTxTemplate: # the mempool (i.e. does it violate policy but not consensus)? valid_in_block = False + # Do we need a signature for this tx + wants_signature = True + def __init__(self, *, spend_tx=None, spend_block=None): self.spend_tx = spend_block.vtx[0] if spend_block else spend_tx self.spend_avail = sum(o.nValue for o in self.spend_tx.vout) @@ -101,6 +104,7 @@ def get_tx(self): class InputMissing(BadTxTemplate): reject_reason = "bad-txns-vin-empty" expect_disconnect = True + wants_signature = False # We use a blank transaction here to make sure # it is interpreted as a non-witness transaction. @@ -118,7 +122,9 @@ def get_tx(self): class SizeExactly64(BadTxTemplate): reject_reason = "tx-size-small" expect_disconnect = False - valid_in_block = True + valid_in_block = False + wants_signature = False + block_reject_reason = "64-byte-transaction" def get_tx(self): tx = CTransaction() @@ -133,6 +139,7 @@ class SizeSub64(BadTxTemplate): reject_reason = "tx-size-small" expect_disconnect = False valid_in_block = True + wants_signature = False def get_tx(self): tx = CTransaction() diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 8a95975184b11e..c89ef8c2596809 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -160,7 +160,7 @@ def run_test(self): blockname = f"for_invalid.{TxTemplate.__name__}" self.next_block(blockname) badtx = template.get_tx() - if TxTemplate != invalid_txs.InputMissing: + if template.wants_signature: self.sign_tx(badtx, attempt_spend_tx) badtx.rehash() badblock = self.update_block(blockname, [badtx]) diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 87477b30defca1..b78db89059ce25 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -261,6 +261,20 @@ def check_signalling_deploymentinfo_result(self, gdi_result, height, blockhash): 'height': 0, 'active': True, }, + '64bytetx': { + 'type': 'heretical', + 'heretical': { + 'binana-id': 'BIN-2016-0000-001', + 'start_time': -1, + 'timeout': 0x7fffffffffffffff, # testdummy does not have a timeout so is set to the max int64 value + 'period': 144, + 'status': 'active', + 'status_next': 'active', + 'since': 0, + }, + 'active': True, + 'height': 0, + }, } })