Skip to content

Commit 205c4fc

Browse files
committed
Add validation of the fees predicted by next_commitment_stats
Anytime we build a (feerate, nondust-htlc-count, fee) pair, cache it, and check that the fee matches if the feerate and nondust-htlc-count match when building a commitment transaction.
1 parent 991f248 commit 205c4fc

File tree

1 file changed

+104
-4
lines changed

1 file changed

+104
-4
lines changed

lightning/src/ln/channel.rs

Lines changed: 104 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1984,6 +1984,10 @@ pub(super) struct FundingScope {
19841984
next_local_commitment_tx_fee_info_cached: Mutex<Option<CommitmentTxInfoCached>>,
19851985
#[cfg(any(test, fuzzing))]
19861986
next_remote_commitment_tx_fee_info_cached: Mutex<Option<CommitmentTxInfoCached>>,
1987+
#[cfg(any(test, fuzzing))]
1988+
next_local_fee: Mutex<PredictedNextFee>,
1989+
#[cfg(any(test, fuzzing))]
1990+
next_remote_fee: Mutex<PredictedNextFee>,
19871991

19881992
pub(super) channel_transaction_parameters: ChannelTransactionParameters,
19891993

@@ -2060,6 +2064,10 @@ impl Readable for FundingScope {
20602064
next_local_commitment_tx_fee_info_cached: Mutex::new(None),
20612065
#[cfg(any(test, fuzzing))]
20622066
next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
2067+
#[cfg(any(test, fuzzing))]
2068+
next_local_fee: Mutex::new(PredictedNextFee::default()),
2069+
#[cfg(any(test, fuzzing))]
2070+
next_remote_fee: Mutex::new(PredictedNextFee::default()),
20632071
})
20642072
}
20652073
}
@@ -3206,6 +3214,10 @@ where
32063214
next_local_commitment_tx_fee_info_cached: Mutex::new(None),
32073215
#[cfg(any(test, fuzzing))]
32083216
next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
3217+
#[cfg(any(test, fuzzing))]
3218+
next_local_fee: Mutex::new(PredictedNextFee::default()),
3219+
#[cfg(any(test, fuzzing))]
3220+
next_remote_fee: Mutex::new(PredictedNextFee::default()),
32093221

32103222
channel_transaction_parameters: ChannelTransactionParameters {
32113223
holder_pubkeys: pubkeys,
@@ -3449,6 +3461,10 @@ where
34493461
next_local_commitment_tx_fee_info_cached: Mutex::new(None),
34503462
#[cfg(any(test, fuzzing))]
34513463
next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
3464+
#[cfg(any(test, fuzzing))]
3465+
next_local_fee: Mutex::new(PredictedNextFee::default()),
3466+
#[cfg(any(test, fuzzing))]
3467+
next_remote_fee: Mutex::new(PredictedNextFee::default()),
34523468

34533469
channel_transaction_parameters: ChannelTransactionParameters {
34543470
holder_pubkeys: pubkeys,
@@ -4220,7 +4236,8 @@ where
42204236
include_counterparty_unknown_htlcs,
42214237
);
42224238
let next_value_to_self_msat = self.get_next_commitment_value_to_self_msat(true, funding);
4223-
SpecTxBuilder {}.get_next_commitment_stats(
4239+
4240+
let ret = SpecTxBuilder {}.get_next_commitment_stats(
42244241
true,
42254242
funding.is_outbound(),
42264243
funding.get_value_satoshis(),
@@ -4231,7 +4248,38 @@ where
42314248
dust_exposure_limiting_feerate,
42324249
self.holder_dust_limit_satoshis,
42334250
funding.get_channel_type(),
4234-
)
4251+
);
4252+
4253+
#[cfg(any(test, fuzzing))]
4254+
{
4255+
if addl_nondust_htlc_count == 0 {
4256+
*funding.next_local_fee.lock().unwrap() = PredictedNextFee {
4257+
predicted_feerate: feerate_per_kw,
4258+
predicted_nondust_htlc_count: ret.nondust_htlc_count,
4259+
predicted_fee_sat: ret.commit_tx_fee_sat,
4260+
};
4261+
} else {
4262+
let predicted_stats = SpecTxBuilder {}.get_next_commitment_stats(
4263+
true,
4264+
funding.is_outbound(),
4265+
funding.get_value_satoshis(),
4266+
next_value_to_self_msat,
4267+
&next_commitment_htlcs,
4268+
0,
4269+
feerate_per_kw,
4270+
dust_exposure_limiting_feerate,
4271+
self.holder_dust_limit_satoshis,
4272+
funding.get_channel_type(),
4273+
);
4274+
*funding.next_local_fee.lock().unwrap() = PredictedNextFee {
4275+
predicted_feerate: feerate_per_kw,
4276+
predicted_nondust_htlc_count: predicted_stats.nondust_htlc_count,
4277+
predicted_fee_sat: predicted_stats.commit_tx_fee_sat,
4278+
};
4279+
}
4280+
}
4281+
4282+
ret
42354283
}
42364284

42374285
fn get_next_remote_commitment_stats(
@@ -4245,7 +4293,8 @@ where
42454293
include_counterparty_unknown_htlcs,
42464294
);
42474295
let next_value_to_self_msat = self.get_next_commitment_value_to_self_msat(false, funding);
4248-
SpecTxBuilder {}.get_next_commitment_stats(
4296+
4297+
let ret = SpecTxBuilder {}.get_next_commitment_stats(
42494298
false,
42504299
funding.is_outbound(),
42514300
funding.get_value_satoshis(),
@@ -4256,7 +4305,38 @@ where
42564305
dust_exposure_limiting_feerate,
42574306
self.counterparty_dust_limit_satoshis,
42584307
funding.get_channel_type(),
4259-
)
4308+
);
4309+
4310+
#[cfg(any(test, fuzzing))]
4311+
{
4312+
if addl_nondust_htlc_count == 0 {
4313+
*funding.next_remote_fee.lock().unwrap() = PredictedNextFee {
4314+
predicted_feerate: feerate_per_kw,
4315+
predicted_nondust_htlc_count: ret.nondust_htlc_count,
4316+
predicted_fee_sat: ret.commit_tx_fee_sat,
4317+
};
4318+
} else {
4319+
let predicted_stats = SpecTxBuilder {}.get_next_commitment_stats(
4320+
false,
4321+
funding.is_outbound(),
4322+
funding.get_value_satoshis(),
4323+
next_value_to_self_msat,
4324+
&next_commitment_htlcs,
4325+
0,
4326+
feerate_per_kw,
4327+
dust_exposure_limiting_feerate,
4328+
self.counterparty_dust_limit_satoshis,
4329+
funding.get_channel_type(),
4330+
);
4331+
*funding.next_remote_fee.lock().unwrap() = PredictedNextFee {
4332+
predicted_feerate: feerate_per_kw,
4333+
predicted_nondust_htlc_count: predicted_stats.nondust_htlc_count,
4334+
predicted_fee_sat: predicted_stats.commit_tx_fee_sat,
4335+
};
4336+
}
4337+
}
4338+
4339+
ret
42604340
}
42614341

42624342
#[rustfmt::skip]
@@ -4413,6 +4493,10 @@ where
44134493
}
44144494
}
44154495
}
4496+
let PredictedNextFee { predicted_feerate, predicted_nondust_htlc_count, predicted_fee_sat } = *funding.next_local_fee.lock().unwrap();
4497+
if predicted_feerate == commitment_data.tx.feerate_per_kw() && predicted_nondust_htlc_count == commitment_data.tx.nondust_htlcs().len() {
4498+
assert_eq!(predicted_fee_sat, commitment_data.stats.commit_tx_fee_sat);
4499+
}
44164500
}
44174501

44184502
if msg.htlc_signatures.len() != commitment_data.tx.nondust_htlcs().len() {
@@ -6026,6 +6110,14 @@ struct CommitmentTxInfoCached {
60266110
feerate: u32,
60276111
}
60286112

6113+
#[cfg(any(test, fuzzing))]
6114+
#[derive(Clone, Copy, Default)]
6115+
struct PredictedNextFee {
6116+
predicted_feerate: u32,
6117+
predicted_nondust_htlc_count: usize,
6118+
predicted_fee_sat: u64,
6119+
}
6120+
60296121
/// Contents of a wire message that fails an HTLC backwards. Useful for [`FundedChannel::fail_htlc`] to
60306122
/// fail with either [`msgs::UpdateFailMalformedHTLC`] or [`msgs::UpdateFailHTLC`] as needed.
60316123
trait FailHTLCContents {
@@ -10990,6 +11082,10 @@ where
1099011082
}
1099111083
}
1099211084
}
11085+
let PredictedNextFee { predicted_feerate, predicted_nondust_htlc_count, predicted_fee_sat } = *funding.next_remote_fee.lock().unwrap();
11086+
if predicted_feerate == counterparty_commitment_tx.feerate_per_kw() && predicted_nondust_htlc_count == counterparty_commitment_tx.nondust_htlcs().len() {
11087+
assert_eq!(predicted_fee_sat, commitment_data.stats.commit_tx_fee_sat);
11088+
}
1099311089
}
1099411090

1099511091
(commitment_data.htlcs_included, counterparty_commitment_tx)
@@ -13615,6 +13711,10 @@ where
1361513711
next_local_commitment_tx_fee_info_cached: Mutex::new(None),
1361613712
#[cfg(any(test, fuzzing))]
1361713713
next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
13714+
#[cfg(any(test, fuzzing))]
13715+
next_local_fee: Mutex::new(PredictedNextFee::default()),
13716+
#[cfg(any(test, fuzzing))]
13717+
next_remote_fee: Mutex::new(PredictedNextFee::default()),
1361813718

1361913719
channel_transaction_parameters: channel_parameters,
1362013720
funding_transaction,

0 commit comments

Comments
 (0)