Skip to content

Commit 4e7f464

Browse files
jkczyzTheBlueMatt
authored andcommitted
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. Backport of eb341de
1 parent 65ad2fc commit 4e7f464

File tree

4 files changed

+70
-45
lines changed

4 files changed

+70
-45
lines changed

lightning/src/ln/chan_utils.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,19 +116,24 @@ 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+
crate::sign::COMPRESSED_PUBLIC_KEY_SIZE as u64 + // pubkey1
120120
1 + // data len
121-
33 + // pubkey2
121+
crate::sign::COMPRESSED_PUBLIC_KEY_SIZE as u64 + // pubkey2
122122
1 + // OP_2
123123
1; // OP_CHECKMULTISIG
124-
/// The weight of a funding transaction input (2-of-2 P2WSH)
125-
/// See https://github.com/lightning/bolts/blob/master/03-transactions.md#expected-weight-of-the-commitment-transaction
124+
125+
/// The weight of a funding transaction input (2-of-2 P2WSH).
126+
///
127+
/// Unlike in the [spec], 72 WU is used for the max signature size since 73 WU signatures are
128+
/// non-standard.
129+
///
130+
/// [spec]: https://github.com/lightning/bolts/blob/master/03-transactions.md#expected-weight-of-the-commitment-transaction
126131
pub const FUNDING_TRANSACTION_WITNESS_WEIGHT: u64 = 1 + // number_of_witness_elements
127132
1 + // nil_len
128133
1 + // sig len
129-
73 + // sig1
134+
crate::sign::MAX_STANDARD_SIGNATURE_SIZE as u64 + // sig1
130135
1 + // sig len
131-
73 + // sig2
136+
crate::sign::MAX_STANDARD_SIGNATURE_SIZE as u64 + // sig2
132137
1 + // witness_script_length
133138
MULTISIG_SCRIPT_SIZE;
134139

lightning/src/ln/channel.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17413,19 +17413,19 @@ mod tests {
1741317413
// 2 inputs, initiator, 2000 sat/kw feerate
1741417414
assert_eq!(
1741517415
estimate_v2_funding_transaction_fee(&two_inputs, &[], true, false, 2000),
17416-
1520,
17416+
1516,
1741717417
);
1741817418

1741917419
// higher feerate
1742017420
assert_eq!(
1742117421
estimate_v2_funding_transaction_fee(&two_inputs, &[], true, false, 3000),
17422-
2280,
17422+
2274,
1742317423
);
1742417424

1742517425
// only 1 input
1742617426
assert_eq!(
1742717427
estimate_v2_funding_transaction_fee(&one_input, &[], true, false, 2000),
17428-
974,
17428+
972,
1742917429
);
1743017430

1743117431
// 0 inputs
@@ -17443,13 +17443,13 @@ mod tests {
1744317443
// splice initiator
1744417444
assert_eq!(
1744517445
estimate_v2_funding_transaction_fee(&one_input, &[], true, true, 2000),
17446-
1746,
17446+
1740,
1744717447
);
1744817448

1744917449
// splice acceptor
1745017450
assert_eq!(
1745117451
estimate_v2_funding_transaction_fee(&one_input, &[], false, true, 2000),
17452-
546,
17452+
544,
1745317453
);
1745417454
}
1745517455

@@ -17484,7 +17484,7 @@ mod tests {
1748417484
true,
1748517485
2000,
1748617486
).unwrap(),
17487-
2292,
17487+
2284,
1748817488
);
1748917489

1749017490
// negative case, inputs clearly insufficient
@@ -17500,13 +17500,13 @@ mod tests {
1750017500
);
1750117501
assert_eq!(
1750217502
res.err().unwrap(),
17503-
"Total input amount 100000 is lower than needed for contribution 220000, considering fees of 1746. Need more inputs.",
17503+
"Total input amount 100000 is lower than needed for contribution 220000, considering fees of 1740. Need more inputs.",
1750417504
);
1750517505
}
1750617506

1750717507
// barely covers
1750817508
{
17509-
let expected_fee: u64 = 2292;
17509+
let expected_fee: u64 = 2284;
1751017510
assert_eq!(
1751117511
check_v2_funding_inputs_sufficient(
1751217512
(300_000 - expected_fee - 20) as i64,
@@ -17536,13 +17536,13 @@ mod tests {
1753617536
);
1753717537
assert_eq!(
1753817538
res.err().unwrap(),
17539-
"Total input amount 300000 is lower than needed for contribution 298032, considering fees of 2522. Need more inputs.",
17539+
"Total input amount 300000 is lower than needed for contribution 298032, considering fees of 2513. Need more inputs.",
1754017540
);
1754117541
}
1754217542

1754317543
// barely covers, less fees (no extra weight, no init)
1754417544
{
17545-
let expected_fee: u64 = 1092;
17545+
let expected_fee: u64 = 1088;
1754617546
assert_eq!(
1754717547
check_v2_funding_inputs_sufficient(
1754817548
(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
@@ -3351,38 +3351,38 @@ mod tests {
33513351
FundingTxInput::new_p2wpkh(prevtx, 0).unwrap()
33523352
})
33533353
.collect();
3354-
let our_contributed = 110_000;
33553354
let txout = TxOut { value: Amount::from_sat(10_000), script_pubkey: ScriptBuf::new() };
33563355
let outputs = vec![txout];
33573356
let funding_feerate_sat_per_1000_weight = 3000;
33583357

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

33653363
// There is leftover for change
33663364
let context = FundingNegotiationContext {
33673365
is_initiator: true,
3368-
our_funding_contribution: SignedAmount::from_sat(our_contributed as i64),
3366+
our_funding_contribution: SignedAmount::from_sat(110_000),
33693367
funding_tx_locktime: AbsoluteLockTime::ZERO,
33703368
funding_feerate_sat_per_1000_weight,
33713369
shared_funding_input: None,
33723370
our_funding_inputs: inputs,
33733371
our_funding_outputs: outputs,
33743372
change_script: None,
33753373
};
3374+
let gross_change =
3375+
total_inputs - total_outputs - context.our_funding_contribution.to_unsigned().unwrap();
33763376
assert_eq!(
33773377
calculate_change_output_value(&context, false, &ScriptBuf::new(), 300),
3378-
Ok(Some(gross_change - fees - common_fees)),
3378+
Ok(Some((gross_change - fees - common_fees).to_sat())),
33793379
);
33803380

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

33883388
// Insufficient inputs, no leftover
@@ -3413,21 +3413,25 @@ mod tests {
34133413
our_funding_contribution: SignedAmount::from_sat(117_992),
34143414
..context
34153415
};
3416+
let gross_change =
3417+
total_inputs - total_outputs - context.our_funding_contribution.to_unsigned().unwrap();
34163418
assert_eq!(
34173419
calculate_change_output_value(&context, false, &ScriptBuf::new(), 100),
3418-
Ok(Some(262)),
3420+
Ok(Some((gross_change - fees).to_sat())),
34193421
);
34203422

34213423
// Larger fee, smaller change
34223424
let context = FundingNegotiationContext {
34233425
is_initiator: true,
3424-
our_funding_contribution: SignedAmount::from_sat(our_contributed as i64),
3426+
our_funding_contribution: SignedAmount::from_sat(110_000),
34253427
funding_feerate_sat_per_1000_weight: funding_feerate_sat_per_1000_weight * 3,
34263428
..context
34273429
};
3430+
let gross_change =
3431+
total_inputs - total_outputs - context.our_funding_contribution.to_unsigned().unwrap();
34283432
assert_eq!(
34293433
calculate_change_output_value(&context, false, &ScriptBuf::new(), 300),
3430-
Ok(Some(4060)),
3434+
Ok(Some((gross_change - fees * 3 - common_fees * 3).to_sat())),
34313435
);
34323436
}
34333437

lightning/src/sign/mod.rs

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,11 @@ pub mod ecdsa;
8181
pub mod taproot;
8282
pub mod tx_builder;
8383

84+
pub(crate) const COMPRESSED_PUBLIC_KEY_SIZE: usize = bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE;
85+
86+
pub(crate) const MAX_STANDARD_SIGNATURE_SIZE: usize =
87+
bitcoin::secp256k1::constants::MAX_SIGNATURE_SIZE;
88+
8489
/// Information about a spendable output to a P2WSH script.
8590
///
8691
/// See [`SpendableOutputDescriptor::DelayedPaymentOutput`] for more details on how to spend this.
@@ -114,10 +119,12 @@ impl DelayedPaymentOutputDescriptor {
114119
/// The maximum length a well-formed witness spending one of these should have.
115120
/// Note: If you have the grind_signatures feature enabled, this will be at least 1 byte
116121
/// 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;
122+
pub const MAX_WITNESS_LENGTH: u64 = (1 /* witness items */
123+
+ 1 /* sig push */
124+
+ MAX_STANDARD_SIGNATURE_SIZE
125+
+ 1 /* empty vec push */
126+
+ 1 /* redeemscript push */
127+
+ chan_utils::REVOKEABLE_REDEEMSCRIPT_MAX_LENGTH) as u64;
121128
}
122129

123130
impl_writeable_tlv_based!(DelayedPaymentOutputDescriptor, {
@@ -131,15 +138,18 @@ impl_writeable_tlv_based!(DelayedPaymentOutputDescriptor, {
131138
(13, channel_transaction_parameters, (option: ReadableArgs, Some(channel_value_satoshis.0.unwrap()))),
132139
});
133140

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 */;
141+
/// Witness weight for satisfying a P2WPKH spend.
142+
pub(crate) const P2WPKH_WITNESS_WEIGHT: u64 = (1 /* witness items */
143+
+ 1 /* sig push */
144+
+ MAX_STANDARD_SIGNATURE_SIZE
145+
+ 1 /* pubkey push */
146+
+ COMPRESSED_PUBLIC_KEY_SIZE) as u64;
139147

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 */;
148+
/// Witness weight for satisfying a P2TR key-path spend.
149+
pub(crate) const P2TR_KEY_PATH_WITNESS_WEIGHT: u64 = (1 /* witness items */
150+
+ 1 /* sig push */
151+
+ bitcoin::secp256k1::constants::SCHNORR_SIGNATURE_SIZE)
152+
as u64;
143153

144154
/// If a [`KeysManager`] is built with [`KeysManager::new`] with `v2_remote_key_derivation` set
145155
/// (and for all channels after they've been spliced), the script which we receive funds to on-chain
@@ -192,10 +202,16 @@ impl StaticPaymentOutputDescriptor {
192202
/// shorter.
193203
pub fn max_witness_length(&self) -> u64 {
194204
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
205+
let witness_script_weight = 1 /* pubkey push */
206+
+ COMPRESSED_PUBLIC_KEY_SIZE
207+
+ 1 /* OP_CHECKSIGVERIFY */
208+
+ 1 /* OP_1 */
209+
+ 1 /* OP_CHECKSEQUENCEVERIFY */;
210+
(1 /* num witness items */
211+
+ 1 /* sig push */
212+
+ MAX_STANDARD_SIGNATURE_SIZE
213+
+ 1 /* witness script push */
214+
+ witness_script_weight) as u64
199215
} else {
200216
P2WPKH_WITNESS_WEIGHT
201217
}
@@ -511,7 +527,7 @@ impl SpendableOutputDescriptor {
511527
sequence: Sequence::ZERO,
512528
witness: Witness::new(),
513529
});
514-
witness_weight += 1 + 73 + 34;
530+
witness_weight += P2WPKH_WITNESS_WEIGHT;
515531
#[cfg(feature = "grind_signatures")]
516532
{
517533
// Guarantees a low R signature

0 commit comments

Comments
 (0)