Skip to content

Commit dd46f9c

Browse files
committed
Add validation of the fees predicted by next_commitment_stats
Anytime we build a (feerate, HTLC-set, fee) pair, cache it, and check that the fee matches if the feerate and HTLC-set match when building a commitment transaction. The fee in the old caches are never actually checked in the current test suite; both of the new cached fees are checked in ~200 tests.
1 parent a386f6f commit dd46f9c

File tree

1 file changed

+119
-0
lines changed

1 file changed

+119
-0
lines changed

lightning/src/ln/channel.rs

Lines changed: 119 additions & 0 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,
@@ -4322,6 +4338,25 @@ where
43224338
}
43234339
}
43244340

4341+
#[cfg(any(test, fuzzing))]
4342+
{
4343+
let mut local_predicted_htlcs = next_local_commitment_stats.next_commitment_htlcs;
4344+
local_predicted_htlcs.sort_unstable();
4345+
*funding.next_local_fee.lock().unwrap() = PredictedNextFee {
4346+
predicted_feerate: self.feerate_per_kw,
4347+
predicted_htlcs: local_predicted_htlcs,
4348+
predicted_fee_sat: next_local_commitment_stats.commit_tx_fee_sat,
4349+
};
4350+
4351+
let mut remote_predicted_htlcs = next_remote_commitment_stats.next_commitment_htlcs;
4352+
remote_predicted_htlcs.sort_unstable();
4353+
*funding.next_remote_fee.lock().unwrap() = PredictedNextFee {
4354+
predicted_feerate: self.feerate_per_kw,
4355+
predicted_htlcs: remote_predicted_htlcs,
4356+
predicted_fee_sat: next_remote_commitment_stats.commit_tx_fee_sat,
4357+
};
4358+
}
4359+
43254360
Ok(())
43264361
}
43274362

@@ -4352,6 +4387,25 @@ where
43524387
msg.feerate_per_kw, next_remote_commitment_stats.dust_exposure_msat)));
43534388
}
43544389

4390+
#[cfg(any(test, fuzzing))]
4391+
{
4392+
let mut local_predicted_htlcs = next_local_commitment_stats.next_commitment_htlcs;
4393+
local_predicted_htlcs.sort_unstable();
4394+
*funding.next_local_fee.lock().unwrap() = PredictedNextFee {
4395+
predicted_feerate: msg.feerate_per_kw,
4396+
predicted_htlcs: local_predicted_htlcs,
4397+
predicted_fee_sat: next_local_commitment_stats.commit_tx_fee_sat,
4398+
};
4399+
4400+
let mut remote_predicted_htlcs = next_remote_commitment_stats.next_commitment_htlcs;
4401+
remote_predicted_htlcs.sort_unstable();
4402+
*funding.next_remote_fee.lock().unwrap() = PredictedNextFee {
4403+
predicted_feerate: msg.feerate_per_kw,
4404+
predicted_htlcs: remote_predicted_htlcs,
4405+
predicted_fee_sat: next_remote_commitment_stats.commit_tx_fee_sat,
4406+
};
4407+
}
4408+
43554409
Ok(())
43564410
}
43574411

@@ -4411,6 +4465,12 @@ where
44114465
}
44124466
}
44134467
}
4468+
let PredictedNextFee { predicted_feerate, predicted_htlcs, predicted_fee_sat } = funding.next_local_fee.lock().unwrap().clone();
4469+
let mut actual_nondust_htlcs: Vec<_> = commitment_data.tx.nondust_htlcs().iter().map(|&HTLCOutputInCommitment { offered, amount_msat, .. } | HTLCAmountDirection { outbound: offered, amount_msat }).collect();
4470+
actual_nondust_htlcs.sort_unstable();
4471+
if predicted_feerate == commitment_data.tx.feerate_per_kw() && predicted_htlcs == actual_nondust_htlcs {
4472+
assert_eq!(predicted_fee_sat, commitment_data.stats.commit_tx_fee_sat);
4473+
}
44144474
}
44154475

44164476
if msg.htlc_signatures.len() != commitment_data.tx.nondust_htlcs().len() {
@@ -4483,6 +4543,18 @@ where
44834543
return false;
44844544
}
44854545

4546+
#[cfg(any(test, fuzzing))]
4547+
{
4548+
let next_remote_commitment_stats = self.get_next_remote_commitment_stats(funding, None, include_counterparty_unknown_htlcs, 0, feerate_per_kw, dust_exposure_limiting_feerate);
4549+
let mut remote_predicted_htlcs = next_remote_commitment_stats.next_commitment_htlcs;
4550+
remote_predicted_htlcs.sort_unstable();
4551+
*funding.next_remote_fee.lock().unwrap() = PredictedNextFee {
4552+
predicted_feerate: feerate_per_kw,
4553+
predicted_htlcs: remote_predicted_htlcs,
4554+
predicted_fee_sat: next_remote_commitment_stats.commit_tx_fee_sat,
4555+
};
4556+
}
4557+
44864558
return true;
44874559
}
44884560

@@ -4537,6 +4609,35 @@ where
45374609
}
45384610
}
45394611

4612+
#[cfg(any(test, fuzzing))]
4613+
{
4614+
let next_local_commitment_stats = if fee_spike_buffer_htlc == 1 {
4615+
self.get_next_local_commitment_stats(funding, None, do_not_include_counterparty_unknown_htlcs, 0, self.feerate_per_kw, dust_exposure_limiting_feerate)
4616+
} else {
4617+
next_local_commitment_stats
4618+
};
4619+
let mut local_predicted_htlcs = next_local_commitment_stats.next_commitment_htlcs;
4620+
local_predicted_htlcs.sort_unstable();
4621+
*funding.next_local_fee.lock().unwrap() = PredictedNextFee {
4622+
predicted_feerate: self.feerate_per_kw,
4623+
predicted_htlcs: local_predicted_htlcs,
4624+
predicted_fee_sat: next_local_commitment_stats.commit_tx_fee_sat,
4625+
};
4626+
4627+
let next_remote_commitment_stats = if fee_spike_buffer_htlc == 1 {
4628+
self.get_next_remote_commitment_stats(funding, None, do_not_include_counterparty_unknown_htlcs, 0, self.feerate_per_kw, dust_exposure_limiting_feerate)
4629+
} else {
4630+
next_remote_commitment_stats
4631+
};
4632+
let mut remote_predicted_htlcs = next_remote_commitment_stats.next_commitment_htlcs;
4633+
remote_predicted_htlcs.sort_unstable();
4634+
*funding.next_remote_fee.lock().unwrap() = PredictedNextFee {
4635+
predicted_feerate: self.feerate_per_kw,
4636+
predicted_htlcs: remote_predicted_htlcs,
4637+
predicted_fee_sat: next_remote_commitment_stats.commit_tx_fee_sat,
4638+
};
4639+
}
4640+
45404641
Ok(())
45414642
}
45424643

@@ -6026,6 +6127,14 @@ struct CommitmentTxInfoCached {
60266127
feerate: u32,
60276128
}
60286129

6130+
#[cfg(any(test, fuzzing))]
6131+
#[derive(Clone, Default)]
6132+
struct PredictedNextFee {
6133+
predicted_feerate: u32,
6134+
predicted_htlcs: Vec<HTLCAmountDirection>,
6135+
predicted_fee_sat: u64,
6136+
}
6137+
60296138
/// Contents of a wire message that fails an HTLC backwards. Useful for [`FundedChannel::fail_htlc`] to
60306139
/// fail with either [`msgs::UpdateFailMalformedHTLC`] or [`msgs::UpdateFailHTLC`] as needed.
60316140
trait FailHTLCContents {
@@ -10990,6 +11099,12 @@ where
1099011099
}
1099111100
}
1099211101
}
11102+
let PredictedNextFee { predicted_feerate, predicted_htlcs, predicted_fee_sat } = funding.next_remote_fee.lock().unwrap().clone();
11103+
let mut actual_nondust_htlcs: Vec<_> = counterparty_commitment_tx.nondust_htlcs().iter().map(|&HTLCOutputInCommitment { offered, amount_msat, .. }| HTLCAmountDirection { outbound: !offered, amount_msat }).collect();
11104+
actual_nondust_htlcs.sort_unstable();
11105+
if predicted_feerate == counterparty_commitment_tx.feerate_per_kw() && predicted_htlcs == actual_nondust_htlcs {
11106+
assert_eq!(predicted_fee_sat, commitment_data.stats.commit_tx_fee_sat);
11107+
}
1099311108
}
1099411109

1099511110
(commitment_data.htlcs_included, counterparty_commitment_tx)
@@ -13615,6 +13730,10 @@ where
1361513730
next_local_commitment_tx_fee_info_cached: Mutex::new(None),
1361613731
#[cfg(any(test, fuzzing))]
1361713732
next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
13733+
#[cfg(any(test, fuzzing))]
13734+
next_local_fee: Mutex::new(PredictedNextFee::default()),
13735+
#[cfg(any(test, fuzzing))]
13736+
next_remote_fee: Mutex::new(PredictedNextFee::default()),
1361813737

1361913738
channel_transaction_parameters: channel_parameters,
1362013739
funding_transaction,

0 commit comments

Comments
 (0)