Skip to content

Commit 1d5cb4b

Browse files
committed
bitcoin: fix out-by-one-error in bitcoin_tx_input_weight.
We need one byte for the number of witness elements. Some callers added it themselves, but it's always needed. So document and fix the callers. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
1 parent 70f0513 commit 1d5cb4b

File tree

7 files changed

+13
-10
lines changed

7 files changed

+13
-10
lines changed

bitcoin/tx.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -893,7 +893,8 @@ size_t bitcoin_tx_input_sig_weight(void)
893893
/* Input weight */
894894
size_t bitcoin_tx_input_weight(bool p2sh, size_t witness_weight)
895895
{
896-
size_t weight = witness_weight;
896+
/* We assume < 253 witness elements */
897+
size_t weight = 1 + witness_weight;
897898

898899
/* Input weight: txid + index + sequence */
899900
weight += (32 + 4 + 4) * 4;
@@ -914,8 +915,8 @@ size_t bitcoin_tx_input_weight(bool p2sh, size_t witness_weight)
914915

915916
size_t bitcoin_tx_simple_input_witness_weight(void)
916917
{
917-
/* Account for witness (1 byte count + sig + key) */
918-
return 1 + (bitcoin_tx_input_sig_weight() + 1 + 33);
918+
/* Account for witness (sig + key) */
919+
return bitcoin_tx_input_sig_weight() + 1 + 33;
919920
}
920921

921922
/* We only do segwit inputs, and we assume witness is sig + key */

bitcoin/tx.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,9 @@ size_t bitcoin_tx_output_weight(size_t outscript_len);
315315
/* Weight to push sig on stack. */
316316
size_t bitcoin_tx_input_sig_weight(void);
317317

318-
/* Segwit input, but with parameter for witness weight (size) */
318+
/* Segwit input, but with parameter for witness weight (size).
319+
* witness_weight must include the varint_size() for each witness element,
320+
* but not the varint_size() for the number of elements. */
319321
size_t bitcoin_tx_input_weight(bool p2sh, size_t witness_weight);
320322

321323
/* The witness weight for a simple (sig + key) input */

onchaind/onchaind.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,9 @@ static void trim_maximum_feerate(struct amount_sat funding,
186186
* * `txin[0]` script bytes: 0
187187
* * `txin[0]` witness: `0 <signature_for_pubkey1> <signature_for_pubkey2>`
188188
*/
189-
/* Account for witness (1 byte count + 1 empty + sig + sig) */
189+
/* Account for witness (1 empty + sig + sig) */
190190
assert(tal_count(commitment->inputs) == 1);
191-
weight += bitcoin_tx_input_weight(false, 1 + 1 + 2 * bitcoin_tx_input_sig_weight());
191+
weight += bitcoin_tx_input_weight(false, 1 + 2 * bitcoin_tx_input_sig_weight());
192192

193193
for (size_t i = 0; i < tal_count(commitment->outputs); i++) {
194194
struct amount_asset amt;

tests/test_bookkeeper.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ def _check_events(node, channel_id, exp_events):
410410
_check_events(l1, channel_id, exp_events)
411411

412412
exp_events = [('channel_open', open_amt * 1000, 0),
413-
('onchain_fee', 892000, 0),
413+
('onchain_fee', 894000, 0),
414414
('lease_fee', lease_fee, 0),
415415
('journal_entry', invoice_msat, 0)]
416416
_check_events(l2, channel_id, exp_events)

tests/test_closing.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4274,7 +4274,7 @@ def censoring_sendrawtx(r):
42744274
height = bitcoind.rpc.getblockchaininfo()['blocks']
42754275
l1.daemon.wait_for_log(r"Low-priority anchorspend aiming for block {} \(feerate 7458\)".format(height + 13))
42764276
# Can be out-by-one (short sig)!
4277-
l1.daemon.wait_for_log(r"Anchorspend for local commit tx fee 12022sat \(w=672\), commit_tx fee 1735sat \(w=768\): package feerate 9553 perkw")
4277+
l1.daemon.wait_for_log(r"Anchorspend for local commit tx fee 12037 \(w=674\), commit_tx fee 1735sat \(w=768\): package feerate 9550 perkw")
42784278
assert not l1.daemon.is_in_log("Low-priority anchorspend aiming for block {}".format(height + 12))
42794279

42804280
bitcoind.generate_block(1)

tests/test_misc.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -746,7 +746,7 @@ def dont_spend_outputs(n, txid):
746746
{'type': 'chain_mvt', 'credit_msat': 2000000000, 'debit_msat': 0, 'tags': ['deposit']},
747747
{'type': 'chain_mvt', 'credit_msat': 2000000000, 'debit_msat': 0, 'tags': ['deposit']},
748748
{'type': 'chain_mvt', 'credit_msat': 2000000000, 'debit_msat': 0, 'tags': ['deposit']},
749-
{'type': 'chain_mvt', 'credit_msat': 11957423000, 'debit_msat': 0, 'tags': ['deposit']},
749+
{'type': 'chain_mvt', 'credit_msat': 11957393000, 'debit_msat': 0, 'tags': ['deposit']},
750750
]
751751

752752
check_coin_moves(l1, 'external', external_moves, chainparams)

tests/test_opening.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1526,7 +1526,7 @@ def test_funder_contribution_limits(node_factory, bitcoind):
15261526

15271527
l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
15281528
l1.fundchannel(l2, 10**7)
1529-
assert l2.daemon.is_in_log('Policy .* returned funding amount of 107530sat')
1529+
assert l2.daemon.is_in_log('Policy .* returned funding amount of 107470sat')
15301530
assert l2.daemon.is_in_log(r'calling `signpsbt` .* inputs')
15311531

15321532
l1.rpc.connect(l3.info['id'], 'localhost', l3.port)

0 commit comments

Comments
 (0)