Skip to content

Commit

Permalink
Fix cmp::max execution in ChannelContext::get_dust_buffer_feerate
Browse files Browse the repository at this point in the history
  The current `cmp::max` doesnt align with the function comment, ie its
  comparing 2530 and `feerate_plus_quarter` instead of `feerate_per_kw
  + 2530` and `feerate_plus_quarter` which is fixed in this commit
  • Loading branch information
jbesraa committed Mar 27, 2024
1 parent 68d5e88 commit 30e08c5
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 7 deletions.
2 changes: 1 addition & 1 deletion lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2699,7 +2699,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
feerate_per_kw = cmp::max(feerate_per_kw, feerate);
}
let feerate_plus_quarter = feerate_per_kw.checked_mul(1250).map(|v| v / 1000);
cmp::max(2530, feerate_plus_quarter.unwrap_or(u32::max_value()))
cmp::max(feerate_per_kw + 2530, feerate_plus_quarter.unwrap_or(u32::max_value()))
}

/// Get forwarding information for the counterparty.
Expand Down
17 changes: 11 additions & 6 deletions lightning/src/ln/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9891,8 +9891,8 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
// Default test fee estimator rate is 253 sat/kw, so we set the multiplier to 5_000_000 / 253
// to get roughly the same initial value as the default setting when this test was
// originally written.
MaxDustHTLCExposure::FeeRateMultiplier(5_000_000 / 253)
} else { MaxDustHTLCExposure::FixedLimitMsat(5_000_000) }; // initial default setting value
MaxDustHTLCExposure::FeeRateMultiplier(6_000_000 / 253)
} else { MaxDustHTLCExposure::FixedLimitMsat(6_000_000) }; // initial default setting value
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config), None]);
let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
Expand All @@ -9901,6 +9901,8 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
let mut open_channel = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id());
open_channel.common_fields.max_htlc_value_in_flight_msat = 50_000_000;
open_channel.common_fields.max_accepted_htlcs = 60;
// --

if on_holder_tx {
open_channel.common_fields.dust_limit_satoshis = 546;
}
Expand Down Expand Up @@ -9935,18 +9937,21 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
let (channel_ready, channel_id) = create_chan_between_nodes_with_value_confirm(&nodes[0], &nodes[1], &tx);
let (announcement, as_update, bs_update) = create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &channel_ready);
update_nodes_with_chan_announce(&nodes, 0, 1, &announcement, &as_update, &bs_update);
// Node A < - > Node B channel is now established

// Fetch a route in advance as we will be unable to once we're unable to send.
let (mut route, payment_hash, _, payment_secret) =
get_route_and_payment_hash!(nodes[0], nodes[1], 1000);


let (dust_buffer_feerate, max_dust_htlc_exposure_msat) = {
let per_peer_state = nodes[0].node.per_peer_state.read().unwrap();
let chan_lock = per_peer_state.get(&nodes[1].node.get_our_node_id()).unwrap().lock().unwrap();
let chan = chan_lock.channel_by_id.get(&channel_id).unwrap();
(chan.context().get_dust_buffer_feerate(None) as u64,
chan.context().get_max_dust_htlc_exposure_msat(&LowerBoundedFeeEstimator(nodes[0].fee_estimator)))
};

let dust_outbound_htlc_on_holder_tx_msat: u64 = (dust_buffer_feerate * htlc_timeout_tx_weight(&channel_type_features) / 1000 + open_channel.common_fields.dust_limit_satoshis - 1) * 1000;
let dust_outbound_htlc_on_holder_tx: u64 = max_dust_htlc_exposure_msat / dust_outbound_htlc_on_holder_tx_msat;

Expand Down Expand Up @@ -10032,7 +10037,7 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
// For the multiplier dust exposure limit, since it scales with feerate,
// we need to add a lot of HTLCs that will become dust at the new feerate
// to cross the threshold.
for _ in 0..20 {
for _ in 0..30 {
let (_, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[1], Some(1_000), None);
nodes[0].node.send_payment_with_route(&route, payment_hash,
RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap();
Expand All @@ -10054,12 +10059,12 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
fn do_test_max_dust_htlc_exposure_by_threshold_type(multiplier_dust_limit: bool) {
do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCForward, true, multiplier_dust_limit);
do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCForward, true, multiplier_dust_limit);
do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCForward, false, multiplier_dust_limit);
do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCForward, false, multiplier_dust_limit);
do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCReception, true, multiplier_dust_limit);
do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCReception, false, multiplier_dust_limit);
do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCForward, false, multiplier_dust_limit);
do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCReception, false, multiplier_dust_limit);
do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCReception, true, multiplier_dust_limit);
do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCForward, false, multiplier_dust_limit);
do_test_max_dust_htlc_exposure(true, ExposureEvent::AtUpdateFeeOutbound, true, multiplier_dust_limit);
do_test_max_dust_htlc_exposure(true, ExposureEvent::AtUpdateFeeOutbound, false, multiplier_dust_limit);
do_test_max_dust_htlc_exposure(false, ExposureEvent::AtUpdateFeeOutbound, false, multiplier_dust_limit);
Expand All @@ -10068,7 +10073,7 @@ fn do_test_max_dust_htlc_exposure_by_threshold_type(multiplier_dust_limit: bool)

#[test]
fn test_max_dust_htlc_exposure() {
do_test_max_dust_htlc_exposure_by_threshold_type(false);
do_test_max_dust_htlc_exposure_by_threshold_type(false);
do_test_max_dust_htlc_exposure_by_threshold_type(true);
}

Expand Down

0 comments on commit 30e08c5

Please sign in to comment.