Skip to content

Commit d8b9f6b

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 1c4a496 commit d8b9f6b

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
@@ -3760,14 +3760,15 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
37603760
}
37613761
}
37623762

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

37723773
#[cfg(debug_assertions)]
37733774
{
@@ -3778,33 +3779,33 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
37783779
} else {
37793780
funding.counterparty_max_commitment_tx_output.lock().unwrap()
37803781
};
3781-
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);
3782-
broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, value_to_self_msat as u64);
3783-
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);
3784-
broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat as u64);
3782+
debug_assert!(broadcaster_max_commitment_tx_output.0 <= value_to_self_msat || value_to_self_msat / 1000 >= funding.counterparty_selected_channel_reserve_satoshis.unwrap());
3783+
broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, value_to_self_msat);
3784+
debug_assert!(broadcaster_max_commitment_tx_output.1 <= value_to_remote_msat || value_to_remote_msat / 1000 >= funding.holder_selected_channel_reserve_satoshis);
3785+
broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat);
37853786
}
37863787

37873788
let total_fee_sat = commit_tx_fee_sat(feerate_per_kw, included_non_dust_htlcs.len(), &funding.channel_transaction_parameters.channel_type_features);
3788-
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;
3789+
let anchors_val = if funding.channel_transaction_parameters.channel_type_features.supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 };
37893790
let (value_to_self, value_to_remote) = if funding.is_outbound() {
3790-
(value_to_self_msat / 1000 - anchors_val - total_fee_sat as i64, value_to_remote_msat / 1000)
3791+
((value_to_self_msat / 1000).saturating_sub(anchors_val).saturating_sub(total_fee_sat), value_to_remote_msat / 1000)
37913792
} else {
3792-
(value_to_self_msat / 1000, value_to_remote_msat / 1000 - anchors_val - total_fee_sat as i64)
3793+
(value_to_self_msat / 1000, (value_to_remote_msat / 1000).saturating_sub(anchors_val).saturating_sub(total_fee_sat))
37933794
};
37943795

3795-
let mut value_to_a = if local { value_to_self } else { value_to_remote };
3796-
let mut value_to_b = if local { value_to_remote } else { value_to_self };
3796+
let mut to_broadcaster_value_sat = if local { value_to_self } else { value_to_remote };
3797+
let mut to_countersignatory_value_sat = if local { value_to_remote } else { value_to_self };
37973798

3798-
if value_to_a >= (broadcaster_dust_limit_satoshis as i64) {
3799-
log_trace!(logger, " ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, value_to_a);
3799+
if to_broadcaster_value_sat >= broadcaster_dust_limit_satoshis {
3800+
log_trace!(logger, " ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, to_broadcaster_value_sat);
38003801
} else {
3801-
value_to_a = 0;
3802+
to_broadcaster_value_sat = 0;
38023803
}
38033804

3804-
if value_to_b >= (broadcaster_dust_limit_satoshis as i64) {
3805-
log_trace!(logger, " ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, value_to_b);
3805+
if to_countersignatory_value_sat >= broadcaster_dust_limit_satoshis {
3806+
log_trace!(logger, " ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, to_countersignatory_value_sat);
38063807
} else {
3807-
value_to_b = 0;
3808+
to_countersignatory_value_sat = 0;
38083809
}
38093810

38103811
let num_nondust_htlcs = included_non_dust_htlcs.len();
@@ -3814,8 +3815,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
38143815
else { funding.channel_transaction_parameters.as_counterparty_broadcastable() };
38153816
let tx = CommitmentTransaction::new_with_auxiliary_htlc_data(commitment_number,
38163817
per_commitment_point,
3817-
value_to_a as u64,
3818-
value_to_b as u64,
3818+
to_broadcaster_value_sat,
3819+
to_countersignatory_value_sat,
38193820
feerate_per_kw,
38203821
&mut included_non_dust_htlcs,
38213822
&channel_parameters,
@@ -3832,8 +3833,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
38323833
total_fee_sat,
38333834
num_nondust_htlcs,
38343835
htlcs_included,
3835-
local_balance_msat: value_to_self_msat as u64,
3836-
remote_balance_msat: value_to_remote_msat as u64,
3836+
local_balance_msat: value_to_self_msat,
3837+
remote_balance_msat: value_to_remote_msat,
38373838
inbound_htlc_preimages,
38383839
outbound_htlc_preimages,
38393840
}

0 commit comments

Comments
 (0)