Skip to content

Commit c9b30cb

Browse files
tankyleotankyleo
committed
Use helper functions in commitment stats calculations and tx builds
This commit is solely a refactor, no functional changes are intended.
1 parent 31300c3 commit c9b30cb

File tree

1 file changed

+125
-92
lines changed

1 file changed

+125
-92
lines changed

‎lightning/src/ln/channel.rs

+125-92
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,36 @@ impl From<&InboundHTLCState> for Option<InboundHTLCStateDetails> {
223223
}
224224
}
225225

226+
impl InboundHTLCState {
227+
fn as_str(&self) -> &str {
228+
match self {
229+
InboundHTLCState::RemoteAnnounced(_) => "RemoteAnnounced",
230+
InboundHTLCState::AwaitingRemoteRevokeToAnnounce(_) => "AwaitingRemoteRevokeToAnnounce",
231+
InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) => "AwaitingAnnouncedRemoteRevoke",
232+
InboundHTLCState::Committed => "Committed",
233+
InboundHTLCState::LocalRemoved(_) => "LocalRemoved",
234+
}
235+
}
236+
237+
fn preimage(&self) -> Option<PaymentPreimage> {
238+
match self {
239+
InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(preimage)) => Some(*preimage),
240+
_ => None,
241+
}
242+
}
243+
244+
// Determines whether a HTLC is present on a commitment transaction, either as a dust or non-dust HTLC
245+
fn included_in_commitment(&self, generated_by_local: bool) -> bool {
246+
match self {
247+
InboundHTLCState::RemoteAnnounced(_) => !generated_by_local,
248+
InboundHTLCState::AwaitingRemoteRevokeToAnnounce(_) => !generated_by_local,
249+
InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) => true,
250+
InboundHTLCState::Committed => true,
251+
InboundHTLCState::LocalRemoved(_) => !generated_by_local,
252+
}
253+
}
254+
}
255+
226256
struct InboundHTLCOutput {
227257
htlc_id: u64,
228258
amount_msat: u64,
@@ -231,6 +261,23 @@ struct InboundHTLCOutput {
231261
state: InboundHTLCState,
232262
}
233263

264+
impl InboundHTLCOutput {
265+
fn is_dust(&self, local: bool, feerate_per_kw: u32, broadcaster_dust_limit_sat: u64, features: &ChannelTypeFeatures) -> bool {
266+
let htlc_tx_fee = if features.supports_anchors_zero_fee_htlc_tx() {
267+
0
268+
} else {
269+
let htlc_tx_weight = if !local {
270+
// this is an offered htlc
271+
htlc_timeout_tx_weight(features)
272+
} else {
273+
htlc_success_tx_weight(features)
274+
};
275+
feerate_per_kw as u64 * htlc_tx_weight / 1000
276+
};
277+
self.amount_msat / 1000 < broadcaster_dust_limit_sat + htlc_tx_fee
278+
}
279+
}
280+
234281
#[cfg_attr(test, derive(Clone, Debug, PartialEq))]
235282
enum OutboundHTLCState {
236283
/// Added by us and included in a commitment_signed (if we were AwaitingRemoteRevoke when we
@@ -287,6 +334,38 @@ impl From<&OutboundHTLCState> for OutboundHTLCStateDetails {
287334
}
288335
}
289336

337+
impl OutboundHTLCState {
338+
fn as_str(&self) -> &str {
339+
match self {
340+
OutboundHTLCState::LocalAnnounced(_) => "LocalAnnounced",
341+
OutboundHTLCState::Committed => "Committed",
342+
OutboundHTLCState::RemoteRemoved(_) => "RemoteRemoved",
343+
OutboundHTLCState::AwaitingRemoteRevokeToRemove(_) => "AwaitingRemoteRevokeToRemove",
344+
OutboundHTLCState::AwaitingRemovedRemoteRevoke(_) => "AwaitingRemovedRemoteRevoke",
345+
}
346+
}
347+
348+
fn preimage(&self) -> Option<PaymentPreimage> {
349+
match self {
350+
OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success(p)) => p.as_ref().copied(),
351+
OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(p)) => p.as_ref().copied(),
352+
OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(p)) => p.as_ref().copied(),
353+
_ => None,
354+
}
355+
}
356+
357+
// Determines whether a HTLC is present on a commitment transaction, either as a dust or non-dust HTLC
358+
fn included_in_commitment(&self, generated_by_local: bool) -> bool {
359+
match self {
360+
OutboundHTLCState::LocalAnnounced(_) => generated_by_local,
361+
OutboundHTLCState::Committed => true,
362+
OutboundHTLCState::RemoteRemoved(_) => generated_by_local,
363+
OutboundHTLCState::AwaitingRemoteRevokeToRemove(_) => generated_by_local,
364+
OutboundHTLCState::AwaitingRemovedRemoteRevoke(_) => false,
365+
}
366+
}
367+
}
368+
290369
#[derive(Clone)]
291370
#[cfg_attr(test, derive(Debug, PartialEq))]
292371
enum OutboundHTLCOutcome {
@@ -325,6 +404,23 @@ struct OutboundHTLCOutput {
325404
skimmed_fee_msat: Option<u64>,
326405
}
327406

407+
impl OutboundHTLCOutput {
408+
fn is_dust(&self, local: bool, feerate_per_kw: u32, broadcaster_dust_limit_sat: u64, features: &ChannelTypeFeatures) -> bool {
409+
let htlc_tx_fee = if features.supports_anchors_zero_fee_htlc_tx() {
410+
0
411+
} else {
412+
let htlc_tx_weight = if local {
413+
// this is an offered htlc
414+
htlc_timeout_tx_weight(features)
415+
} else {
416+
htlc_success_tx_weight(features)
417+
};
418+
feerate_per_kw as u64 * htlc_tx_weight / 1000
419+
};
420+
self.amount_msat / 1000 < broadcaster_dust_limit_sat + htlc_tx_fee
421+
}
422+
}
423+
328424
/// See AwaitingRemoteRevoke ChannelState for more info
329425
#[cfg_attr(test, derive(Clone, Debug, PartialEq))]
330426
enum HTLCUpdateAwaitingACK {
@@ -3635,46 +3731,28 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
36353731
}
36363732

36373733
log_trace!(logger, "Building commitment stats for channel {} for {}, generated by {} with fee {}...",
3638-
&self.channel_id,
3734+
self.channel_id,
36393735
if local { "us" } else { "remote" }, if generated_by_local { "us" } else { "remote" }, feerate_per_kw);
36403736

36413737
macro_rules! count_nondust_htlc {
3642-
($htlc: expr, $outbound: expr, $state_name: expr) => {
3643-
let offered = $outbound == local;
3644-
let htlc_tx_fee = if self.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
3645-
0
3738+
($htlc: expr, $outbound: expr) => {
3739+
if $htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_sat, self.get_channel_type()) {
3740+
log_trace!(logger, " ...not counting {} {} dust HTLC {} (hash {}) with value {} due to dust limit", if $outbound { "outbound" } else { "inbound" }, $htlc.state.as_str(), $htlc.htlc_id, $htlc.payment_hash, $htlc.amount_msat);
36463741
} else {
3647-
feerate_per_kw as u64 * if offered {
3648-
htlc_timeout_tx_weight(self.get_channel_type())
3649-
} else {
3650-
htlc_success_tx_weight(self.get_channel_type())
3651-
} / 1000
3652-
};
3653-
if $htlc.amount_msat / 1000 >= broadcaster_dust_limit_sat + htlc_tx_fee {
3654-
log_trace!(logger, " ...counting {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, &$htlc.payment_hash, $htlc.amount_msat);
3742+
log_trace!(logger, " ...counting {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $htlc.state.as_str(), $htlc.htlc_id, $htlc.payment_hash, $htlc.amount_msat);
36553743
non_dust_htlc_count += 1;
3656-
} else {
3657-
log_trace!(logger, " ...not counting {} {} dust HTLC {} (hash {}) with value {} due to dust limit", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, &$htlc.payment_hash, $htlc.amount_msat);
36583744
}
36593745
}
36603746
}
36613747

36623748
for ref htlc in self.pending_inbound_htlcs.iter() {
3663-
let (include, state_name) = match htlc.state {
3664-
InboundHTLCState::RemoteAnnounced(_) => (!generated_by_local, "RemoteAnnounced"),
3665-
InboundHTLCState::AwaitingRemoteRevokeToAnnounce(_) => (!generated_by_local, "AwaitingRemoteRevokeToAnnounce"),
3666-
InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) => (true, "AwaitingAnnouncedRemoteRevoke"),
3667-
InboundHTLCState::Committed => (true, "Committed"),
3668-
InboundHTLCState::LocalRemoved(_) => (!generated_by_local, "LocalRemoved"),
3669-
};
3670-
3671-
if include {
3672-
count_nondust_htlc!(htlc, false, state_name);
3749+
if htlc.state.included_in_commitment(generated_by_local) {
3750+
count_nondust_htlc!(htlc, false);
36733751
remote_htlc_total_msat += htlc.amount_msat;
36743752
} else {
3675-
log_trace!(logger, " ...not counting inbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, &htlc.payment_hash, htlc.amount_msat, state_name);
3676-
match &htlc.state {
3677-
&InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(_preimage)) => {
3753+
log_trace!(logger, " ...not counting inbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, htlc.payment_hash, htlc.amount_msat, htlc.state.as_str());
3754+
match htlc.state {
3755+
InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(_preimage)) => {
36783756
value_to_self_msat_offset += htlc.amount_msat as i64;
36793757
},
36803758
_ => {},
@@ -3683,19 +3761,11 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
36833761
}
36843762

36853763
for ref htlc in self.pending_outbound_htlcs.iter() {
3686-
let (include, state_name) = match htlc.state {
3687-
OutboundHTLCState::LocalAnnounced(_) => (generated_by_local, "LocalAnnounced"),
3688-
OutboundHTLCState::Committed => (true, "Committed"),
3689-
OutboundHTLCState::RemoteRemoved(_) => (generated_by_local, "RemoteRemoved"),
3690-
OutboundHTLCState::AwaitingRemoteRevokeToRemove(_) => (generated_by_local, "AwaitingRemoteRevokeToRemove"),
3691-
OutboundHTLCState::AwaitingRemovedRemoteRevoke(_) => (false, "AwaitingRemovedRemoteRevoke"),
3692-
};
3693-
3694-
if include {
3695-
count_nondust_htlc!(htlc, true, state_name);
3764+
if htlc.state.included_in_commitment(generated_by_local) {
3765+
count_nondust_htlc!(htlc, true);
36963766
local_htlc_total_msat += htlc.amount_msat;
36973767
} else {
3698-
log_trace!(logger, " ...not counting outbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, &htlc.payment_hash, htlc.amount_msat, state_name);
3768+
log_trace!(logger, " ...not counting outbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, htlc.payment_hash, htlc.amount_msat, htlc.state.as_str());
36993769
match htlc.state {
37003770
OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_)) |
37013771
OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) |
@@ -3781,7 +3851,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
37813851
log_trace!(logger, "Building commitment transaction number {} (really {} xor {}) for channel {} for {}, generated by {} with fee {}...",
37823852
commitment_number, (INITIAL_COMMITMENT_NUMBER - commitment_number),
37833853
get_commitment_transaction_number_obscure_factor(&funding.get_holder_pubkeys().payment_point, &funding.get_counterparty_pubkeys().payment_point, funding.is_outbound()),
3784-
&self.channel_id,
3854+
self.channel_id,
37853855
if local { "us" } else { "remote" }, if generated_by_local { "us" } else { "remote" }, feerate_per_kw);
37863856

37873857
macro_rules! get_htlc_in_commitment {
@@ -3797,78 +3867,41 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
37973867
}
37983868

37993869
macro_rules! add_htlc_output {
3800-
($htlc: expr, $outbound: expr, $source: expr, $state_name: expr) => {
3801-
let offered = $outbound == local;
3802-
let htlc_in_tx = get_htlc_in_commitment!($htlc, offered);
3803-
let htlc_tx_fee = if self.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
3804-
0
3870+
($htlc: expr, $outbound: expr, $source: expr) => {
3871+
let htlc_in_tx = get_htlc_in_commitment!($htlc, $outbound == local);
3872+
if $htlc.is_dust(local, feerate_per_kw, broadcaster_dust_limit_sat, self.get_channel_type()) {
3873+
log_trace!(logger, " ...including {} {} dust HTLC {} (hash {}) with value {} due to dust limit", if $outbound { "outbound" } else { "inbound" }, $htlc.state.as_str(), $htlc.htlc_id, $htlc.payment_hash, $htlc.amount_msat);
3874+
included_dust_htlcs.push((htlc_in_tx, $source));
38053875
} else {
3806-
feerate_per_kw as u64 * if offered {
3807-
htlc_timeout_tx_weight(self.get_channel_type())
3808-
} else {
3809-
htlc_success_tx_weight(self.get_channel_type())
3810-
} / 1000
3811-
};
3812-
if $htlc.amount_msat / 1000 >= broadcaster_dust_limit_sat + htlc_tx_fee {
3813-
log_trace!(logger, " ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, &$htlc.payment_hash, $htlc.amount_msat);
3876+
log_trace!(logger, " ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $htlc.state.as_str(), $htlc.htlc_id, $htlc.payment_hash, $htlc.amount_msat);
38143877
included_non_dust_htlcs.push((htlc_in_tx, $source));
3815-
} else {
3816-
log_trace!(logger, " ...including {} {} dust HTLC {} (hash {}) with value {} due to dust limit", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, &$htlc.payment_hash, $htlc.amount_msat);
3817-
included_dust_htlcs.push((htlc_in_tx, $source));
38183878
}
38193879
}
38203880
}
38213881

38223882
let mut inbound_htlc_preimages: Vec<PaymentPreimage> = Vec::new();
38233883

38243884
for ref htlc in self.pending_inbound_htlcs.iter() {
3825-
let (include, state_name) = match htlc.state {
3826-
InboundHTLCState::RemoteAnnounced(_) => (!generated_by_local, "RemoteAnnounced"),
3827-
InboundHTLCState::AwaitingRemoteRevokeToAnnounce(_) => (!generated_by_local, "AwaitingRemoteRevokeToAnnounce"),
3828-
InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) => (true, "AwaitingAnnouncedRemoteRevoke"),
3829-
InboundHTLCState::Committed => (true, "Committed"),
3830-
InboundHTLCState::LocalRemoved(_) => (!generated_by_local, "LocalRemoved"),
3831-
};
3832-
3833-
if include {
3834-
add_htlc_output!(htlc, false, None, state_name);
3885+
if htlc.state.included_in_commitment(generated_by_local) {
3886+
add_htlc_output!(htlc, false, None);
38353887
} else {
3836-
log_trace!(logger, " ...not including inbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, &htlc.payment_hash, htlc.amount_msat, state_name);
3837-
match &htlc.state {
3838-
&InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(preimage)) => {
3839-
inbound_htlc_preimages.push(preimage);
3840-
},
3841-
_ => {},
3888+
log_trace!(logger, " ...not including inbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, htlc.payment_hash, htlc.amount_msat, htlc.state.as_str());
3889+
if let Some(preimage) = htlc.state.preimage() {
3890+
inbound_htlc_preimages.push(preimage);
38423891
}
38433892
}
38443893
}
38453894

38463895
let mut outbound_htlc_preimages: Vec<PaymentPreimage> = Vec::new();
38473896

38483897
for ref htlc in self.pending_outbound_htlcs.iter() {
3849-
let (include, state_name) = match htlc.state {
3850-
OutboundHTLCState::LocalAnnounced(_) => (generated_by_local, "LocalAnnounced"),
3851-
OutboundHTLCState::Committed => (true, "Committed"),
3852-
OutboundHTLCState::RemoteRemoved(_) => (generated_by_local, "RemoteRemoved"),
3853-
OutboundHTLCState::AwaitingRemoteRevokeToRemove(_) => (generated_by_local, "AwaitingRemoteRevokeToRemove"),
3854-
OutboundHTLCState::AwaitingRemovedRemoteRevoke(_) => (false, "AwaitingRemovedRemoteRevoke"),
3855-
};
3856-
3857-
let preimage_opt = match htlc.state {
3858-
OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success(p)) => p,
3859-
OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(p)) => p,
3860-
OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(p)) => p,
3861-
_ => None,
3862-
};
3863-
3864-
if let Some(preimage) = preimage_opt {
3898+
if let Some(preimage) = htlc.state.preimage() {
38653899
outbound_htlc_preimages.push(preimage);
38663900
}
3867-
3868-
if include {
3869-
add_htlc_output!(htlc, true, Some(&htlc.source), state_name);
3901+
if htlc.state.included_in_commitment(generated_by_local) {
3902+
add_htlc_output!(htlc, true, Some(&htlc.source));
38703903
} else {
3871-
log_trace!(logger, " ...not including outbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, &htlc.payment_hash, htlc.amount_msat, state_name);
3904+
log_trace!(logger, " ...not including outbound HTLC {} (hash {}) with value {} due to state ({})", htlc.htlc_id, htlc.payment_hash, htlc.amount_msat, htlc.state.as_str());
38723905
}
38733906
}
38743907

0 commit comments

Comments
 (0)