diff --git a/src/consensus/params.h b/src/consensus/params.h index 628fb858c2f6d2..85e2621afc6c73 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 8b5736f17b18c4..34ba00c5186512 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 71dcbe30db6d1a..73ee7a619507aa 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -584,6 +584,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 ef05d4c3d2d2cd..b37a358f48e4c2 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1396,6 +1396,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 a3c0c741d5e8f1..6f14bfac3d16e7 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2401,6 +2401,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); /** Apply the effects of this block (with given index) on the UTXO set represented by coins. * Validity checks that depend on the UTXO set are also done; ConnectBlock() @@ -2418,6 +2419,11 @@ 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)) { + LogError("%s: Consensus::ContextualBlockPreCheck: %s", __func__, state.ToString()); + return false; + } + // 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 @@ -4196,6 +4202,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 @@ -4478,7 +4506,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; @@ -4613,6 +4642,10 @@ bool TestBlockValidity(BlockValidationState& state, LogError("%s: Consensus::ContextualCheckBlockHeader: %s\n", __func__, state.ToString()); return false; } + if (!ContextualBlockPreCheck(block, state, chainstate.m_chainman, pindexPrev)) { + LogError("%s: Consensus::ContextualBlockPreCheck: %s", __func__, state.ToString()); + return false; + } if (!CheckBlock(block, state, chainparams.GetConsensus(), fCheckPOW, fCheckMerkleRoot)) { LogError("%s: Consensus::CheckBlock: %s\n", __func__, state.ToString()); return false; @@ -4733,6 +4766,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 01e2aa21530753..7217d05c2534e6 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 384ca311c74e55..ce862184e5909e 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -159,7 +159,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 f54fd0851b1d5e..767206f2e8852f 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, + }, } })