Skip to content

Commit 205dc20

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 61fd4eb commit 205dc20

File tree

1 file changed

+20
-19
lines changed

1 file changed

+20
-19
lines changed

lightning/src/ln/channel.rs

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3736,14 +3736,15 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
37363736
}
37373737
}
37383738

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

37483749
#[cfg(debug_assertions)]
37493750
{
@@ -3754,30 +3755,30 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
37543755
} else {
37553756
funding.counterparty_max_commitment_tx_output.lock().unwrap()
37563757
};
3757-
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);
3758-
broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, value_to_self_msat as u64);
3759-
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);
3760-
broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat as u64);
3758+
debug_assert!(broadcaster_max_commitment_tx_output.0 <= value_to_self_msat || value_to_self_msat / 1000 >= funding.counterparty_selected_channel_reserve_satoshis.unwrap());
3759+
broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, value_to_self_msat);
3760+
debug_assert!(broadcaster_max_commitment_tx_output.1 <= value_to_remote_msat || value_to_remote_msat / 1000 >= funding.holder_selected_channel_reserve_satoshis);
3761+
broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat);
37613762
}
37623763

37633764
let total_fee_sat = commit_tx_fee_sat(feerate_per_kw, included_non_dust_htlcs.len(), &funding.channel_transaction_parameters.channel_type_features);
3764-
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;
3765+
let anchors_val = if funding.channel_transaction_parameters.channel_type_features.supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 };
37653766
let (value_to_self, value_to_remote) = if funding.is_outbound() {
3766-
(value_to_self_msat / 1000 - anchors_val - total_fee_sat as i64, value_to_remote_msat / 1000)
3767+
((value_to_self_msat / 1000).saturating_sub(anchors_val).saturating_sub(total_fee_sat), value_to_remote_msat / 1000)
37673768
} else {
3768-
(value_to_self_msat / 1000, value_to_remote_msat / 1000 - anchors_val - total_fee_sat as i64)
3769+
(value_to_self_msat / 1000, (value_to_remote_msat / 1000).saturating_sub(anchors_val).saturating_sub(total_fee_sat))
37693770
};
37703771

37713772
let mut value_to_a = if local { value_to_self } else { value_to_remote };
37723773
let mut value_to_b = if local { value_to_remote } else { value_to_self };
37733774

3774-
if value_to_a >= (broadcaster_dust_limit_satoshis as i64) {
3775+
if value_to_a >= broadcaster_dust_limit_satoshis {
37753776
log_trace!(logger, " ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, value_to_a);
37763777
} else {
37773778
value_to_a = 0;
37783779
}
37793780

3780-
if value_to_b >= (broadcaster_dust_limit_satoshis as i64) {
3781+
if value_to_b >= broadcaster_dust_limit_satoshis {
37813782
log_trace!(logger, " ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, value_to_b);
37823783
} else {
37833784
value_to_b = 0;
@@ -3790,8 +3791,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
37903791
else { funding.channel_transaction_parameters.as_counterparty_broadcastable() };
37913792
let tx = CommitmentTransaction::new(commitment_number,
37923793
&per_commitment_point,
3793-
value_to_a as u64,
3794-
value_to_b as u64,
3794+
value_to_a,
3795+
value_to_b,
37953796
feerate_per_kw,
37963797
included_non_dust_htlcs.iter_mut().map(|(htlc, _)| htlc).collect(),
37973798
&channel_parameters,
@@ -3808,8 +3809,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
38083809
total_fee_sat,
38093810
num_nondust_htlcs,
38103811
htlcs_included,
3811-
local_balance_msat: value_to_self_msat as u64,
3812-
remote_balance_msat: value_to_remote_msat as u64,
3812+
local_balance_msat: value_to_self_msat,
3813+
remote_balance_msat: value_to_remote_msat,
38133814
inbound_htlc_preimages,
38143815
outbound_htlc_preimages,
38153816
}

0 commit comments

Comments
 (0)