aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGreg Sanders <gsanders87@gmail.com>2022-10-05 14:50:30 -0400
committerGreg Sanders <gsanders87@gmail.com>2022-12-19 10:03:51 -0500
commit8c5b3646b5afe8a61f5c66478d8e11f0d2ce5108 (patch)
tree8925907b89fa1a534f2548166288bdaaf5a211b2
parent9ca39d69dfe03c19662867c3d53a5e6e41277d4a (diff)
downloadbitcoin-8c5b3646b5afe8a61f5c66478d8e11f0d2ce5108.tar.xz
Relax MIN_STANDARD_TX_NONWITNESS_SIZE to 65 non-witness bytes
Since the original fix was set to be a "reasonable" transaction to reduce allocations and the true motivation later revealed, it makes sense to relax this check to something more principled. There are more exotic transaction patterns that could take advantage of a relaxed requirement, such as 1 input, 1 output OP_RETURN to burn a utxo to fees for CPFP purposes when change isn't practical. Two changes could be accomplished: 1) Anything not 64 bytes could be allowed 2) Anything above 64 bytes could be allowed In the Great Consensus Cleanup, suggestion (2) was the route taken. It would not allow an "empty" OP_RETURN but would reduce the required padding from 22 bytes to 5. The functional test is also modified to test the actual case we care about: 64 bytes
-rw-r--r--src/policy/policy.h4
-rw-r--r--src/validation.cpp5
-rw-r--r--test/functional/data/invalid_txs.py9
-rwxr-xr-xtest/functional/mempool_accept.py37
-rwxr-xr-xtest/functional/test_framework/script_util.py21
5 files changed, 57 insertions, 19 deletions
diff --git a/src/policy/policy.h b/src/policy/policy.h
index 29764ea2d9..8a26e70ef4 100644
--- a/src/policy/policy.h
+++ b/src/policy/policy.h
@@ -25,8 +25,8 @@ static constexpr unsigned int DEFAULT_BLOCK_MAX_WEIGHT{MAX_BLOCK_WEIGHT - 4000};
static constexpr unsigned int DEFAULT_BLOCK_MIN_TX_FEE{1000};
/** The maximum weight for transactions we're willing to relay/mine */
static constexpr unsigned int MAX_STANDARD_TX_WEIGHT{400000};
-/** The minimum non-witness size for transactions we're willing to relay/mine (1 segwit input + 1 P2WPKH output = 82 bytes) */
-static constexpr unsigned int MIN_STANDARD_TX_NONWITNESS_SIZE{82};
+/** The minimum non-witness size for transactions we're willing to relay/mine: one larger than 64 */
+static constexpr unsigned int MIN_STANDARD_TX_NONWITNESS_SIZE{65};
/** Maximum number of signature check operations in an IsStandard() P2SH script */
static constexpr unsigned int MAX_P2SH_SIGOPS{15};
/** The maximum number of sigops we're willing to relay/mine in a single tx */
diff --git a/src/validation.cpp b/src/validation.cpp
index 514a528314..b6c1c57aea 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -696,10 +696,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
return state.Invalid(TxValidationResult::TX_NOT_STANDARD, reason);
}
- // Do not work on transactions that are too small.
- // A transaction with 1 segwit input and 1 P2WPHK output has non-witness size of 82 bytes.
- // Transactions smaller than this are not relayed to mitigate CVE-2017-12842 by not relaying
- // 64-byte transactions.
+ // Transactions smaller than 65 non-witness bytes are not relayed to mitigate CVE-2017-12842.
if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) < MIN_STANDARD_TX_NONWITNESS_SIZE)
return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "tx-size-small");
diff --git a/test/functional/data/invalid_txs.py b/test/functional/data/invalid_txs.py
index 3747b2a98d..7892410826 100644
--- a/test/functional/data/invalid_txs.py
+++ b/test/functional/data/invalid_txs.py
@@ -46,18 +46,19 @@ from test_framework.script import (
OP_MOD,
OP_MUL,
OP_OR,
+ OP_RETURN,
OP_RIGHT,
OP_RSHIFT,
OP_SUBSTR,
- OP_TRUE,
OP_XOR,
)
from test_framework.script_util import (
+ MIN_PADDING,
+ MIN_STANDARD_TX_NONWITNESS_SIZE,
script_to_p2sh_script,
)
basic_p2sh = script_to_p2sh_script(CScript([OP_0]))
-
class BadTxTemplate:
"""Allows simple construction of a certain kind of invalid tx. Base class to be subclassed."""
__metaclass__ = abc.ABCMeta
@@ -122,7 +123,9 @@ class SizeTooSmall(BadTxTemplate):
def get_tx(self):
tx = CTransaction()
tx.vin.append(self.valid_txin)
- tx.vout.append(CTxOut(0, CScript([OP_TRUE])))
+ tx.vout.append(CTxOut(0, CScript([OP_RETURN] + ([OP_0] * (MIN_PADDING - 2)))))
+ assert len(tx.serialize_without_witness()) == 64
+ assert MIN_STANDARD_TX_NONWITNESS_SIZE - 1 == 64
tx.calc_sha256()
return tx
diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py
index 02ec18140c..7ec3fbe7cf 100755
--- a/test/functional/mempool_accept.py
+++ b/test/functional/mempool_accept.py
@@ -14,7 +14,9 @@ from test_framework.messages import (
MAX_BIP125_RBF_SEQUENCE,
COIN,
COutPoint,
+ CTransaction,
CTxIn,
+ CTxInWitness,
CTxOut,
MAX_BLOCK_WEIGHT,
MAX_MONEY,
@@ -26,13 +28,19 @@ from test_framework.script import (
OP_0,
OP_HASH160,
OP_RETURN,
+ OP_TRUE,
)
from test_framework.script_util import (
+ DUMMY_MIN_OP_RETURN_SCRIPT,
keys_to_multisig_script,
+ MIN_PADDING,
+ MIN_STANDARD_TX_NONWITNESS_SIZE,
script_to_p2sh_script,
+ script_to_p2wsh_script,
)
from test_framework.util import (
assert_equal,
+ assert_greater_than,
assert_raises_rpc_error,
)
from test_framework.wallet import MiniWallet
@@ -333,6 +341,35 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
maxfeerate=0,
)
+ # Prep for tiny-tx tests with wsh(OP_TRUE) output
+ seed_tx = self.wallet.send_to(from_node=node, scriptPubKey=script_to_p2wsh_script(CScript([OP_TRUE])), amount=COIN)
+ self.generate(node, 1)
+
+ self.log.info('A tiny transaction(in non-witness bytes) that is disallowed')
+ tx = CTransaction()
+ tx.vin.append(CTxIn(COutPoint(int(seed_tx[0], 16), seed_tx[1]), b"", SEQUENCE_FINAL))
+ 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!
+ assert_equal(len(tx.serialize_without_witness()), 64)
+ assert_equal(MIN_STANDARD_TX_NONWITNESS_SIZE - 1, 64)
+ assert_greater_than(len(tx.serialize()), 64)
+
+ self.check_mempool_result(
+ result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'tx-size-small'}],
+ rawtxs=[tx.serialize().hex()],
+ maxfeerate=0,
+ )
+
+ self.log.info('Minimally-small transaction(in non-witness bytes) that is allowed')
+ tx.vout[0] = CTxOut(COIN - 1000, DUMMY_MIN_OP_RETURN_SCRIPT)
+ assert_equal(len(tx.serialize_without_witness()), MIN_STANDARD_TX_NONWITNESS_SIZE)
+ 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,
+ )
if __name__ == '__main__':
MempoolAcceptanceTest().main()
diff --git a/test/functional/test_framework/script_util.py b/test/functional/test_framework/script_util.py
index b114002145..39dcb418b7 100755
--- a/test/functional/test_framework/script_util.py
+++ b/test/functional/test_framework/script_util.py
@@ -13,12 +13,13 @@ from test_framework.script import (
OP_EQUAL,
OP_EQUALVERIFY,
OP_HASH160,
+ OP_RETURN,
hash160,
sha256,
)
# To prevent a "tx-size-small" policy rule error, a transaction has to have a
-# non-witness size of at least 82 bytes (MIN_STANDARD_TX_NONWITNESS_SIZE in
+# non-witness size of at least 65 bytes (MIN_STANDARD_TX_NONWITNESS_SIZE in
# src/policy/policy.h). Considering a Tx with the smallest possible single
# input (blank, empty scriptSig), and with an output omitting the scriptPubKey,
# we get to a minimum size of 60 bytes:
@@ -28,15 +29,15 @@ from test_framework.script import (
# Output: 8 [Amount] + 1 [scriptPubKeyLen] = 9 bytes
#
# Hence, the scriptPubKey of the single output has to have a size of at
-# least 22 bytes, which corresponds to the size of a P2WPKH scriptPubKey.
-# The following script constant consists of a single push of 21 bytes of 'a':
-# <PUSH_21> <21-bytes of 'a'>
-# resulting in a 22-byte size. It should be used whenever (small) fake
-# scriptPubKeys are needed, to guarantee that the minimum transaction size is
-# met.
-DUMMY_P2WPKH_SCRIPT = CScript([b'a' * 21])
-DUMMY_2_P2WPKH_SCRIPT = CScript([b'b' * 21])
-
+# least 5 bytes.
+MIN_STANDARD_TX_NONWITNESS_SIZE = 65
+MIN_PADDING = MIN_STANDARD_TX_NONWITNESS_SIZE - 10 - 41 - 9
+assert MIN_PADDING == 5
+
+# This script cannot be spent, allowing dust output values under
+# standardness checks
+DUMMY_MIN_OP_RETURN_SCRIPT = CScript([OP_RETURN] + ([OP_0] * (MIN_PADDING - 1)))
+assert len(DUMMY_MIN_OP_RETURN_SCRIPT) == MIN_PADDING
def key_to_p2pk_script(key):
key = check_key(key)