Skip to content

Correct non-dust HTLC accounting in next_remote_commit_tx_fee_msat #3933

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5201,14 +5201,14 @@ where
// committed outbound HTLCs, see below.
let mut included_htlcs = 0;
for ref htlc in context.pending_inbound_htlcs.iter() {
if htlc.amount_msat / 1000 <= real_dust_limit_timeout_sat {
if htlc.amount_msat / 1000 < real_dust_limit_timeout_sat {
continue
}
included_htlcs += 1;
}

for ref htlc in context.pending_outbound_htlcs.iter() {
if htlc.amount_msat / 1000 <= real_dust_limit_success_sat {
if htlc.amount_msat / 1000 < real_dust_limit_success_sat {
continue
}
// We only include outbound HTLCs if it will not be included in their next commitment_signed,
Expand Down
299 changes: 299 additions & 0 deletions lightning/src/ln/htlc_reserve_unit_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2078,3 +2078,302 @@ pub fn test_update_fulfill_htlc_bolt2_missing_badonion_bit_for_malformed_htlc_me
let reason = ClosureReason::ProcessingError { err: err_msg.data };
check_closed_event!(nodes[0], 1, reason, [node_b_id], 1000000);
}

#[xtest(feature = "_externalize_tests")]
pub fn test_dust_limit_fee_accounting() {
do_test_dust_limit_fee_accounting(false);
do_test_dust_limit_fee_accounting(true);
}

pub fn do_test_dust_limit_fee_accounting(can_afford: bool) {
// Test that when a channel funder sends HTLCs exactly on the dust limit
// of the funder, the fundee correctly accounts for the additional fee on the
// funder's commitment transaction due to those additional non-dust HTLCs when
// checking for any infrigements on the funder's reserve.

let channel_type = ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies();

let chanmon_cfgs = create_chanmon_cfgs(2);

let mut default_config = test_default_channel_config();
default_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true;
default_config.manually_accept_inbound_channels = true;

let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
let node_chanmgrs =
create_node_chanmgrs(2, &node_cfgs, &[Some(default_config.clone()), Some(default_config)]);

let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);

let node_a_id = nodes[0].node.get_our_node_id();
let node_b_id = nodes[1].node.get_our_node_id();

// Set a HTLC amount that is equal to the dust limit of the funder
const HTLC_AMT_SAT: u64 = 354;

const CHANNEL_VALUE_SAT: u64 = 100_000;

const FEERATE_PER_KW: u32 = 253;

let commit_tx_fee_sat =
chan_utils::commit_tx_fee_sat(FEERATE_PER_KW, MIN_AFFORDABLE_HTLC_COUNT, &channel_type);

// By default the reserve is set to 1% or 1000sat, whichever is higher
let channel_reserve_satoshis = 1_000;

// Set node 0's balance to pay for exactly MIN_AFFORDABLE_HTLC_COUNT non-dust HTLCs on the channel, minus some offset
let node_0_balance_sat = commit_tx_fee_sat
+ channel_reserve_satoshis
+ 2 * crate::ln::channel::ANCHOR_OUTPUT_VALUE_SATOSHI
+ MIN_AFFORDABLE_HTLC_COUNT as u64 * HTLC_AMT_SAT
- if can_afford { 0 } else { 1 };
let mut node_1_balance_sat = CHANNEL_VALUE_SAT - node_0_balance_sat;

let chan_id = create_chan_between_nodes_with_value(
&nodes[0],
&nodes[1],
CHANNEL_VALUE_SAT,
node_1_balance_sat * 1000,
)
.3;

{
// Double check the reserve that node 0 has to maintain here
let per_peer_state_lock;
let mut peer_state_lock;
let chan =
get_channel_ref!(nodes[1], nodes[0], per_peer_state_lock, peer_state_lock, chan_id);
assert_eq!(
chan.funding().holder_selected_channel_reserve_satoshis,
channel_reserve_satoshis
);
}
{
// Double check the dust limit on node 0's commitment transactions; when node 0
// adds a HTLC, node 1 will check that the fee on node 0's commitment transaction
// does not dip under the node 1 selected reserve.
let per_peer_state_lock;
let mut peer_state_lock;
let chan =
get_channel_ref!(nodes[0], nodes[1], per_peer_state_lock, peer_state_lock, chan_id);
assert_eq!(chan.context().holder_dust_limit_satoshis, HTLC_AMT_SAT);
}

// Precompute the route to skip any router complaints when sending the last HTLC
let (route_0_1, payment_hash_0_1, _, payment_secret_0_1) =
get_route_and_payment_hash!(nodes[0], nodes[1], HTLC_AMT_SAT * 1000);

let mut htlcs = Vec::new();
for _ in 0..MIN_AFFORDABLE_HTLC_COUNT - 1 {
let (_payment_preimage, payment_hash, ..) =
route_payment(&nodes[0], &[&nodes[1]], HTLC_AMT_SAT * 1000);
// Grab a snapshot of these HTLCs to manually build the commitment transaction later...
let accepted_htlc = chan_utils::HTLCOutputInCommitment {
offered: false,
amount_msat: HTLC_AMT_SAT * 1000,
// Hard-coded to match the expected value
cltv_expiry: 81,
payment_hash,
transaction_output_index: None,
};
htlcs.push(accepted_htlc);
}

// Need to manually create the update_add_htlc message to go around the channel reserve check in send_htlc()
let secp_ctx = Secp256k1::new();
let session_priv = SecretKey::from_slice(&[42; 32]).expect("RNG is bad!");

let cur_height = nodes[1].node.best_block.read().unwrap().height + 1;

let onion_keys =
onion_utils::construct_onion_keys(&secp_ctx, &route_0_1.paths[0], &session_priv);
let recipient_onion_fields = RecipientOnionFields::secret_only(payment_secret_0_1);
let (onion_payloads, amount_msat, cltv_expiry) = onion_utils::build_onion_payloads(
&route_0_1.paths[0],
HTLC_AMT_SAT * 1000,
&recipient_onion_fields,
cur_height,
&None,
None,
None,
)
.unwrap();
let onion_routing_packet =
onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash_0_1)
.unwrap();
// Double check the hard-coded value
assert_eq!(cltv_expiry, 81);
let msg = msgs::UpdateAddHTLC {
channel_id: chan_id,
htlc_id: MIN_AFFORDABLE_HTLC_COUNT as u64 - 1,
amount_msat,
payment_hash: payment_hash_0_1,
cltv_expiry,
onion_routing_packet,
skimmed_fee_msat: None,
blinding_point: None,
};

nodes[1].node.handle_update_add_htlc(node_a_id, &msg);

if !can_afford {
let err = "Remote HTLC add would put them under remote reserve value".to_string();
nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", &err, 3);
let events = nodes[1].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 2);
let reason = ClosureReason::ProcessingError { err };
check_closed_event(&nodes[1], 1, reason, false, &[node_a_id], CHANNEL_VALUE_SAT);
check_added_monitors(&nodes[1], 1);
} else {
// Now manually create the commitment_signed message corresponding to the update_add
// nodes[0] just sent. In the code for construction of this message, "local" refers
// to the sender of the message, and "remote" refers to the receiver.

const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;

let (local_secret, next_local_point) = {
let per_peer_state = nodes[0].node.per_peer_state.read().unwrap();
let chan_lock = per_peer_state.get(&node_b_id).unwrap().lock().unwrap();
let local_chan =
chan_lock.channel_by_id.get(&chan_id).and_then(Channel::as_funded).unwrap();
let chan_signer = local_chan.get_signer();
// Make the signer believe we validated another commitment, so we can release the secret
chan_signer.as_ecdsa().unwrap().get_enforcement_state().last_holder_commitment -= 1;

(
chan_signer
.as_ref()
.release_commitment_secret(
INITIAL_COMMITMENT_NUMBER - MIN_AFFORDABLE_HTLC_COUNT as u64 + 1,
)
.unwrap(),
chan_signer
.as_ref()
.get_per_commitment_point(
INITIAL_COMMITMENT_NUMBER - MIN_AFFORDABLE_HTLC_COUNT as u64,
&secp_ctx,
)
.unwrap(),
)
};
let remote_point = {
let per_peer_lock;
let mut peer_state_lock;

let channel =
get_channel_ref!(nodes[1], nodes[0], per_peer_lock, peer_state_lock, chan_id);
let chan_signer = channel.as_funded().unwrap().get_signer();
chan_signer
.as_ref()
.get_per_commitment_point(
INITIAL_COMMITMENT_NUMBER - MIN_AFFORDABLE_HTLC_COUNT as u64,
&secp_ctx,
)
.unwrap()
};

// Build the remote commitment transaction so we can sign it, and then later use the
// signature for the commitment_signed message.
let local_chan_balance = node_0_balance_sat
- HTLC_AMT_SAT * MIN_AFFORDABLE_HTLC_COUNT as u64
- 2 * crate::ln::channel::ANCHOR_OUTPUT_VALUE_SATOSHI
- chan_utils::commit_tx_fee_sat(
FEERATE_PER_KW,
MIN_AFFORDABLE_HTLC_COUNT,
&channel_type,
);

let accepted_htlc_info = chan_utils::HTLCOutputInCommitment {
offered: false,
amount_msat: HTLC_AMT_SAT * 1000,
cltv_expiry,
payment_hash: payment_hash_0_1,
transaction_output_index: None,
};
htlcs.push(accepted_htlc_info);

let commitment_number = INITIAL_COMMITMENT_NUMBER - MIN_AFFORDABLE_HTLC_COUNT as u64;

let res = {
let per_peer_lock;
let mut peer_state_lock;

let channel =
get_channel_ref!(nodes[0], nodes[1], per_peer_lock, peer_state_lock, chan_id);
let chan_signer = channel.as_funded().unwrap().get_signer();

let commitment_tx = CommitmentTransaction::new(
commitment_number,
&remote_point,
node_1_balance_sat,
local_chan_balance,
FEERATE_PER_KW,
htlcs,
&channel.funding().channel_transaction_parameters.as_counterparty_broadcastable(),
&secp_ctx,
);
let params = &channel.funding().channel_transaction_parameters;
chan_signer
.as_ecdsa()
.unwrap()
.sign_counterparty_commitment(
params,
&commitment_tx,
Vec::new(),
Vec::new(),
&secp_ctx,
)
.unwrap()
};

let commit_signed_msg = msgs::CommitmentSigned {
channel_id: chan_id,
signature: res.0,
htlc_signatures: res.1,
funding_txid: None,
#[cfg(taproot)]
partial_signature_with_nonce: None,
};

// Send the commitment_signed message to the nodes[1].
nodes[1].node.handle_commitment_signed(node_a_id, &commit_signed_msg);
let _ = nodes[1].node.get_and_clear_pending_msg_events();

// Send the RAA to nodes[1].
let raa_msg = msgs::RevokeAndACK {
channel_id: chan_id,
per_commitment_secret: local_secret,
next_per_commitment_point: next_local_point,
#[cfg(taproot)]
next_local_nonce: None,
};
nodes[1].node.handle_revoke_and_ack(node_a_id, &raa_msg);

// The HTLC actually fails here in `fn validate_commitment_signed` due to a fee spike buffer
// violation. It nonetheless passed all checks in `fn validate_update_add_htlc`.

expect_pending_htlcs_forwardable!(nodes[1]);
expect_htlc_handling_failed_destinations!(
nodes[1].node.get_and_clear_pending_events(),
&[HTLCHandlingFailureType::Receive { payment_hash: payment_hash_0_1 }]
);

let events = nodes[1].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 1);
// Make sure the HTLC failed in the way we expect.
match events[0] {
MessageSendEvent::UpdateHTLCs {
updates: msgs::CommitmentUpdate { ref update_fail_htlcs, .. },
..
} => {
assert_eq!(update_fail_htlcs.len(), 1);
update_fail_htlcs[0].clone()
},
_ => panic!("Unexpected event"),
};
nodes[1].logger.assert_log("lightning::ln::channel",
format!("Attempting to fail HTLC due to fee spike buffer violation in channel {}. Rebalancing is required.", raa_msg.channel_id), 1);

check_added_monitors(&nodes[1], 3);
}
}
Loading