Skip to content

Commit aa54132

Browse files
committed
Merge bitcoin/bitcoin#24454: tests: Fix calculation of external input weights
9f5ab67 tests: Use descriptor that requires both legacy and segwit (Andrew Chow) 8a04a38 tests: Calculate input weight more accurately (Andrew Chow) Pull request description: The external input tests with specifying input weight would sometimes result in a test failure because it would add 2 to the calculated byte size in order to account for some of the variation in signature and script sizes. However 1 in 128 signatures are actually 1 byte smaller than we expect, so the difference between the actual signature size and our calculated size becomes 3 bytes which is outside of the tolerance of `assert_fee_amount` and would thus cause the test failure. To resolve this, the 2 byte buffer is reduced to 1 byte, so in the above scenario, the difference is 2 bytes which is within the tolerance of `assert_fee_amount`. Additionally, instead of putting a fixed size that we assume is the correct size for the length of the compact size length prefix of data, we actually get the length of the compact size uint. Lastly, the size calculation for a scriptWitness was simply incorrect and used fields that did not exist. This is fixed, and the test slightly modified so that it also produces a scriptWitness. Fixes #24151 ACKs for top commit: jonatack: re-ACK 9f5ab67 glozow: code review ACK 9f5ab67 Tree-SHA512: b7c7ffe8fb0c07bc9e72fbff1f9ef57ee01a57c56bf54b8873345c8b9572c3ce9402b24dc211910b478114a9e6420faef5a4bf8866f38c299971354e54ec4745
2 parents b31ba3a + 9f5ab67 commit aa54132

File tree

2 files changed

+22
-12
lines changed

2 files changed

+22
-12
lines changed

test/functional/rpc_psbt.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010

1111
from test_framework.descriptors import descsum_create
1212
from test_framework.key import ECKey
13+
from test_framework.messages import (
14+
ser_compact_size,
15+
WITNESS_SCALE_FACTOR,
16+
)
1317
from test_framework.test_framework import BitcoinTestFramework
1418
from test_framework.util import (
1519
assert_approx,
@@ -615,8 +619,8 @@ def test_psbt_input_keys(psbt_input, keys):
615619
self.nodes[1].createwallet("extfund")
616620
wallet = self.nodes[1].get_wallet_rpc("extfund")
617621

618-
# Make a weird but signable script. sh(pkh()) descriptor accomplishes this
619-
desc = descsum_create("sh(pkh({}))".format(privkey))
622+
# Make a weird but signable script. sh(wsh(pkh())) descriptor accomplishes this
623+
desc = descsum_create("sh(wsh(pkh({})))".format(privkey))
620624
if self.options.descriptors:
621625
res = self.nodes[0].importdescriptors([{"desc": desc, "timestamp": "now"}])
622626
else:
@@ -634,7 +638,7 @@ def test_psbt_input_keys(psbt_input, keys):
634638
assert_raises_rpc_error(-4, "Insufficient funds", wallet.walletcreatefundedpsbt, [ext_utxo], {self.nodes[0].getnewaddress(): 15})
635639

636640
# But funding should work when the solving data is provided
637-
psbt = wallet.walletcreatefundedpsbt([ext_utxo], {self.nodes[0].getnewaddress(): 15}, 0, {"add_inputs": True, "solving_data": {"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"]]}})
641+
psbt = wallet.walletcreatefundedpsbt([ext_utxo], {self.nodes[0].getnewaddress(): 15}, 0, {"add_inputs": True, "solving_data": {"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"], addr_info["embedded"]["embedded"]["scriptPubKey"]]}})
638642
signed = wallet.walletprocesspsbt(psbt['psbt'])
639643
assert not signed['complete']
640644
signed = self.nodes[0].walletprocesspsbt(signed['psbt'])
@@ -655,10 +659,11 @@ def test_psbt_input_keys(psbt_input, keys):
655659
break
656660
psbt_in = dec["inputs"][input_idx]
657661
# Calculate the input weight
658-
# (prevout + sequence + length of scriptSig + 2 bytes buffer) * 4 + len of scriptwitness
662+
# (prevout + sequence + length of scriptSig + scriptsig + 1 byte buffer) * WITNESS_SCALE_FACTOR + num scriptWitness stack items + (length of stack item + stack item) * N stack items + 1 byte buffer
659663
len_scriptsig = len(psbt_in["final_scriptSig"]["hex"]) // 2 if "final_scriptSig" in psbt_in else 0
660-
len_scriptwitness = len(psbt_in["final_scriptwitness"]["hex"]) // 2 if "final_scriptwitness" in psbt_in else 0
661-
input_weight = ((41 + len_scriptsig + 2) * 4) + len_scriptwitness
664+
len_scriptsig += len(ser_compact_size(len_scriptsig)) + 1
665+
len_scriptwitness = (sum([(len(x) // 2) + len(ser_compact_size(len(x) // 2)) for x in psbt_in["final_scriptwitness"]]) + len(psbt_in["final_scriptwitness"]) + 1) if "final_scriptwitness" in psbt_in else 0
666+
input_weight = ((40 + len_scriptsig) * WITNESS_SCALE_FACTOR) + len_scriptwitness
662667
low_input_weight = input_weight // 2
663668
high_input_weight = input_weight * 2
664669

test/functional/wallet_send.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010
from test_framework.authproxy import JSONRPCException
1111
from test_framework.descriptors import descsum_create
1212
from test_framework.key import ECKey
13+
from test_framework.messages import (
14+
ser_compact_size,
15+
WITNESS_SCALE_FACTOR,
16+
)
1317
from test_framework.test_framework import BitcoinTestFramework
1418
from test_framework.util import (
1519
assert_equal,
@@ -488,8 +492,8 @@ def run_test(self):
488492
self.nodes[1].createwallet("extfund")
489493
ext_fund = self.nodes[1].get_wallet_rpc("extfund")
490494

491-
# Make a weird but signable script. sh(pkh()) descriptor accomplishes this
492-
desc = descsum_create("sh(pkh({}))".format(privkey))
495+
# Make a weird but signable script. sh(wsh(pkh())) descriptor accomplishes this
496+
desc = descsum_create("sh(wsh(pkh({})))".format(privkey))
493497
if self.options.descriptors:
494498
res = ext_fund.importdescriptors([{"desc": desc, "timestamp": "now"}])
495499
else:
@@ -507,7 +511,7 @@ def run_test(self):
507511
self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, expect_error=(-4, "Insufficient funds"))
508512

509513
# But funding should work when the solving data is provided
510-
res = self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, solving_data={"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"]]})
514+
res = self.test_send(from_wallet=ext_wallet, to_wallet=self.nodes[0], amount=15, inputs=[ext_utxo], add_inputs=True, psbt=True, include_watching=True, solving_data={"pubkeys": [addr_info['pubkey']], "scripts": [addr_info["embedded"]["scriptPubKey"], addr_info["embedded"]["embedded"]["scriptPubKey"]]})
511515
signed = ext_wallet.walletprocesspsbt(res["psbt"])
512516
signed = ext_fund.walletprocesspsbt(res["psbt"])
513517
assert signed["complete"]
@@ -526,10 +530,11 @@ def run_test(self):
526530
break
527531
psbt_in = dec["inputs"][input_idx]
528532
# Calculate the input weight
529-
# (prevout + sequence + length of scriptSig + 2 bytes buffer) * 4 + len of scriptwitness
533+
# (prevout + sequence + length of scriptSig + scriptsig + 1 byte buffer) * WITNESS_SCALE_FACTOR + num scriptWitness stack items + (length of stack item + stack item) * N stack items + 1 byte buffer
530534
len_scriptsig = len(psbt_in["final_scriptSig"]["hex"]) // 2 if "final_scriptSig" in psbt_in else 0
531-
len_scriptwitness = len(psbt_in["final_scriptwitness"]["hex"]) // 2 if "final_scriptwitness" in psbt_in else 0
532-
input_weight = ((41 + len_scriptsig + 2) * 4) + len_scriptwitness
535+
len_scriptsig += len(ser_compact_size(len_scriptsig)) + 1
536+
len_scriptwitness = (sum([(len(x) // 2) + len(ser_compact_size(len(x) // 2)) for x in psbt_in["final_scriptwitness"]]) + len(psbt_in["final_scriptwitness"]) + 1) if "final_scriptwitness" in psbt_in else 0
537+
input_weight = ((40 + len_scriptsig) * WITNESS_SCALE_FACTOR) + len_scriptwitness
533538

534539
# Input weight error conditions
535540
assert_raises_rpc_error(

0 commit comments

Comments
 (0)