From ce024b1c0ef2dcd307023aaaab40373c8bf17db1 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 27 May 2021 18:09:07 +0200 Subject: [PATCH 1/2] test: MiniWallet: force P2PK signature to have fixed size (71 bytes) In order to enable exact fee calculation for transactions that spend P2PK outputs in the MiniWallet, we enforce the created signatures to have a fixed length (>49.89% probability) by default. With that it is easier to check the created transactions vsize and avoid flaky tests that would appear whenever the signatures R- or S-values are smaller (due to leading zero bytes). Note that to get the total scriptSig size one has to add another 2 bytes, as there is also the OP_PUSHx instruction on the front and the sighash type byte on the back, leading to a final scriptSig size of 73 bytes. --- test/functional/test_framework/wallet.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index 0962a5cb54..4b761128ee 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -88,12 +88,20 @@ class MiniWallet: if out['scriptPubKey']['hex'] == self._scriptPubKey.hex(): self._utxos.append({'txid': tx['txid'], 'vout': out['n'], 'value': out['value']}) - def sign_tx(self, tx): + def sign_tx(self, tx, fixed_length=True): """Sign tx that has been created by MiniWallet in P2PK mode""" assert self._priv_key is not None (sighash, err) = LegacySignatureHash(CScript(self._scriptPubKey), tx, 0, SIGHASH_ALL) assert err is None - tx.vin[0].scriptSig = CScript([self._priv_key.sign_ecdsa(sighash) + bytes(bytearray([SIGHASH_ALL]))]) + # for exact fee calculation, create only signatures with fixed size by default (>49.89% probability): + # 65 bytes: high-R val (33 bytes) + low-S val (32 bytes) + # with the DER header/skeleton data of 6 bytes added, this leads to a target size of 71 bytes + der_sig = b'' + while not len(der_sig) == 71: + der_sig = self._priv_key.sign_ecdsa(sighash) + if not fixed_length: + break + tx.vin[0].scriptSig = CScript([der_sig + bytes(bytearray([SIGHASH_ALL]))]) def generate(self, num_blocks): """Generate blocks with coinbase outputs to the internal address, and append the outputs to the internal list""" From d6d2ab984547be4a9f7ba859a2a4c9ac9bfbf206 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 27 May 2021 20:15:02 +0200 Subject: [PATCH 2/2] test: MiniWallet: fix fee calculation for P2PK and check tx vsize --- test/functional/test_framework/wallet.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index 4b761128ee..19628c0791 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -142,7 +142,10 @@ class MiniWallet: """Create and return a tx with the specified fee_rate. Fee may be exact or at most one satoshi higher than needed.""" self._utxos = sorted(self._utxos, key=lambda k: k['value']) utxo_to_spend = utxo_to_spend or self._utxos.pop() # Pick the largest utxo (if none provided) and hope it covers the fee - vsize = Decimal(96) + if self._priv_key is None: + vsize = Decimal(96) # anyone-can-spend + else: + vsize = Decimal(168) # P2PK (73 bytes scriptSig + 35 bytes scriptPubKey + 60 bytes other) send_value = satoshi_round(utxo_to_spend['value'] - fee_rate * (vsize / 1000)) fee = utxo_to_spend['value'] - send_value assert send_value > 0 @@ -167,10 +170,7 @@ class MiniWallet: tx_info = from_node.testmempoolaccept([tx_hex])[0] assert_equal(mempool_valid, tx_info['allowed']) if mempool_valid: - # TODO: for P2PK, vsize is not constant due to varying scriptSig length, - # so only check this for anyone-can-spend outputs right now - if self._priv_key is None: - assert_equal(tx_info['vsize'], vsize) + assert_equal(tx_info['vsize'], vsize) assert_equal(tx_info['fees']['base'], fee) return {'txid': tx_info['txid'], 'wtxid': tx_info['wtxid'], 'hex': tx_hex, 'tx': tx}