Skip to content

Correct payment resolution after on chain failure of dust HTLCs #1691

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.

Sign up for GitHub

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 2 commits into from
Sep 7, 2022
Merged

Correct payment resolution after on chain failure of dust HTLCs #1691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8ae6c1
Move open_zero_conf_channel utility to common test utils
TheBlueMatt Sep 2, 2022
a795e45
Correct payment resolution after on chain failure of dust HTLCs
TheBlueMatt Sep 2, 2022
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
56 changes: 41 additions & 15 deletions lightning/src/chain/channelmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ pub enum Balance {
/// An HTLC which has been irrevocably resolved on-chain, and has reached ANTI_REORG_DELAY.
#[derive(PartialEq)]
struct IrrevocablyResolvedHTLC {
commitment_tx_output_idx: u32,
commitment_tx_output_idx: Option<u32>,
/// The txid of the transaction which resolved the HTLC, this may be a commitment (if the HTLC
/// was not present in the confirmed commitment transaction), HTLC-Success, or HTLC-Timeout
/// transaction.
Expand All @@ -628,11 +628,39 @@ struct IrrevocablyResolvedHTLC {
payment_preimage: Option<PaymentPreimage>,
}

impl_writeable_tlv_based!(IrrevocablyResolvedHTLC, {
(0, commitment_tx_output_idx, required),
(1, resolving_txid, option),
(2, payment_preimage, option),
});
// In LDK versions prior to 0.0.111 commitment_tx_output_idx was not Option-al and
// IrrevocablyResolvedHTLC objects only existed for non-dust HTLCs. This was a bug, but to maintain
// backwards compatibility we must ensure we always write out a commitment_tx_output_idx field,
// using `u32::max_value()` as a sentinal to indicate the HTLC was dust.
impl Writeable for IrrevocablyResolvedHTLC {
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
let mapped_commitment_tx_output_idx = self.commitment_tx_output_idx.unwrap_or(u32::max_value());
write_tlv_fields!(writer, {
(0, mapped_commitment_tx_output_idx, required),
(1, self.resolving_txid, option),
(2, self.payment_preimage, option),
});
Ok(())
}
}

impl Readable for IrrevocablyResolvedHTLC {
fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
let mut mapped_commitment_tx_output_idx = 0;
let mut resolving_txid = None;
let mut payment_preimage = None;
read_tlv_fields!(reader, {
(0, mapped_commitment_tx_output_idx, required),
(1, resolving_txid, option),
(2, payment_preimage, option),
});
Ok(Self {
commitment_tx_output_idx: if mapped_commitment_tx_output_idx == u32::max_value() { None } else { Some(mapped_commitment_tx_output_idx) },
resolving_txid,
payment_preimage,
})
}
}

/// A ChannelMonitor handles chain events (blocks connected and disconnected) and generates
/// on-chain transactions to ensure no loss of funds occurs.
Expand Down Expand Up @@ -1485,7 +1513,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
}
}
let htlc_resolved = self.htlcs_resolved_on_chain.iter()
.find(|v| if v.commitment_tx_output_idx == htlc_commitment_tx_output_idx {
.find(|v| if v.commitment_tx_output_idx == Some(htlc_commitment_tx_output_idx) {
debug_assert!(htlc_spend_txid_opt.is_none());
htlc_spend_txid_opt = v.resolving_txid;
true
Expand Down Expand Up @@ -1775,7 +1803,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
macro_rules! walk_htlcs {
($holder_commitment: expr, $htlc_iter: expr) => {
for (htlc, source) in $htlc_iter {
if us.htlcs_resolved_on_chain.iter().any(|v| Some(v.commitment_tx_output_idx) == htlc.transaction_output_index) {
if us.htlcs_resolved_on_chain.iter().any(|v| v.commitment_tx_output_idx == htlc.transaction_output_index) {
// We should assert that funding_spend_confirmed is_some() here, but we
// have some unit tests which violate HTLC transaction CSVs entirely and
// would fail.
Expand Down Expand Up @@ -2902,12 +2930,10 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
source: source.clone(),
htlc_value_satoshis,
}));
if let Some(idx) = commitment_tx_output_idx {
self.htlcs_resolved_on_chain.push(IrrevocablyResolvedHTLC {
commitment_tx_output_idx: idx, resolving_txid: Some(entry.txid),
payment_preimage: None,
});
}
self.htlcs_resolved_on_chain.push(IrrevocablyResolvedHTLC {
commitment_tx_output_idx, resolving_txid: Some(entry.txid),
payment_preimage: None,
});
},
OnchainEvent::MaturingOutput { descriptor } => {
log_debug!(logger, "Descriptor {} has got enough confirmations to be passed upstream", log_spendable!(descriptor));
Expand All @@ -2917,7 +2943,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
},
OnchainEvent::HTLCSpendConfirmation { commitment_tx_output_idx, preimage, .. } => {
self.htlcs_resolved_on_chain.push(IrrevocablyResolvedHTLC {
commitment_tx_output_idx, resolving_txid: Some(entry.txid),
commitment_tx_output_idx: Some(commitment_tx_output_idx), resolving_txid: Some(entry.txid),
payment_preimage: preimage,
});
},
Expand Down
66 changes: 66 additions & 0 deletions lightning/src/ln/functional_test_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,72 @@ pub fn sign_funding_transaction<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &
tx
}

// Receiver must have been initialized with manually_accept_inbound_channels set to true.
pub fn open_zero_conf_channel<'a, 'b, 'c, 'd>(initiator: &'a Node<'b, 'c, 'd>, receiver: &'a Node<'b, 'c, 'd>, initiator_config: Option<UserConfig>) -> (bitcoin::Transaction, [u8; 32]) {
let initiator_channels = initiator.node.list_usable_channels().len();
let receiver_channels = receiver.node.list_usable_channels().len();

initiator.node.create_channel(receiver.node.get_our_node_id(), 100_000, 10_001, 42, initiator_config).unwrap();
let open_channel = get_event_msg!(initiator, MessageSendEvent::SendOpenChannel, receiver.node.get_our_node_id());

receiver.node.handle_open_channel(&initiator.node.get_our_node_id(), InitFeatures::known(), &open_channel);
let events = receiver.node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::OpenChannelRequest { temporary_channel_id, .. } => {
receiver.node.accept_inbound_channel_from_trusted_peer_0conf(&temporary_channel_id, &initiator.node.get_our_node_id(), 0).unwrap();
},
_ => panic!("Unexpected event"),
};

let accept_channel = get_event_msg!(receiver, MessageSendEvent::SendAcceptChannel, initiator.node.get_our_node_id());
assert_eq!(accept_channel.minimum_depth, 0);
initiator.node.handle_accept_channel(&receiver.node.get_our_node_id(), InitFeatures::known(), &accept_channel);

let (temporary_channel_id, tx, _) = create_funding_transaction(&initiator, &receiver.node.get_our_node_id(), 100_000, 42);
initiator.node.funding_transaction_generated(&temporary_channel_id, &receiver.node.get_our_node_id(), tx.clone()).unwrap();
let funding_created = get_event_msg!(initiator, MessageSendEvent::SendFundingCreated, receiver.node.get_our_node_id());

receiver.node.handle_funding_created(&initiator.node.get_our_node_id(), &funding_created);
check_added_monitors!(receiver, 1);
let bs_signed_locked = receiver.node.get_and_clear_pending_msg_events();
assert_eq!(bs_signed_locked.len(), 2);
let as_channel_ready;
match &bs_signed_locked[0] {
MessageSendEvent::SendFundingSigned { node_id, msg } => {
assert_eq!(*node_id, initiator.node.get_our_node_id());
initiator.node.handle_funding_signed(&receiver.node.get_our_node_id(), &msg);
check_added_monitors!(initiator, 1);

assert_eq!(initiator.tx_broadcaster.txn_broadcasted.lock().unwrap().len(), 1);
assert_eq!(initiator.tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0)[0], tx);

as_channel_ready = get_event_msg!(initiator, MessageSendEvent::SendChannelReady, receiver.node.get_our_node_id());
}
_ => panic!("Unexpected event"),
}
match &bs_signed_locked[1] {
MessageSendEvent::SendChannelReady { node_id, msg } => {
assert_eq!(*node_id, initiator.node.get_our_node_id());
initiator.node.handle_channel_ready(&receiver.node.get_our_node_id(), &msg);
}
_ => panic!("Unexpected event"),
}

receiver.node.handle_channel_ready(&initiator.node.get_our_node_id(), &as_channel_ready);

let as_channel_update = get_event_msg!(initiator, MessageSendEvent::SendChannelUpdate, receiver.node.get_our_node_id());
let bs_channel_update = get_event_msg!(receiver, MessageSendEvent::SendChannelUpdate, initiator.node.get_our_node_id());

initiator.node.handle_channel_update(&receiver.node.get_our_node_id(), &bs_channel_update);
receiver.node.handle_channel_update(&initiator.node.get_our_node_id(), &as_channel_update);

assert_eq!(initiator.node.list_usable_channels().len(), initiator_channels + 1);
assert_eq!(receiver.node.list_usable_channels().len(), receiver_channels + 1);

(tx, as_channel_ready.channel_id)
}

pub fn create_chan_between_nodes_with_value_init<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, channel_value: u64, push_msat: u64, a_flags: InitFeatures, b_flags: InitFeatures) -> Transaction {
let create_chan_id = node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42, None).unwrap();
let open_channel_msg = get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id());
Expand Down
Loading