Skip to content

Commit 280c1a4

Browse files
committed
Remove i64 casts in ChannelContext::build_commitment_transaction
Instead of converting operands to `i64` and checking if the subtractions overflowed by checking if the `i64` is smaller than zero, we instead choose to do checked and saturating subtractions on the original unsigned integers.
1 parent 2d8e724 commit 280c1a4

File tree

1 file changed

+26
-25
lines changed

1 file changed

+26
-25
lines changed

lightning/src/ln/channel.rs

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3764,14 +3764,15 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
37643764
}
37653765
}
37663766

3767-
let value_to_self_msat: i64 = (funding.value_to_self_msat - local_htlc_total_msat) as i64 + value_to_self_msat_offset;
3768-
assert!(value_to_self_msat >= 0);
3767+
// TODO: When MSRV >= 1.66.0, use u64::checked_add_signed
3768+
let mut value_to_self_msat = u64::try_from(funding.value_to_self_msat as i64 + value_to_self_msat_offset).unwrap();
37693769
// Note that in case they have several just-awaiting-last-RAA fulfills in-progress (ie
37703770
// AwaitingRemoteRevokeToRemove or AwaitingRemovedRemoteRevoke) we may have allowed them to
3771-
// "violate" their reserve value by couting those against it. Thus, we have to convert
3772-
// everything to i64 before subtracting as otherwise we can overflow.
3773-
let value_to_remote_msat: i64 = (funding.get_value_satoshis() * 1000) as i64 - (funding.value_to_self_msat as i64) - (remote_htlc_total_msat as i64) - value_to_self_msat_offset;
3774-
assert!(value_to_remote_msat >= 0);
3771+
// "violate" their reserve value by couting those against it. Thus, we have to do checked subtraction
3772+
// as otherwise we can overflow.
3773+
let mut value_to_remote_msat = u64::checked_sub(funding.get_value_satoshis() * 1000, value_to_self_msat).unwrap();
3774+
value_to_self_msat = u64::checked_sub(value_to_self_msat, local_htlc_total_msat).unwrap();
3775+
value_to_remote_msat = u64::checked_sub(value_to_remote_msat, remote_htlc_total_msat).unwrap();
37753776

37763777
#[cfg(debug_assertions)]
37773778
{
@@ -3782,33 +3783,33 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
37823783
} else {
37833784
funding.counterparty_max_commitment_tx_output.lock().unwrap()
37843785
};
3785-
debug_assert!(broadcaster_max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= funding.counterparty_selected_channel_reserve_satoshis.unwrap() as i64);
3786-
broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, value_to_self_msat as u64);
3787-
debug_assert!(broadcaster_max_commitment_tx_output.1 <= value_to_remote_msat as u64 || value_to_remote_msat / 1000 >= funding.holder_selected_channel_reserve_satoshis as i64);
3788-
broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat as u64);
3786+
debug_assert!(broadcaster_max_commitment_tx_output.0 <= value_to_self_msat || value_to_self_msat / 1000 >= funding.counterparty_selected_channel_reserve_satoshis.unwrap());
3787+
broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, value_to_self_msat);
3788+
debug_assert!(broadcaster_max_commitment_tx_output.1 <= value_to_remote_msat || value_to_remote_msat / 1000 >= funding.holder_selected_channel_reserve_satoshis);
3789+
broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat);
37893790
}
37903791

37913792
let total_fee_sat = commit_tx_fee_sat(feerate_per_kw, included_non_dust_htlcs.len(), &funding.channel_transaction_parameters.channel_type_features);
3792-
let anchors_val = if funding.channel_transaction_parameters.channel_type_features.supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 } as i64;
3793+
let anchors_val = if funding.channel_transaction_parameters.channel_type_features.supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 };
37933794
let (value_to_self, value_to_remote) = if funding.is_outbound() {
3794-
(value_to_self_msat / 1000 - anchors_val - total_fee_sat as i64, value_to_remote_msat / 1000)
3795+
((value_to_self_msat / 1000).saturating_sub(anchors_val).saturating_sub(total_fee_sat), value_to_remote_msat / 1000)
37953796
} else {
3796-
(value_to_self_msat / 1000, value_to_remote_msat / 1000 - anchors_val - total_fee_sat as i64)
3797+
(value_to_self_msat / 1000, (value_to_remote_msat / 1000).saturating_sub(anchors_val).saturating_sub(total_fee_sat))
37973798
};
37983799

3799-
let mut value_to_a = if local { value_to_self } else { value_to_remote };
3800-
let mut value_to_b = if local { value_to_remote } else { value_to_self };
3800+
let mut to_broadcaster_value_sat = if local { value_to_self } else { value_to_remote };
3801+
let mut to_countersignatory_value_sat = if local { value_to_remote } else { value_to_self };
38013802

3802-
if value_to_a >= (broadcaster_dust_limit_satoshis as i64) {
3803-
log_trace!(logger, " ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, value_to_a);
3803+
if to_broadcaster_value_sat >= broadcaster_dust_limit_satoshis {
3804+
log_trace!(logger, " ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, to_broadcaster_value_sat);
38043805
} else {
3805-
value_to_a = 0;
3806+
to_broadcaster_value_sat = 0;
38063807
}
38073808

3808-
if value_to_b >= (broadcaster_dust_limit_satoshis as i64) {
3809-
log_trace!(logger, " ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, value_to_b);
3809+
if to_countersignatory_value_sat >= broadcaster_dust_limit_satoshis {
3810+
log_trace!(logger, " ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, to_countersignatory_value_sat);
38103811
} else {
3811-
value_to_b = 0;
3812+
to_countersignatory_value_sat = 0;
38123813
}
38133814

38143815
let num_nondust_htlcs = included_non_dust_htlcs.len();
@@ -3818,8 +3819,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
38183819
else { funding.channel_transaction_parameters.as_counterparty_broadcastable() };
38193820
let tx = CommitmentTransaction::new_with_auxiliary_htlc_data(commitment_number,
38203821
per_commitment_point,
3821-
value_to_a as u64,
3822-
value_to_b as u64,
3822+
to_broadcaster_value_sat,
3823+
to_countersignatory_value_sat,
38233824
feerate_per_kw,
38243825
&mut included_non_dust_htlcs,
38253826
&channel_parameters,
@@ -3836,8 +3837,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
38363837
total_fee_sat,
38373838
num_nondust_htlcs,
38383839
htlcs_included,
3839-
local_balance_msat: value_to_self_msat as u64,
3840-
remote_balance_msat: value_to_remote_msat as u64,
3840+
local_balance_msat: value_to_self_msat,
3841+
remote_balance_msat: value_to_remote_msat,
38413842
inbound_htlc_preimages,
38423843
outbound_htlc_preimages,
38433844
}

0 commit comments

Comments
 (0)