Relax MIN_STANDARD_TX_NONWITNESS_SIZE to 64 non-witness bytes exactly

Restricting sizes below 64 vbytes interferes with specific
use-cases, and does not interfere with proposed consensus
changes such as the Great Consensus Cleanup of 2025. We
change software to protect against the case we care about:
64 bytes
This commit is contained in:
Greg Sanders 2022-10-05 14:50:30 -04:00
parent 74c23f80ab
commit 72ddf4237b
7 changed files with 45 additions and 43 deletions

View file

@ -32,8 +32,8 @@ static constexpr unsigned int MINIMUM_BLOCK_RESERVED_WEIGHT{2000};
static constexpr unsigned int DEFAULT_BLOCK_MIN_TX_FEE{1000}; static constexpr unsigned int DEFAULT_BLOCK_MIN_TX_FEE{1000};
/** The maximum weight for transactions we're willing to relay/mine */ /** The maximum weight for transactions we're willing to relay/mine */
static constexpr int32_t MAX_STANDARD_TX_WEIGHT{400000}; static constexpr int32_t MAX_STANDARD_TX_WEIGHT{400000};
/** The minimum non-witness size for transactions we're willing to relay/mine: one larger than 64 */ /** In addition to transactions that are too big, do not relay/mine ones that are 64 non-witness bytes exactly: CVE-2017-12842 */
static constexpr unsigned int MIN_STANDARD_TX_NONWITNESS_SIZE{65}; static constexpr unsigned int NONSTANDARD_TX_NONWITNESS_SIZE{64};
/** Maximum number of signature check operations in an IsStandard() P2SH script */ /** Maximum number of signature check operations in an IsStandard() P2SH script */
static constexpr unsigned int MAX_P2SH_SIGOPS{15}; static constexpr unsigned int MAX_P2SH_SIGOPS{15};
/** The maximum number of sigops we're willing to relay/mine in a single tx */ /** The maximum number of sigops we're willing to relay/mine in a single tx */

View file

@ -794,9 +794,10 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
return state.Invalid(TxValidationResult::TX_NOT_STANDARD, reason); return state.Invalid(TxValidationResult::TX_NOT_STANDARD, reason);
} }
// Transactions smaller than 65 non-witness bytes are not relayed to mitigate CVE-2017-12842. // Transactions exactly 64 non-witness bytes are not relayed to mitigate CVE-2017-12842.
if (::GetSerializeSize(TX_NO_WITNESS(tx)) < MIN_STANDARD_TX_NONWITNESS_SIZE) if (::GetSerializeSize(TX_NO_WITNESS(tx)) == NONSTANDARD_TX_NONWITNESS_SIZE) {
return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "tx-size-small"); return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "tx-bad-nonwit-size");
}
// Only accept nLockTime-using transactions that can be mined in the next // Only accept nLockTime-using transactions that can be mined in the next
// block; we don't want our mempool filled up with transactions that can't // block; we don't want our mempool filled up with transactions that can't

View file

@ -1027,7 +1027,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
coin_selection_params.m_avoid_partial_spends = coin_control.m_avoid_partial_spends; coin_selection_params.m_avoid_partial_spends = coin_control.m_avoid_partial_spends;
coin_selection_params.m_include_unsafe_inputs = coin_control.m_include_unsafe_inputs; coin_selection_params.m_include_unsafe_inputs = coin_control.m_include_unsafe_inputs;
coin_selection_params.m_max_tx_weight = coin_control.m_max_tx_weight.value_or(MAX_STANDARD_TX_WEIGHT); coin_selection_params.m_max_tx_weight = coin_control.m_max_tx_weight.value_or(MAX_STANDARD_TX_WEIGHT);
int minimum_tx_weight = MIN_STANDARD_TX_NONWITNESS_SIZE * WITNESS_SCALE_FACTOR; int minimum_tx_weight = (NONSTANDARD_TX_NONWITNESS_SIZE + 1) * WITNESS_SCALE_FACTOR;
if (coin_selection_params.m_max_tx_weight.value() < minimum_tx_weight || coin_selection_params.m_max_tx_weight.value() > MAX_STANDARD_TX_WEIGHT) { if (coin_selection_params.m_max_tx_weight.value() < minimum_tx_weight || coin_selection_params.m_max_tx_weight.value() > MAX_STANDARD_TX_WEIGHT) {
return util::Error{strprintf(_("Maximum transaction weight must be between %d and %d"), minimum_tx_weight, MAX_STANDARD_TX_WEIGHT)}; return util::Error{strprintf(_("Maximum transaction weight must be between %d and %d"), minimum_tx_weight, MAX_STANDARD_TX_WEIGHT)};
} }

View file

@ -46,15 +46,14 @@ from test_framework.script import (
OP_MOD, OP_MOD,
OP_MUL, OP_MUL,
OP_OR, OP_OR,
OP_RETURN,
OP_RIGHT, OP_RIGHT,
OP_RSHIFT, OP_RSHIFT,
OP_SUBSTR, OP_SUBSTR,
OP_XOR, OP_XOR,
) )
from test_framework.script_util import ( from test_framework.script_util import (
MIN_PADDING, NONSTANDARD_OP_RETURN_SCRIPT,
MIN_STANDARD_TX_NONWITNESS_SIZE, NONSTANDARD_TX_NONWITNESS_SIZE,
script_to_p2sh_script, script_to_p2sh_script,
) )
basic_p2sh = script_to_p2sh_script(CScript([OP_0])) basic_p2sh = script_to_p2sh_script(CScript([OP_0]))
@ -115,17 +114,16 @@ class InputMissing(BadTxTemplate):
# The following check prevents exploit of lack of merkle # The following check prevents exploit of lack of merkle
# tree depth commitment (CVE-2017-12842) # tree depth commitment (CVE-2017-12842)
class SizeTooSmall(BadTxTemplate): class BadNonWitSize(BadTxTemplate):
reject_reason = "tx-size-small" reject_reason = "tx-bad-nonwit-size"
expect_disconnect = False expect_disconnect = False
valid_in_block = True valid_in_block = True
def get_tx(self): def get_tx(self):
tx = CTransaction() tx = CTransaction()
tx.vin.append(self.valid_txin) tx.vin.append(self.valid_txin)
tx.vout.append(CTxOut(0, CScript([OP_RETURN] + ([OP_0] * (MIN_PADDING - 2))))) tx.vout.append(CTxOut(0, NONSTANDARD_OP_RETURN_SCRIPT))
assert len(tx.serialize_without_witness()) == 64 assert len(tx.serialize_without_witness()) == NONSTANDARD_TX_NONWITNESS_SIZE
assert MIN_STANDARD_TX_NONWITNESS_SIZE - 1 == 64
tx.calc_sha256() tx.calc_sha256()
return tx return tx

View file

@ -33,17 +33,16 @@ from test_framework.script import (
sign_input_legacy, sign_input_legacy,
) )
from test_framework.script_util import ( from test_framework.script_util import (
DUMMY_MIN_OP_RETURN_SCRIPT, INVALID_SPK_LEN,
keys_to_multisig_script, keys_to_multisig_script,
MIN_PADDING, NONSTANDARD_OP_RETURN_SCRIPT,
MIN_STANDARD_TX_NONWITNESS_SIZE, NONSTANDARD_TX_NONWITNESS_SIZE,
PAY_TO_ANCHOR, PAY_TO_ANCHOR,
script_to_p2sh_script, script_to_p2sh_script,
script_to_p2wsh_script, script_to_p2wsh_script,
) )
from test_framework.util import ( from test_framework.util import (
assert_equal, assert_equal,
assert_greater_than,
assert_raises_rpc_error, assert_raises_rpc_error,
sync_txindex, sync_txindex,
) )
@ -352,30 +351,35 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
maxfeerate=0, maxfeerate=0,
) )
# Prep for tiny-tx tests with wsh(OP_TRUE) output # Prep for tiny-tx tests with MiniWallet anyone-can-spend output
seed_tx = self.wallet.send_to(from_node=node, scriptPubKey=script_to_p2wsh_script(CScript([OP_TRUE])), amount=COIN) seed_utxo = self.wallet.send_self_transfer(from_node=node)["new_utxo"]
self.generate(node, 1) self.generate(node, 1)
self.log.info('A tiny transaction(in non-witness bytes) that is disallowed') self.log.info('A tiny transaction(in non-witness bytes) that is disallowed')
tx = CTransaction() tx = self.wallet.create_self_transfer(utxo_to_spend=seed_utxo)["tx"]
tx.vin.append(CTxIn(COutPoint(int(seed_tx["txid"], 16), seed_tx["sent_vout"]), b"", SEQUENCE_FINAL)) tx.vout[0].scriptPubKey = NONSTANDARD_OP_RETURN_SCRIPT
tx.wit.vtxinwit = [CTxInWitness()]
tx.wit.vtxinwit[0].scriptWitness.stack = [CScript([OP_TRUE])]
tx.vout.append(CTxOut(0, CScript([OP_RETURN] + ([OP_0] * (MIN_PADDING - 2)))))
# Note it's only non-witness size that matters! # Note it's only non-witness size that matters!
assert_equal(len(tx.serialize_without_witness()), 64) assert_equal(len(tx.serialize_without_witness()), NONSTANDARD_TX_NONWITNESS_SIZE)
assert_equal(MIN_STANDARD_TX_NONWITNESS_SIZE - 1, 64) assert len(tx.serialize()) != NONSTANDARD_TX_NONWITNESS_SIZE
assert_greater_than(len(tx.serialize()), 64)
self.check_mempool_result( self.check_mempool_result(
result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'tx-size-small'}], result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'tx-bad-nonwit-size'}],
rawtxs=[tx.serialize().hex()], rawtxs=[tx.serialize().hex()],
maxfeerate=0, maxfeerate=0,
) )
self.log.info('Minimally-small transaction(in non-witness bytes) that is allowed') self.log.info('Just-below size transaction(in non-witness bytes) that is allowed')
tx.vout[0] = CTxOut(COIN - 1000, DUMMY_MIN_OP_RETURN_SCRIPT) tx.vout[0] = CTxOut(int(seed_utxo["value"] * COIN) - 1000, CScript([OP_RETURN] + ([OP_0] * (INVALID_SPK_LEN - 2))))
assert_equal(len(tx.serialize_without_witness()), MIN_STANDARD_TX_NONWITNESS_SIZE) assert_equal(len(tx.serialize_without_witness()), NONSTANDARD_TX_NONWITNESS_SIZE - 1)
self.check_mempool_result(
result_expected=[{'txid': tx.rehash(), 'allowed': True, 'vsize': tx.get_vsize(), 'fees': { 'base': Decimal('0.00001000')}}],
rawtxs=[tx.serialize().hex()],
maxfeerate=0,
)
self.log.info('Just-above size transaction(in non-witness bytes) that is allowed')
tx.vout[0] = CTxOut(int(seed_utxo["value"] * COIN) - 1000, CScript([OP_RETURN] + ([OP_0] * (INVALID_SPK_LEN))))
assert_equal(len(tx.serialize_without_witness()), NONSTANDARD_TX_NONWITNESS_SIZE + 1)
self.check_mempool_result( self.check_mempool_result(
result_expected=[{'txid': tx.rehash(), 'allowed': True, 'vsize': tx.get_vsize(), 'fees': { 'base': Decimal('0.00001000')}}], result_expected=[{'txid': tx.rehash(), 'allowed': True, 'vsize': tx.get_vsize(), 'fees': { 'base': Decimal('0.00001000')}}],
rawtxs=[tx.serialize().hex()], rawtxs=[tx.serialize().hex()],

View file

@ -34,7 +34,7 @@ from test_framework.psbt import (
PSBT_OUT_TAP_TREE, PSBT_OUT_TAP_TREE,
) )
from test_framework.script import CScript, OP_TRUE from test_framework.script import CScript, OP_TRUE
from test_framework.script_util import MIN_STANDARD_TX_NONWITNESS_SIZE from test_framework.script_util import NONSTANDARD_TX_NONWITNESS_SIZE
from test_framework.test_framework import BitcoinTestFramework from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import ( from test_framework.util import (
assert_approx, assert_approx,
@ -215,7 +215,7 @@ class PSBTTest(BitcoinTestFramework):
self.log.info("Test for invalid maximum transaction weights") self.log.info("Test for invalid maximum transaction weights")
dest_arg = [{self.nodes[0].getnewaddress(): 1}] dest_arg = [{self.nodes[0].getnewaddress(): 1}]
min_tx_weight = MIN_STANDARD_TX_NONWITNESS_SIZE * WITNESS_SCALE_FACTOR min_tx_weight = (NONSTANDARD_TX_NONWITNESS_SIZE + 1) * WITNESS_SCALE_FACTOR
assert_raises_rpc_error(-4, f"Maximum transaction weight must be between {min_tx_weight} and {MAX_STANDARD_TX_WEIGHT}", self.nodes[0].walletcreatefundedpsbt, [], dest_arg, 0, {"max_tx_weight": -1}) assert_raises_rpc_error(-4, f"Maximum transaction weight must be between {min_tx_weight} and {MAX_STANDARD_TX_WEIGHT}", self.nodes[0].walletcreatefundedpsbt, [], dest_arg, 0, {"max_tx_weight": -1})
assert_raises_rpc_error(-4, f"Maximum transaction weight must be between {min_tx_weight} and {MAX_STANDARD_TX_WEIGHT}", self.nodes[0].walletcreatefundedpsbt, [], dest_arg, 0, {"max_tx_weight": 0}) assert_raises_rpc_error(-4, f"Maximum transaction weight must be between {min_tx_weight} and {MAX_STANDARD_TX_WEIGHT}", self.nodes[0].walletcreatefundedpsbt, [], dest_arg, 0, {"max_tx_weight": 0})
assert_raises_rpc_error(-4, f"Maximum transaction weight must be between {min_tx_weight} and {MAX_STANDARD_TX_WEIGHT}", self.nodes[0].walletcreatefundedpsbt, [], dest_arg, 0, {"max_tx_weight": MAX_STANDARD_TX_WEIGHT + 1}) assert_raises_rpc_error(-4, f"Maximum transaction weight must be between {min_tx_weight} and {MAX_STANDARD_TX_WEIGHT}", self.nodes[0].walletcreatefundedpsbt, [], dest_arg, 0, {"max_tx_weight": MAX_STANDARD_TX_WEIGHT + 1})

View file

@ -22,8 +22,8 @@ from test_framework.script import (
sha256, sha256,
) )
# To prevent a "tx-size-small" policy rule error, a transaction has to have a # To prevent a "tx-bad-nonwit-size" policy rule error, a transaction may not have a
# non-witness size of at least 65 bytes (MIN_STANDARD_TX_NONWITNESS_SIZE in # non-witness size of 64 bytes (NONSTANDARD_TX_NONWITNESS_SIZE in
# src/policy/policy.h). Considering a Tx with the smallest possible single # src/policy/policy.h). Considering a Tx with the smallest possible single
# input (blank, empty scriptSig), and with an output omitting the scriptPubKey, # input (blank, empty scriptSig), and with an output omitting the scriptPubKey,
# we get to a minimum size of 60 bytes: # we get to a minimum size of 60 bytes:
@ -32,16 +32,15 @@ from test_framework.script import (
# Blank Input: 32 [PrevTxHash] + 4 [Index] + 1 [scriptSigLen] + 4 [SeqNo] = 41 bytes # Blank Input: 32 [PrevTxHash] + 4 [Index] + 1 [scriptSigLen] + 4 [SeqNo] = 41 bytes
# Output: 8 [Amount] + 1 [scriptPubKeyLen] = 9 bytes # Output: 8 [Amount] + 1 [scriptPubKeyLen] = 9 bytes
# #
# Hence, the scriptPubKey of the single output has to have a size of at # Hence, the scriptPubKey of the single output MUST NOT have a size of 4.
# least 5 bytes. NONSTANDARD_TX_NONWITNESS_SIZE = 64
MIN_STANDARD_TX_NONWITNESS_SIZE = 65 INVALID_SPK_LEN = NONSTANDARD_TX_NONWITNESS_SIZE - 10 - 41 - 9
MIN_PADDING = MIN_STANDARD_TX_NONWITNESS_SIZE - 10 - 41 - 9 assert INVALID_SPK_LEN == 4
assert MIN_PADDING == 5
# This script cannot be spent, allowing dust output values under # This script cannot be spent, allowing dust output values under
# standardness checks # standardness checks
DUMMY_MIN_OP_RETURN_SCRIPT = CScript([OP_RETURN] + ([OP_0] * (MIN_PADDING - 1))) NONSTANDARD_OP_RETURN_SCRIPT = CScript([OP_RETURN] + ([OP_0] * (INVALID_SPK_LEN - 1)))
assert len(DUMMY_MIN_OP_RETURN_SCRIPT) == MIN_PADDING assert len(NONSTANDARD_OP_RETURN_SCRIPT) == INVALID_SPK_LEN
PAY_TO_ANCHOR = CScript([OP_1, bytes.fromhex("4e73")]) PAY_TO_ANCHOR = CScript([OP_1, bytes.fromhex("4e73")])