Skip to content

Commit 21e1bf5

Browse files
committed
Fix cmp::max execution in ChannelContext::get_dust_buffer_feerate
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
1 parent 68d5e88 commit 21e1bf5

File tree

2 files changed

+20
-21
lines changed

2 files changed

+20
-21
lines changed

lightning/src/ln/channel.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2699,7 +2699,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
26992699
feerate_per_kw = cmp::max(feerate_per_kw, feerate);
27002700
}
27012701
let feerate_plus_quarter = feerate_per_kw.checked_mul(1250).map(|v| v / 1000);
2702-
cmp::max(2530, feerate_plus_quarter.unwrap_or(u32::max_value()))
2702+
cmp::max(feerate_per_kw + 2530, feerate_plus_quarter.unwrap_or(u32::max_value()))
27032703
}
27042704

27052705
/// Get forwarding information for the counterparty.

lightning/src/ln/functional_tests.rs

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9888,11 +9888,10 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
98889888
let chanmon_cfgs = create_chanmon_cfgs(2);
98899889
let mut config = test_default_channel_config();
98909890
config.channel_config.max_dust_htlc_exposure = if multiplier_dust_limit {
9891-
// Default test fee estimator rate is 253 sat/kw, so we set the multiplier to 5_000_000 / 253
9892-
// to get roughly the same initial value as the default setting when this test was
9893-
// originally written.
9894-
MaxDustHTLCExposure::FeeRateMultiplier(5_000_000 / 253)
9895-
} else { MaxDustHTLCExposure::FixedLimitMsat(5_000_000) }; // initial default setting value
9891+
// Default test fee estimator rate is 253 sat/kw. so we set the multiplier to 6_000_000 /
9892+
// 253 to accomdate 253 sat/kw plus an additional 2530 sat/kw for the dust buffer fee rate.
9893+
MaxDustHTLCExposure::FeeRateMultiplier(6_000_000 / 253)
9894+
} else { MaxDustHTLCExposure::FixedLimitMsat(6_000_000) };
98969895
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
98979896
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config), None]);
98989897
let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
@@ -9958,34 +9957,34 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
99589957

99599958
if on_holder_tx {
99609959
if dust_outbound_balance {
9961-
// Outbound dust threshold: 2223 sats (`dust_buffer_feerate` * HTLC_TIMEOUT_TX_WEIGHT / 1000 + holder's `dust_limit_satoshis`)
9962-
// Outbound dust balance: 4372 sats
9963-
// Note, we need sent payment to be above outbound dust threshold on counterparty_tx of 2132 sats
9960+
// Outbound dust threshold: 2390 sats (`dust_buffer_feerate` * HTLC_TIMEOUT_TX_WEIGHT / 1000 + holder's `dust_limit_satoshis`)
9961+
// Outbound dust balance: 4780 sats(dust_outbound_htlc_on_holder_tx * dust_outbound_htlc_on_holder_tx_msat)
9962+
// Note, we need sent payment to be above outbound dust threshold on counterparty_tx of 1500 sats(dust_htlc_on_counterparty_tx * dust_htlc_on_counterparty_tx_msat)
99649963
for _ in 0..dust_outbound_htlc_on_holder_tx {
99659964
let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], dust_outbound_htlc_on_holder_tx_msat);
99669965
nodes[0].node.send_payment_with_route(&route, payment_hash,
99679966
RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap();
99689967
}
99699968
} else {
9970-
// Inbound dust threshold: 2324 sats (`dust_buffer_feerate` * HTLC_SUCCESS_TX_WEIGHT / 1000 + holder's `dust_limit_satoshis`)
9971-
// Inbound dust balance: 4372 sats
9972-
// Note, we need sent payment to be above outbound dust threshold on counterparty_tx of 2031 sats
9969+
// Inbound dust threshold: 2501 sats (`dust_buffer_feerate` * HTLC_SUCCESS_TX_WEIGHT / 1000 + holder's `dust_limit_satoshis`)
9970+
// Inbound dust balance: 5002 sats(dust_inbound_htlc_on_holder_tx * dust_inbound_htlc_on_holder_tx_msat)
9971+
// Note, we need sent payment to be above outbound dust threshold on counterparty_tx of 1500 sats(dust_htlc_on_counterparty_tx * dust_htlc_on_counterparty_tx_msat)
99739972
for _ in 0..dust_inbound_htlc_on_holder_tx {
99749973
route_payment(&nodes[1], &[&nodes[0]], dust_inbound_htlc_on_holder_tx_msat);
99759974
}
99769975
}
99779976
} else {
99789977
if dust_outbound_balance {
9979-
// Outbound dust threshold: 2132 sats (`dust_buffer_feerate` * HTLC_TIMEOUT_TX_WEIGHT / 1000 + counteparty's `dust_limit_satoshis`)
9980-
// Outbound dust balance: 5000 sats
9978+
// Outbound dust threshold: 2198 sats (`dust_buffer_feerate` * HTLC_TIMEOUT_TX_WEIGHT / 1000 + counteparty's `dust_limit_satoshis`)
9979+
// Outbound dust balance: 4396 sats(dust_outbound_htlc_on_holder_tx * dust_outbound_htlc_on_holder_tx_msat)
99819980
for _ in 0..dust_htlc_on_counterparty_tx - 1 {
99829981
let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], dust_htlc_on_counterparty_tx_msat);
99839982
nodes[0].node.send_payment_with_route(&route, payment_hash,
99849983
RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap();
99859984
}
99869985
} else {
9987-
// Inbound dust threshold: 2031 sats (`dust_buffer_feerate` * HTLC_TIMEOUT_TX_WEIGHT / 1000 + counteparty's `dust_limit_satoshis`)
9988-
// Inbound dust balance: 5000 sats
9986+
// Inbound dust threshold: 2309 sats (`dust_buffer_feerate` * HTLC_SUCCESS_TX_WEIGHT / 1000 + counteparty's `dust_limit_satoshis`)
9987+
// Inbound dust balance: 4618 sats(dust_inbound_htlc_on_holder_tx * dust_inbound_htlc_on_holder_tx_msat)
99899988
for _ in 0..dust_htlc_on_counterparty_tx - 1 {
99909989
route_payment(&nodes[1], &[&nodes[0]], dust_htlc_on_counterparty_tx_msat);
99919990
}
@@ -10016,12 +10015,12 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
1001610015
nodes[0].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event.msgs[0]);
1001710016
// With default dust exposure: 5000 sats
1001810017
if on_holder_tx {
10019-
// Outbound dust balance: 6399 sats
10018+
// Outbound dust balance: 7503 sats (dust_outbound_htlc_on_holder_tx * dust_outbound_htlc_on_holder_tx_msat + dust_inbound_htlc_on_holder_tx_msat)
1002010019
let dust_inbound_overflow = dust_inbound_htlc_on_holder_tx_msat * (dust_inbound_htlc_on_holder_tx + 1);
1002110020
let dust_outbound_overflow = dust_outbound_htlc_on_holder_tx_msat * dust_outbound_htlc_on_holder_tx + dust_inbound_htlc_on_holder_tx_msat;
1002210021
nodes[0].logger.assert_log("lightning::ln::channel", format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", if dust_outbound_balance { dust_outbound_overflow } else { dust_inbound_overflow }, max_dust_htlc_exposure_msat), 1);
1002310022
} else {
10024-
// Outbound dust balance: 5200 sats
10023+
// Outbound dust balance: 6000 sats(dust_htlc_on_counterparty_tx * dust_htlc_on_counterparty_tx_msat)
1002510024
nodes[0].logger.assert_log("lightning::ln::channel",
1002610025
format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx",
1002710026
dust_htlc_on_counterparty_tx_msat * (dust_htlc_on_counterparty_tx - 1) + dust_htlc_on_counterparty_tx_msat + 4,
@@ -10032,7 +10031,7 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
1003210031
// For the multiplier dust exposure limit, since it scales with feerate,
1003310032
// we need to add a lot of HTLCs that will become dust at the new feerate
1003410033
// to cross the threshold.
10035-
for _ in 0..20 {
10034+
for _ in 0..30 {
1003610035
let (_, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[1], Some(1_000), None);
1003710036
nodes[0].node.send_payment_with_route(&route, payment_hash,
1003810037
RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap();
@@ -10054,12 +10053,12 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e
1005410053
fn do_test_max_dust_htlc_exposure_by_threshold_type(multiplier_dust_limit: bool) {
1005510054
do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCForward, true, multiplier_dust_limit);
1005610055
do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCForward, true, multiplier_dust_limit);
10056+
do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCForward, false, multiplier_dust_limit);
10057+
do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCForward, false, multiplier_dust_limit);
1005710058
do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCReception, true, multiplier_dust_limit);
1005810059
do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCReception, false, multiplier_dust_limit);
10059-
do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCForward, false, multiplier_dust_limit);
1006010060
do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCReception, false, multiplier_dust_limit);
1006110061
do_test_max_dust_htlc_exposure(true, ExposureEvent::AtHTLCReception, true, multiplier_dust_limit);
10062-
do_test_max_dust_htlc_exposure(false, ExposureEvent::AtHTLCForward, false, multiplier_dust_limit);
1006310062
do_test_max_dust_htlc_exposure(true, ExposureEvent::AtUpdateFeeOutbound, true, multiplier_dust_limit);
1006410063
do_test_max_dust_htlc_exposure(true, ExposureEvent::AtUpdateFeeOutbound, false, multiplier_dust_limit);
1006510064
do_test_max_dust_htlc_exposure(false, ExposureEvent::AtUpdateFeeOutbound, false, multiplier_dust_limit);

0 commit comments

Comments
 (0)