Skip to content

Commit 81447c9

Browse files
committed
Use 72 WU instead of 73 WU for signature weight
When estimating signature weight, 73 WU was used in some places while 72 WU was used in others. Consistently use 72 WU and replace hardcoded values with constants. 73 WU signatures are non-standard and won't be produced by LDK. Additionally, using 73 WU along with grind_signatures adjustment is nonsensical.
1 parent 08c2236 commit 81447c9

File tree

4 files changed

+65
-43
lines changed

4 files changed

+65
-43
lines changed

lightning/src/ln/chan_utils.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,19 +116,19 @@ pub const HTLC_SUCCESS_INPUT_P2A_ANCHOR_WITNESS_WEIGHT: u64 = 324;
116116
/// The size of the 2-of-2 multisig script
117117
const MULTISIG_SCRIPT_SIZE: u64 = 1 + // OP_2
118118
1 + // data len
119-
33 + // pubkey1
119+
bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE as u64 + // pubkey1
120120
1 + // data len
121-
33 + // pubkey2
121+
bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE as u64 + // pubkey2
122122
1 + // OP_2
123123
1; // OP_CHECKMULTISIG
124124
/// The weight of a funding transaction input (2-of-2 P2WSH)
125125
/// See https://github.com/lightning/bolts/blob/master/03-transactions.md#expected-weight-of-the-commitment-transaction
126126
pub const FUNDING_TRANSACTION_WITNESS_WEIGHT: u64 = 1 + // number_of_witness_elements
127127
1 + // nil_len
128128
1 + // sig len
129-
73 + // sig1
129+
bitcoin::secp256k1::constants::MAX_SIGNATURE_SIZE as u64 + // sig1
130130
1 + // sig len
131-
73 + // sig2
131+
bitcoin::secp256k1::constants::MAX_SIGNATURE_SIZE as u64 + // sig2
132132
1 + // witness_script_length
133133
MULTISIG_SCRIPT_SIZE;
134134

lightning/src/ln/channel.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17397,19 +17397,19 @@ mod tests {
1739717397
// 2 inputs, initiator, 2000 sat/kw feerate
1739817398
assert_eq!(
1739917399
estimate_v2_funding_transaction_fee(&two_inputs, &[], true, false, 2000),
17400-
1520,
17400+
1516,
1740117401
);
1740217402

1740317403
// higher feerate
1740417404
assert_eq!(
1740517405
estimate_v2_funding_transaction_fee(&two_inputs, &[], true, false, 3000),
17406-
2280,
17406+
2274,
1740717407
);
1740817408

1740917409
// only 1 input
1741017410
assert_eq!(
1741117411
estimate_v2_funding_transaction_fee(&one_input, &[], true, false, 2000),
17412-
974,
17412+
972,
1741317413
);
1741417414

1741517415
// 0 inputs
@@ -17427,13 +17427,13 @@ mod tests {
1742717427
// splice initiator
1742817428
assert_eq!(
1742917429
estimate_v2_funding_transaction_fee(&one_input, &[], true, true, 2000),
17430-
1746,
17430+
1740,
1743117431
);
1743217432

1743317433
// splice acceptor
1743417434
assert_eq!(
1743517435
estimate_v2_funding_transaction_fee(&one_input, &[], false, true, 2000),
17436-
546,
17436+
544,
1743717437
);
1743817438
}
1743917439

@@ -17468,7 +17468,7 @@ mod tests {
1746817468
true,
1746917469
2000,
1747017470
).unwrap(),
17471-
2292,
17471+
2284,
1747217472
);
1747317473

1747417474
// negative case, inputs clearly insufficient
@@ -17484,13 +17484,13 @@ mod tests {
1748417484
);
1748517485
assert_eq!(
1748617486
res.err().unwrap(),
17487-
"Total input amount 100000 is lower than needed for contribution 220000, considering fees of 1746. Need more inputs.",
17487+
"Total input amount 100000 is lower than needed for contribution 220000, considering fees of 1740. Need more inputs.",
1748817488
);
1748917489
}
1749017490

1749117491
// barely covers
1749217492
{
17493-
let expected_fee: u64 = 2292;
17493+
let expected_fee: u64 = 2284;
1749417494
assert_eq!(
1749517495
check_v2_funding_inputs_sufficient(
1749617496
(300_000 - expected_fee - 20) as i64,
@@ -17520,13 +17520,13 @@ mod tests {
1752017520
);
1752117521
assert_eq!(
1752217522
res.err().unwrap(),
17523-
"Total input amount 300000 is lower than needed for contribution 298032, considering fees of 2522. Need more inputs.",
17523+
"Total input amount 300000 is lower than needed for contribution 298032, considering fees of 2513. Need more inputs.",
1752417524
);
1752517525
}
1752617526

1752717527
// barely covers, less fees (no extra weight, no init)
1752817528
{
17529-
let expected_fee: u64 = 1092;
17529+
let expected_fee: u64 = 1088;
1753017530
assert_eq!(
1753117531
check_v2_funding_inputs_sufficient(
1753217532
(300_000 - expected_fee - 20) as i64,

lightning/src/ln/interactivetxs.rs

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3353,38 +3353,38 @@ mod tests {
33533353
FundingTxInput::new_p2wpkh(prevtx, 0).unwrap()
33543354
})
33553355
.collect();
3356-
let our_contributed = 110_000;
33573356
let txout = TxOut { value: Amount::from_sat(10_000), script_pubkey: ScriptBuf::new() };
33583357
let outputs = vec![txout];
33593358
let funding_feerate_sat_per_1000_weight = 3000;
33603359

3361-
let total_inputs: u64 = input_prevouts.iter().map(|o| o.value.to_sat()).sum();
3362-
let total_outputs: u64 = outputs.iter().map(|o| o.value.to_sat()).sum();
3363-
let gross_change = total_inputs - total_outputs - our_contributed;
3364-
let fees = 1746;
3365-
let common_fees = 234;
3360+
let total_inputs: Amount = input_prevouts.iter().map(|o| o.value).sum();
3361+
let total_outputs: Amount = outputs.iter().map(|o| o.value).sum();
3362+
let fees = Amount::from_sat(1740);
3363+
let common_fees = Amount::from_sat(234);
33663364

33673365
// There is leftover for change
33683366
let context = FundingNegotiationContext {
33693367
is_initiator: true,
3370-
our_funding_contribution: SignedAmount::from_sat(our_contributed as i64),
3368+
our_funding_contribution: SignedAmount::from_sat(110_000),
33713369
funding_tx_locktime: AbsoluteLockTime::ZERO,
33723370
funding_feerate_sat_per_1000_weight,
33733371
shared_funding_input: None,
33743372
our_funding_inputs: inputs,
33753373
our_funding_outputs: outputs,
33763374
change_script: None,
33773375
};
3376+
let gross_change =
3377+
total_inputs - total_outputs - context.our_funding_contribution.to_unsigned().unwrap();
33783378
assert_eq!(
33793379
calculate_change_output_value(&context, false, &ScriptBuf::new(), 300),
3380-
Ok(Some(gross_change - fees - common_fees)),
3380+
Ok(Some((gross_change - fees - common_fees).to_sat())),
33813381
);
33823382

33833383
// There is leftover for change, without common fees
33843384
let context = FundingNegotiationContext { is_initiator: false, ..context };
33853385
assert_eq!(
33863386
calculate_change_output_value(&context, false, &ScriptBuf::new(), 300),
3387-
Ok(Some(gross_change - fees)),
3387+
Ok(Some((gross_change - fees).to_sat())),
33883388
);
33893389

33903390
// Insufficient inputs, no leftover
@@ -3415,21 +3415,25 @@ mod tests {
34153415
our_funding_contribution: SignedAmount::from_sat(117_992),
34163416
..context
34173417
};
3418+
let gross_change =
3419+
total_inputs - total_outputs - context.our_funding_contribution.to_unsigned().unwrap();
34183420
assert_eq!(
34193421
calculate_change_output_value(&context, false, &ScriptBuf::new(), 100),
3420-
Ok(Some(262)),
3422+
Ok(Some((gross_change - fees).to_sat())),
34213423
);
34223424

34233425
// Larger fee, smaller change
34243426
let context = FundingNegotiationContext {
34253427
is_initiator: true,
3426-
our_funding_contribution: SignedAmount::from_sat(our_contributed as i64),
3428+
our_funding_contribution: SignedAmount::from_sat(110_000),
34273429
funding_feerate_sat_per_1000_weight: funding_feerate_sat_per_1000_weight * 3,
34283430
..context
34293431
};
3432+
let gross_change =
3433+
total_inputs - total_outputs - context.our_funding_contribution.to_unsigned().unwrap();
34303434
assert_eq!(
34313435
calculate_change_output_value(&context, false, &ScriptBuf::new(), 300),
3432-
Ok(Some(4060)),
3436+
Ok(Some((gross_change - fees * 3 - common_fees * 3).to_sat())),
34333437
);
34343438
}
34353439

lightning/src/sign/mod.rs

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,12 @@ impl DelayedPaymentOutputDescriptor {
114114
/// The maximum length a well-formed witness spending one of these should have.
115115
/// Note: If you have the grind_signatures feature enabled, this will be at least 1 byte
116116
/// shorter.
117-
// Calculated as 1 byte length + 73 byte signature, 1 byte empty vec push, 1 byte length plus
118-
// redeemscript push length.
119-
pub const MAX_WITNESS_LENGTH: u64 =
120-
1 + 73 + 1 + chan_utils::REVOKEABLE_REDEEMSCRIPT_MAX_LENGTH as u64 + 1;
117+
pub const MAX_WITNESS_LENGTH: u64 = (1 /* witness items */
118+
+ 1 /* sig push */
119+
+ bitcoin::secp256k1::constants::MAX_SIGNATURE_SIZE
120+
+ 1 /* empty vec push */
121+
+ 1 /* redeemscript push */
122+
+ chan_utils::REVOKEABLE_REDEEMSCRIPT_MAX_LENGTH) as u64;
121123
}
122124

123125
impl_writeable_tlv_based!(DelayedPaymentOutputDescriptor, {
@@ -131,15 +133,19 @@ impl_writeable_tlv_based!(DelayedPaymentOutputDescriptor, {
131133
(13, channel_transaction_parameters, (option: ReadableArgs, Some(channel_value_satoshis.0.unwrap()))),
132134
});
133135

134-
pub(crate) const P2WPKH_WITNESS_WEIGHT: u64 = 1 /* num stack items */ +
135-
1 /* sig length */ +
136-
73 /* sig including sighash flag */ +
137-
1 /* pubkey length */ +
138-
33 /* pubkey */;
136+
/// Witness weight for satisfying a P2WPKH spend.
137+
pub(crate) const P2WPKH_WITNESS_WEIGHT: u64 = (1 /* witness items */
138+
+ 1 /* sig push */
139+
+ bitcoin::secp256k1::constants::MAX_SIGNATURE_SIZE
140+
+ 1 /* pubkey push */
141+
+ bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE)
142+
as u64;
139143

140-
/// Witness weight for satisying a P2TR key-path spend.
141-
pub(crate) const P2TR_KEY_PATH_WITNESS_WEIGHT: u64 = 1 /* witness items */
142-
+ 1 /* schnorr sig len */ + 64 /* schnorr sig */;
144+
/// Witness weight for satisfying a P2TR key-path spend.
145+
pub(crate) const P2TR_KEY_PATH_WITNESS_WEIGHT: u64 = (1 /* witness items */
146+
+ 1 /* sig push */
147+
+ bitcoin::secp256k1::constants::SCHNORR_SIGNATURE_SIZE)
148+
as u64;
143149

144150
/// If a [`KeysManager`] is built with [`KeysManager::new`] with `v2_remote_key_derivation` set
145151
/// (and for all channels after they've been spliced), the script which we receive funds to on-chain
@@ -192,10 +198,16 @@ impl StaticPaymentOutputDescriptor {
192198
/// shorter.
193199
pub fn max_witness_length(&self) -> u64 {
194200
if self.needs_csv_1_for_spend() {
195-
let witness_script_weight = 1 /* pubkey push */ + 33 /* pubkey */ +
196-
1 /* OP_CHECKSIGVERIFY */ + 1 /* OP_1 */ + 1 /* OP_CHECKSEQUENCEVERIFY */;
197-
1 /* num witness items */ + 1 /* sig push */ + 73 /* sig including sighash flag */ +
198-
1 /* witness script push */ + witness_script_weight
201+
let witness_script_weight = 1 /* pubkey push */
202+
+ bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE
203+
+ 1 /* OP_CHECKSIGVERIFY */
204+
+ 1 /* OP_1 */
205+
+ 1 /* OP_CHECKSEQUENCEVERIFY */;
206+
(1 /* num witness items */
207+
+ 1 /* sig push */
208+
+ bitcoin::secp256k1::constants::MAX_SIGNATURE_SIZE
209+
+ 1 /* witness script push */
210+
+ witness_script_weight) as u64
199211
} else {
200212
P2WPKH_WITNESS_WEIGHT
201213
}
@@ -511,7 +523,13 @@ impl SpendableOutputDescriptor {
511523
sequence: Sequence::ZERO,
512524
witness: Witness::new(),
513525
});
514-
witness_weight += 1 + 73 + 34;
526+
const STATIC_OUTPUT_WITNESS_WEIGHT: u64 = (1 /* witness items */
527+
+ 1 /* sig push */
528+
+ bitcoin::secp256k1::constants::MAX_SIGNATURE_SIZE
529+
+ 1 /* pubkey push */
530+
+ bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE)
531+
as u64;
532+
witness_weight += STATIC_OUTPUT_WITNESS_WEIGHT;
515533
#[cfg(feature = "grind_signatures")]
516534
{
517535
// Guarantees a low R signature

0 commit comments

Comments
 (0)