Skip to content

Commit 2b260ea

Browse files
committed
Merge bitcoin/bitcoin#29502: test: modify weight estimate in functional tests
e67ab17 test: fix flaky wallet_send functional test (Max Edwards) 3c49e69 test: fix weight estimates in functional tests (Max Edwards) Pull request description: Fixes: bitcoin/bitcoin#25164 The wallet_send functional test has been flaky due to a slightly overestimated weight calculation. This PR makes the weight calculation more accurate, although occasionally, due to how ECDSA signatures can be different lengths it might slightly over estimate. The assertion in the test can handle this slight variation and so should continue passing. Update: Because the signature can be shorter that is used in the weight estimation or the final transaction the estimate could be both slightly smaller or slightly larger. ACKs for top commit: achow101: ACK e67ab17 S3RK: Code review ACK e67ab17 Tree-SHA512: 3bf73b355309dce860fa1520afb8461e94268e4bcf0e92a8273c279b41b058c44472cf59daafa15a515529b50bd665b5d498bbe4d934f2315dbe810a05bc73f9
2 parents 98005b6 + e67ab17 commit 2b260ea

File tree

2 files changed

+24
-10
lines changed

2 files changed

+24
-10
lines changed

test/functional/rpc_psbt.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -753,11 +753,16 @@ def test_psbt_input_keys(psbt_input, keys):
753753
break
754754
psbt_in = dec["inputs"][input_idx]
755755
# Calculate the input weight
756-
# (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
756+
# (prevout + sequence + length of scriptSig + scriptsig) * WITNESS_SCALE_FACTOR + len of num scriptWitness stack items + (length of stack item + stack item) * N stack items
757+
# Note that occasionally this weight estimate may be slightly larger or smaller than the real weight
758+
# as sometimes ECDSA signatures are one byte shorter than expected with a probability of 1/128
757759
len_scriptsig = len(psbt_in["final_scriptSig"]["hex"]) // 2 if "final_scriptSig" in psbt_in else 0
758-
len_scriptsig += len(ser_compact_size(len_scriptsig)) + 1
759-
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
760-
input_weight = ((40 + len_scriptsig) * WITNESS_SCALE_FACTOR) + len_scriptwitness
760+
len_scriptsig += len(ser_compact_size(len_scriptsig))
761+
len_scriptwitness = (sum([(len(x) // 2) + len(ser_compact_size(len(x) // 2)) for x in psbt_in["final_scriptwitness"]]) + len(ser_compact_size(len(psbt_in["final_scriptwitness"])))) if "final_scriptwitness" in psbt_in else 0
762+
len_prevout_txid = 32
763+
len_prevout_index = 4
764+
len_sequence = 4
765+
input_weight = ((len_prevout_txid + len_prevout_index + len_sequence + len_scriptsig) * WITNESS_SCALE_FACTOR) + len_scriptwitness
761766
low_input_weight = input_weight // 2
762767
high_input_weight = input_weight * 2
763768

test/functional/wallet_send.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -543,11 +543,16 @@ def run_test(self):
543543
break
544544
psbt_in = dec["inputs"][input_idx]
545545
# Calculate the input weight
546-
# (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
546+
# (prevout + sequence + length of scriptSig + scriptsig) * WITNESS_SCALE_FACTOR + len of num scriptWitness stack items + (length of stack item + stack item) * N stack items
547+
# Note that occasionally this weight estimate may be slightly larger or smaller than the real weight
548+
# as sometimes ECDSA signatures are one byte shorter than expected with a probability of 1/128
547549
len_scriptsig = len(psbt_in["final_scriptSig"]["hex"]) // 2 if "final_scriptSig" in psbt_in else 0
548-
len_scriptsig += len(ser_compact_size(len_scriptsig)) + 1
549-
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
550-
input_weight = ((40 + len_scriptsig) * WITNESS_SCALE_FACTOR) + len_scriptwitness
550+
len_scriptsig += len(ser_compact_size(len_scriptsig))
551+
len_scriptwitness = (sum([(len(x) // 2) + len(ser_compact_size(len(x) // 2)) for x in psbt_in["final_scriptwitness"]]) + len(ser_compact_size(len(psbt_in["final_scriptwitness"])))) if "final_scriptwitness" in psbt_in else 0
552+
len_prevout_txid = 32
553+
len_prevout_index = 4
554+
len_sequence = 4
555+
input_weight = ((len_prevout_txid + len_prevout_index + len_sequence + len_scriptsig) * WITNESS_SCALE_FACTOR) + len_scriptwitness
551556

552557
# Input weight error conditions
553558
assert_raises_rpc_error(
@@ -558,6 +563,7 @@ def run_test(self):
558563
options={"inputs": [ext_utxo], "input_weights": [{"txid": ext_utxo["txid"], "vout": ext_utxo["vout"], "weight": 1000}]}
559564
)
560565

566+
target_fee_rate_sat_vb = 10
561567
# Funding should also work when input weights are provided
562568
res = self.test_send(
563569
from_wallet=ext_wallet,
@@ -567,14 +573,17 @@ def run_test(self):
567573
add_inputs=True,
568574
psbt=True,
569575
include_watching=True,
570-
fee_rate=10
576+
fee_rate=target_fee_rate_sat_vb
571577
)
572578
signed = ext_wallet.walletprocesspsbt(res["psbt"])
573579
signed = ext_fund.walletprocesspsbt(res["psbt"])
574580
assert signed["complete"]
575581
testres = self.nodes[0].testmempoolaccept([signed["hex"]])[0]
576582
assert_equal(testres["allowed"], True)
577-
assert_fee_amount(testres["fees"]["base"], testres["vsize"], Decimal(0.0001))
583+
actual_fee_rate_sat_vb = Decimal(testres["fees"]["base"]) * Decimal(1e8) / Decimal(testres["vsize"])
584+
# Due to ECDSA signatures not always being the same length, the actual fee rate may be slightly different
585+
# but rounded to nearest integer, it should be the same as the target fee rate
586+
assert_equal(round(actual_fee_rate_sat_vb), target_fee_rate_sat_vb)
578587

579588
if __name__ == '__main__':
580589
WalletSendTest().main()

0 commit comments

Comments
 (0)