Improve Robustness of Inbound MPP Claims Across Restart#1434

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
valentinewallace merged 9 commits into from
May 30, 2022
Merged

Improve Robustness of Inbound MPP Claims Across Restart #1434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2a42595
Enable removal of `OnionPayload::Invoice::_legacy_hop_data` later
TheBlueMatt May 4, 2022
bd1e20d
Store an `events::PaymentPurpose` with each claimable payment
TheBlueMatt May 4, 2022
28c70ac
Ensure all HTLCs for a claimed payment are claimed on startup
TheBlueMatt Apr 18, 2022
0a2a40c
Add a `PaymentClaimed` event to indicate a payment was claimed
TheBlueMatt Apr 18, 2022
0e25421
Provide a redundant `Event::PaymentClaimed` on restart if needed
TheBlueMatt May 4, 2022
11c2f12
Do additional pre-flight checks before claiming a payment
TheBlueMatt Apr 19, 2022
a12d37e
Drop return value from `fail_htlc_backwards`, clarify docs
TheBlueMatt Apr 19, 2022
8a56702
Add internal docs for ChannelMonitor::payment_preimages
TheBlueMatt Apr 22, 2022
531d6c8
Change `Event` `amt` fields to `amount_msat` for clarity
TheBlueMatt May 28, 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
5 changes: 3 additions & 2 deletions fuzz/src/chanmon_consistency.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -840,13 +840,14 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
events::Event::PaymentReceived { payment_hash, .. } => {
if claim_set.insert(payment_hash.0) {
if $fail {
assert!(nodes[$node].fail_htlc_backwards(&payment_hash));
nodes[$node].fail_htlc_backwards(&payment_hash);
} else {
assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0)));
nodes[$node].claim_funds(PaymentPreimage(payment_hash.0));
}
}
},
events::Event::PaymentSent { .. } => {},
events::Event::PaymentClaimed { .. } => {},
events::Event::PaymentPathSuccessful { .. } => {},
events::Event::PaymentPathFailed { .. } => {},
events::Event::PaymentForwarded { .. } if $node == 1 => {},
Expand Down
11 changes: 7 additions & 4 deletions lightning/src/chain/chainmonitor.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -731,7 +731,7 @@ impl<ChannelSigner: Sign, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> even
mod tests {
use bitcoin::BlockHeader;
use ::{check_added_monitors, check_closed_broadcast, check_closed_event};
use ::{expect_payment_sent, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
use ::{expect_payment_sent, expect_payment_claimed, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
use ::{get_htlc_update_msgs, get_local_commitment_txn, get_revoke_commit_msgs, get_route_and_payment_hash, unwrap_send_err};
use chain::{ChannelMonitorUpdateErr, Confirm, Watch};
use chain::channelmonitor::LATENCY_GRACE_PERIOD_BLOCKS;
Expand DownExpand Up@@ -798,16 +798,18 @@ mod tests {
create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

// Route two payments to be claimed at the same time.
let payment_preimage_1= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
let payment_preimage_2= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

chanmon_cfgs[1].persister.offchain_monitor_updates.lock().unwrap().clear();
chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));

nodes[1].node.claim_funds(payment_preimage_1);
check_added_monitors!(nodes[1], 1);
expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
nodes[1].node.claim_funds(payment_preimage_2);
check_added_monitors!(nodes[1], 1);
expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);

chanmon_cfgs[1].persister.set_update_ret(Ok(()));

Expand DownExpand Up@@ -877,8 +879,9 @@ mod tests {
let (route, second_payment_hash, _, second_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);

// First route a payment that we will claim on chain and give the recipient the preimage.
let payment_preimage= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
nodes[1].node.claim_funds(payment_preimage);
expect_payment_claimed!(nodes[1], payment_hash, 1_000_000);
nodes[1].node.get_and_clear_pending_msg_events();
check_added_monitors!(nodes[1], 1);
let remote_txn = get_local_commitment_txn!(nodes[1], channel.2);
Expand Down
11 changes: 10 additions & 1 deletion lightning/src/chain/channelmonitor.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -655,6 +655,10 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
// deserialization
current_holder_commitment_number: u64,

/// The set of payment hashes from inbound payments for which we know the preimage. Payment
/// preimages that are not included in any unrevoked local commitment transaction or unrevoked
/// remote commitment transactions are automatically removed when commitment transactions are
/// revoked.
payment_preimages: HashMap<PaymentHash, PaymentPreimage>,

// Note that `MonitorEvent`s MUST NOT be generated during update processing, only generated
Expand DownExpand Up@@ -1085,7 +1089,8 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs).map_err(|_| ())
}

#[cfg(test)]
/// This is used to provide payment preimage(s) out-of-band during startup without updating the
/// off-chain state with a new commitment transaction.
pub(crate) fn provide_payment_preimage<B: Deref, F: Deref, L: Deref>(
&self,
payment_hash: &PaymentHash,
Expand DownExpand Up@@ -1631,6 +1636,10 @@ impl<Signer: Sign> ChannelMonitor<Signer> {

res
}

pub(crate) fn get_stored_preimages(&self) -> HashMap<PaymentHash, PaymentPreimage> {
self.inner.lock().unwrap().payment_preimages.clone()
Comment thread
TheBlueMatt marked this conversation as resolved.
}
}

/// Compares a broadcasted commitment transaction's HTLCs with those in the latest state,
Expand Down
83 changes: 50 additions & 33 deletions lightning/src/ln/chanmon_update_fail_tests.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -89,7 +89,7 @@ fn test_monitor_and_persister_update_fail() {
send_payment(&nodes[0], &vec!(&nodes[1])[..], 10_000_000);

// Route an HTLC from node 0 to node 1 (but don't settle)
let preimage= route_payment(&nodes[0], &vec!(&nodes[1])[..], 9_000_000).0;
let (preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 9_000_000);

// Make a copy of the ChainMonitor so we can capture the error it returns on a
// bogus update. Note that if instead we updated the nodes[0]'s ChainMonitor
Expand DownExpand Up@@ -123,8 +123,10 @@ fn test_monitor_and_persister_update_fail() {
persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));

// Try to update ChannelMonitor
assert!(nodes[1].node.claim_funds(preimage));
nodes[1].node.claim_funds(preimage);
expect_payment_claimed!(nodes[1], payment_hash, 9_000_000);
check_added_monitors!(nodes[1], 1);

let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
assert_eq!(updates.update_fulfill_htlcs.len(), 1);
nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
Expand DownExpand Up@@ -189,9 +191,9 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
let events_3 = nodes[1].node.get_and_clear_pending_events();
assert_eq!(events_3.len(), 1);
match events_3[0] {
Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
assert_eq!(payment_hash_1, *payment_hash);
assert_eq!(amt, 1000000);
assert_eq!(amount_msat, 1_000_000);
match &purpose {
PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -267,7 +269,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

// Now try to send a second payment which will fail to send
let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand All@@ -283,8 +285,10 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {

// Claim the previous payment, which will result in a update_fulfill_htlc/CS from nodes[1]
// but nodes[0] won't respond since it is frozen.
assert!(nodes[1].node.claim_funds(payment_preimage_1));
nodes[1].node.claim_funds(payment_preimage_1);
check_added_monitors!(nodes[1], 1);
expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);

let events_2 = nodes[1].node.get_and_clear_pending_msg_events();
assert_eq!(events_2.len(), 1);
let (bs_initial_fulfill, bs_initial_commitment_signed) = match events_2[0] {
Expand DownExpand Up@@ -555,9 +559,9 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
let events_5 = nodes[1].node.get_and_clear_pending_events();
assert_eq!(events_5.len(), 1);
match events_5[0] {
Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
assert_eq!(payment_hash_2, *payment_hash);
assert_eq!(amt, 1000000);
assert_eq!(amount_msat, 1_000_000);
match &purpose {
PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -672,9 +676,9 @@ fn test_monitor_update_fail_cs() {
let events = nodes[1].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match events[0] {
Event::PaymentReceived { payment_hash, ref purpose, amt } => {
Event::PaymentReceived { payment_hash, ref purpose, amount_msat } => {
assert_eq!(payment_hash, our_payment_hash);
assert_eq!(amt, 1000000);
assert_eq!(amount_msat, 1_000_000);
match &purpose {
PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -827,7 +831,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);

// Fail the payment backwards, failing the monitor update on nodes[1]'s receipt of the RAA
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
nodes[2].node.fail_htlc_backwards(&payment_hash_1);
expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);

Expand DownExpand Up@@ -1088,13 +1092,15 @@ fn test_monitor_update_fail_reestablish() {
let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());

let (payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);

nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);

assert!(nodes[2].node.claim_funds(payment_preimage));
nodes[2].node.claim_funds(payment_preimage);
check_added_monitors!(nodes[2], 1);
expect_payment_claimed!(nodes[2], payment_hash, 1_000_000);

let mut updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
assert!(updates.update_add_htlcs.is_empty());
assert!(updates.update_fail_htlcs.is_empty());
Expand DownExpand Up@@ -1292,13 +1298,14 @@ fn claim_while_disconnected_monitor_update_fail() {
let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

// Forward a payment for B to claim
let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);

assert!(nodes[1].node.claim_funds(payment_preimage_1));
nodes[1].node.claim_funds(payment_preimage_1);
check_added_monitors!(nodes[1], 1);
expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);

nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
Expand DownExpand Up@@ -1578,10 +1585,11 @@ fn test_monitor_update_fail_claim() {
// Rebalance a bit so that we can send backwards from 3 to 2.
send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
assert!(nodes[1].node.claim_funds(payment_preimage_1));
nodes[1].node.claim_funds(payment_preimage_1);
expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Temporary failure claiming HTLC, treating as success: Failed to update ChannelMonitor".to_string(), 1);
check_added_monitors!(nodes[1], 1);

Expand DownExpand Up@@ -1642,9 +1650,9 @@ fn test_monitor_update_fail_claim() {
let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 2);
match events[0] {
Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
assert_eq!(payment_hash_2, *payment_hash);
assert_eq!(1_000_000, amt);
assert_eq!(1_000_000, amount_msat);
match &purpose {
PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand All@@ -1656,9 +1664,9 @@ fn test_monitor_update_fail_claim() {
_ => panic!("Unexpected event"),
}
match events[1] {
Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
assert_eq!(payment_hash_3, *payment_hash);
assert_eq!(1_000_000, amt);
assert_eq!(1_000_000, amount_msat);
match &purpose {
PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -1688,7 +1696,7 @@ fn test_monitor_update_on_pending_forwards() {
send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
nodes[2].node.fail_htlc_backwards(&payment_hash_1);
expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);

Expand DownExpand Up@@ -1754,7 +1762,7 @@ fn monitor_update_claim_fail_no_response() {
let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

// Forward a payment for B to claim
let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

// Now start forwarding a second payment, skipping the last RAA so B is in AwaitingRAA
let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand All@@ -1770,8 +1778,10 @@ fn monitor_update_claim_fail_no_response() {
let as_raa = commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false, true, false, true);

chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
assert!(nodes[1].node.claim_funds(payment_preimage_1));
nodes[1].node.claim_funds(payment_preimage_1);
expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
check_added_monitors!(nodes[1], 1);

let events = nodes[1].node.get_and_clear_pending_msg_events();
assert_eq!(events.len(), 0);
nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Temporary failure claiming HTLC, treating as success: Failed to update ChannelMonitor".to_string(), 1);
Expand DownExpand Up@@ -2076,13 +2086,15 @@ fn test_fail_htlc_on_broadcast_after_claim() {
create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
let chan_id_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known()).2;

let payment_preimage= route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000).0;
let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000);

let bs_txn = get_local_commitment_txn!(nodes[2], chan_id_2);
assert_eq!(bs_txn.len(), 1);

nodes[2].node.claim_funds(payment_preimage);
check_added_monitors!(nodes[2], 1);
expect_payment_claimed!(nodes[2], payment_hash, 2000);

let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &cs_updates.update_fulfill_htlcs[0]);
let bs_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
Expand DownExpand Up@@ -2235,7 +2247,7 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
//
// Note that because, at the end, MonitorUpdateFailed is still set, the HTLC generated in (c)
// will not be freed from the holding cell.
let (payment_preimage_0, _, _) = route_payment(&nodes[1], &[&nodes[0]], 100000);
let (payment_preimage_0, payment_hash_0, _) = route_payment(&nodes[1], &[&nodes[0]], 100_000);

nodes[0].node.send_payment(&route, payment_hash_1, &Some(payment_secret_1)).unwrap();
check_added_monitors!(nodes[0], 1);
Expand All@@ -2246,8 +2258,9 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
check_added_monitors!(nodes[0], 0);

chanmon_cfgs[0].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
assert!(nodes[0].node.claim_funds(payment_preimage_0));
nodes[0].node.claim_funds(payment_preimage_0);
check_added_monitors!(nodes[0], 1);
expect_payment_claimed!(nodes[0], payment_hash_0, 100_000);

nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &send.msgs[0]);
nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &send.commitment_msg);
Expand DownExpand Up@@ -2455,13 +2468,15 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
payment_preimage,
};
if second_fails {
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash));
nodes[2].node.fail_htlc_backwards(&payment_hash);
expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);
get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
} else {
assert!(nodes[2].node.claim_funds(payment_preimage));
nodes[2].node.claim_funds(payment_preimage);
check_added_monitors!(nodes[2], 1);
expect_payment_claimed!(nodes[2], payment_hash, 100_000);

let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
// Check that the message we're about to deliver matches the one generated:
Expand DownExpand Up@@ -2630,20 +2645,22 @@ fn double_temp_error() {

let (_, _, channel_id, _) = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
let (payment_preimage_2, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
// `claim_funds` results in a ChannelMonitorUpdate.
assert!(nodes[1].node.claim_funds(payment_preimage_1));
nodes[1].node.claim_funds(payment_preimage_1);
check_added_monitors!(nodes[1], 1);
expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
let (funding_tx, latest_update_1, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();

chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
// Previously, this would've panicked due to a double-call to `Channel::monitor_update_failed`,
// which had some asserts that prevented it from being called twice.
assert!(nodes[1].node.claim_funds(payment_preimage_2));
nodes[1].node.claim_funds(payment_preimage_2);
check_added_monitors!(nodes[1], 1);
expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);
chanmon_cfgs[1].persister.set_update_ret(Ok(()));

let (_, latest_update_2, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Improve Robustness of Inbound MPP Claims Across Restart#1434

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

valentinewallace
merged 9 commits into

from

May 30, 2022
 Merged


Improve Robustness of Inbound MPP Claims Across Restart
#1434
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


9 commits


Select commit
Hold shift + click to select a range
2a42595

Enable removal of `OnionPayload::Invoice::_legacy_hop_data` later


TheBlueMatt May 4, 2022
bd1e20d

Store an `events::PaymentPurpose` with each claimable payment


TheBlueMatt May 4, 2022
28c70ac

Ensure all HTLCs for a claimed payment are claimed on startup


TheBlueMatt Apr 18, 2022
0a2a40c

Add a `PaymentClaimed` event to indicate a payment was claimed


TheBlueMatt Apr 18, 2022
0e25421

Provide a redundant `Event::PaymentClaimed` on restart if needed


TheBlueMatt May 4, 2022
11c2f12

Do additional pre-flight checks before claiming a payment


TheBlueMatt Apr 19, 2022
a12d37e

Drop return value from `fail_htlc_backwards`, clarify docs


TheBlueMatt Apr 19, 2022
8a56702

Add internal docs for ChannelMonitor::payment_preimages


TheBlueMatt Apr 22, 2022
531d6c8

Change `Event` `amt` fields to `amount_msat` for clarity


TheBlueMatt May 28, 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


5 changes: 3 additions & 2 deletions
fuzz/src/chanmon_consistency.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -840,13 +840,14 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 events::Event::PaymentReceived { payment_hash, .. } => {
 if claim_set.insert(payment_hash.0) {
 if $fail {
assert!(nodes[$node].fail_htlc_backwards(&payment_hash));
 nodes[$node].fail_htlc_backwards(&payment_hash);
 } else {
assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0)));
 nodes[$node].claim_funds(PaymentPreimage(payment_hash.0));
 }
 }
 },
 events::Event::PaymentSent { .. } => {},
 events::Event::PaymentClaimed { .. } => {},
 events::Event::PaymentPathSuccessful { .. } => {},
 events::Event::PaymentPathFailed { .. } => {},
 events::Event::PaymentForwarded { .. } if $node == 1 => {},
Expand Down

11 changes: 7 additions & 4 deletions
lightning/src/chain/chainmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -731,7 +731,7 @@ impl<ChannelSigner: Sign, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> even
mod tests {
	use bitcoin::BlockHeader;
	use ::{check_added_monitors, check_closed_broadcast, check_closed_event};
	use ::{expect_payment_sent, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
	use ::{expect_payment_sent, expect_payment_claimed, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
	use ::{get_htlc_update_msgs, get_local_commitment_txn, get_revoke_commit_msgs, get_route_and_payment_hash, unwrap_send_err};
	use chain::{ChannelMonitorUpdateErr, Confirm, Watch};
	use chain::channelmonitor::LATENCY_GRACE_PERIOD_BLOCKS;
Expand DownExpand Up@@ -798,16 +798,18 @@ mod tests {
 create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

 // Route two payments to be claimed at the same time.
 let payment_preimage_1= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let payment_preimage_2= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
 let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

 chanmon_cfgs[1].persister.offchain_monitor_updates.lock().unwrap().clear();
 chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));

 nodes[1].node.claim_funds(payment_preimage_1);
 check_added_monitors!(nodes[1], 1);
 expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
 nodes[1].node.claim_funds(payment_preimage_2);
 check_added_monitors!(nodes[1], 1);
 expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);

 chanmon_cfgs[1].persister.set_update_ret(Ok(()));

Expand DownExpand Up@@ -877,8 +879,9 @@ mod tests {
 let (route, second_payment_hash, _, second_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);

 // First route a payment that we will claim on chain and give the recipient the preimage.
 let payment_preimage= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
 nodes[1].node.claim_funds(payment_preimage);
 expect_payment_claimed!(nodes[1], payment_hash, 1_000_000);
 nodes[1].node.get_and_clear_pending_msg_events();
 check_added_monitors!(nodes[1], 1);
 let remote_txn = get_local_commitment_txn!(nodes[1], channel.2);
Expand Down

11 changes: 10 additions & 1 deletion
lightning/src/chain/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -655,6 +655,10 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
	// deserialization
	current_holder_commitment_number: u64,

	/// The set of payment hashes from inbound payments for which we know the preimage. Payment
	/// preimages that are not included in any unrevoked local commitment transaction or unrevoked
	/// remote commitment transactions are automatically removed when commitment transactions are
	/// revoked.
	payment_preimages: HashMap<PaymentHash, PaymentPreimage>,

	// Note that `MonitorEvent`s MUST NOT be generated during update processing, only generated
Expand DownExpand Up@@ -1085,7 +1089,8 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs).map_err(|_| ())
	}

	#[cfg(test)]
	/// This is used to provide payment preimage(s) out-of-band during startup without updating the
	/// off-chain state with a new commitment transaction.
	pub(crate) fn provide_payment_preimage<B: Deref, F: Deref, L: Deref>(
 &self,
 payment_hash: &PaymentHash,
Expand DownExpand Up@@ -1631,6 +1636,10 @@ impl<Signer: Sign> ChannelMonitor<Signer> {

 res
	}

	pub(crate) fn get_stored_preimages(&self) -> HashMap<PaymentHash, PaymentPreimage> {
 self.inner.lock().unwrap().payment_preimages.clone()
Comment thread
TheBlueMatt marked this conversation as resolved.

	}
}

/// Compares a broadcasted commitment transaction's HTLCs with those in the latest state,
Expand Down

83 changes: 50 additions & 33 deletions
lightning/src/ln/chanmon_update_fail_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -89,7 +89,7 @@ fn test_monitor_and_persister_update_fail() {
	send_payment(&nodes[0], &vec!(&nodes[1])[..], 10_000_000);

	// Route an HTLC from node 0 to node 1 (but don't settle)
	let preimage= route_payment(&nodes[0], &vec!(&nodes[1])[..], 9_000_000).0;
	let (preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 9_000_000);

	// Make a copy of the ChainMonitor so we can capture the error it returns on a
	// bogus update. Note that if instead we updated the nodes[0]'s ChainMonitor
Expand DownExpand Up@@ -123,8 +123,10 @@ fn test_monitor_and_persister_update_fail() {
	persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));

	// Try to update ChannelMonitor
	assert!(nodes[1].node.claim_funds(preimage));
	nodes[1].node.claim_funds(preimage);
	expect_payment_claimed!(nodes[1], payment_hash, 9_000_000);
	check_added_monitors!(nodes[1], 1);

	let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
	assert_eq!(updates.update_fulfill_htlcs.len(), 1);
	nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
Expand DownExpand Up@@ -189,9 +191,9 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
	let events_3 = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events_3.len(), 1);
	match events_3[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_1, *payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -267,7 +269,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	// Now try to send a second payment which will fail to send
	let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand All@@ -283,8 +285,10 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {

	// Claim the previous payment, which will result in a update_fulfill_htlc/CS from nodes[1]
	// but nodes[0] won't respond since it is frozen.
assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);

	let events_2 = nodes[1].node.get_and_clear_pending_msg_events();
	assert_eq!(events_2.len(), 1);
	let (bs_initial_fulfill, bs_initial_commitment_signed) = match events_2[0] {
Expand DownExpand Up@@ -555,9 +559,9 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
	let events_5 = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events_5.len(), 1);
	match events_5[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_2, *payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -672,9 +676,9 @@ fn test_monitor_update_fail_cs() {
	let events = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events.len(), 1);
	match events[0] {
 Event::PaymentReceived { payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash, our_payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -827,7 +831,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
	let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);

	// Fail the payment backwards, failing the monitor update on nodes[1]'s receipt of the RAA
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
	nodes[2].node.fail_htlc_backwards(&payment_hash_1);
	expect_pending_htlcs_forwardable!(nodes[2]);
	check_added_monitors!(nodes[2], 1);

Expand DownExpand Up@@ -1088,13 +1092,15 @@ fn test_monitor_update_fail_reestablish() {
	let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
	create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());

	let (payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
	let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);

	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);

assert!(nodes[2].node.claim_funds(payment_preimage));
	nodes[2].node.claim_funds(payment_preimage);
	check_added_monitors!(nodes[2], 1);
	expect_payment_claimed!(nodes[2], payment_hash, 1_000_000);

	let mut updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	assert!(updates.update_add_htlcs.is_empty());
	assert!(updates.update_fail_htlcs.is_empty());
Expand DownExpand Up@@ -1292,13 +1298,14 @@ fn claim_while_disconnected_monitor_update_fail() {
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	// Forward a payment for B to claim
	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);

assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);

	nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
	nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
Expand DownExpand Up@@ -1578,10 +1585,11 @@ fn test_monitor_update_fail_claim() {
	// Rebalance a bit so that we can send backwards from 3 to 2.
	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Temporary failure claiming HTLC, treating as success: Failed to update ChannelMonitor".to_string(), 1);
	check_added_monitors!(nodes[1], 1);

Expand DownExpand Up@@ -1642,9 +1650,9 @@ fn test_monitor_update_fail_claim() {
	let events = nodes[0].node.get_and_clear_pending_events();
	assert_eq!(events.len(), 2);
	match events[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_2, *payment_hash);
 assert_eq!(1_000_000, amt);
 assert_eq!(1_000_000, amount_msat);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand All@@ -1656,9 +1664,9 @@ fn test_monitor_update_fail_claim() {
 _ => panic!("Unexpected event"),
	}
	match events[1] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_3, *payment_hash);
 assert_eq!(1_000_000, amt);
 assert_eq!(1_000_000, amount_msat);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -1688,7 +1696,7 @@ fn test_monitor_update_on_pending_forwards() {
	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

	let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
	nodes[2].node.fail_htlc_backwards(&payment_hash_1);
	expect_pending_htlcs_forwardable!(nodes[2]);
	check_added_monitors!(nodes[2], 1);

Expand DownExpand Up@@ -1754,7 +1762,7 @@ fn monitor_update_claim_fail_no_response() {
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	// Forward a payment for B to claim
	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	// Now start forwarding a second payment, skipping the last RAA so B is in AwaitingRAA
	let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand All@@ -1770,8 +1778,10 @@ fn monitor_update_claim_fail_no_response() {
	let as_raa = commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false, true, false, true);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	check_added_monitors!(nodes[1], 1);

	let events = nodes[1].node.get_and_clear_pending_msg_events();
	assert_eq!(events.len(), 0);
	nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Temporary failure claiming HTLC, treating as success: Failed to update ChannelMonitor".to_string(), 1);
Expand DownExpand Up@@ -2076,13 +2086,15 @@ fn test_fail_htlc_on_broadcast_after_claim() {
	create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
	let chan_id_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known()).2;

	let payment_preimage= route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000).0;
	let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000);

	let bs_txn = get_local_commitment_txn!(nodes[2], chan_id_2);
	assert_eq!(bs_txn.len(), 1);

	nodes[2].node.claim_funds(payment_preimage);
	check_added_monitors!(nodes[2], 1);
	expect_payment_claimed!(nodes[2], payment_hash, 2000);

	let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &cs_updates.update_fulfill_htlcs[0]);
	let bs_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
Expand DownExpand Up@@ -2235,7 +2247,7 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
	//
	// Note that because, at the end, MonitorUpdateFailed is still set, the HTLC generated in (c)
	// will not be freed from the holding cell.
	let (payment_preimage_0, _, _) = route_payment(&nodes[1], &[&nodes[0]], 100000);
	let (payment_preimage_0, payment_hash_0, _) = route_payment(&nodes[1], &[&nodes[0]], 100_000);

	nodes[0].node.send_payment(&route, payment_hash_1, &Some(payment_secret_1)).unwrap();
	check_added_monitors!(nodes[0], 1);
Expand All@@ -2246,8 +2258,9 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
	check_added_monitors!(nodes[0], 0);

	chanmon_cfgs[0].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
assert!(nodes[0].node.claim_funds(payment_preimage_0));
	nodes[0].node.claim_funds(payment_preimage_0);
	check_added_monitors!(nodes[0], 1);
	expect_payment_claimed!(nodes[0], payment_hash_0, 100_000);

	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &send.msgs[0]);
	nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &send.commitment_msg);
Expand DownExpand Up@@ -2455,13 +2468,15 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
 payment_preimage,
	};
	if second_fails {
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash));
 nodes[2].node.fail_htlc_backwards(&payment_hash);
 expect_pending_htlcs_forwardable!(nodes[2]);
 check_added_monitors!(nodes[2], 1);
 get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	} else {
assert!(nodes[2].node.claim_funds(payment_preimage));
 nodes[2].node.claim_funds(payment_preimage);
 check_added_monitors!(nodes[2], 1);
 expect_payment_claimed!(nodes[2], payment_hash, 100_000);

 let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
 assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
 // Check that the message we're about to deliver matches the one generated:
Expand DownExpand Up@@ -2630,20 +2645,22 @@ fn double_temp_error() {

	let (_, _, channel_id, _) = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_2, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
	let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	// `claim_funds` results in a ChannelMonitorUpdate.
assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	let (funding_tx, latest_update_1, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	// Previously, this would've panicked due to a double-call to `Channel::monitor_update_failed`,
	// which had some asserts that prevented it from being called twice.
assert!(nodes[1].node.claim_funds(payment_preimage_2));
	nodes[1].node.claim_funds(payment_preimage_2);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);
	chanmon_cfgs[1].persister.set_update_ret(Ok(()));

	let (_, latest_update_2, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Improve Robustness of Inbound MPP Claims Across Restart#1434

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

valentinewallace
merged 9 commits into

from

May 30, 2022
 Merged


Improve Robustness of Inbound MPP Claims Across Restart
#1434
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


9 commits


Select commit
Hold shift + click to select a range
2a42595

Enable removal of `OnionPayload::Invoice::_legacy_hop_data` later


TheBlueMatt May 4, 2022
bd1e20d

Store an `events::PaymentPurpose` with each claimable payment


TheBlueMatt May 4, 2022
28c70ac

Ensure all HTLCs for a claimed payment are claimed on startup


TheBlueMatt Apr 18, 2022
0a2a40c

Add a `PaymentClaimed` event to indicate a payment was claimed


TheBlueMatt Apr 18, 2022
0e25421

Provide a redundant `Event::PaymentClaimed` on restart if needed


TheBlueMatt May 4, 2022
11c2f12

Do additional pre-flight checks before claiming a payment


TheBlueMatt Apr 19, 2022
a12d37e

Drop return value from `fail_htlc_backwards`, clarify docs


TheBlueMatt Apr 19, 2022
8a56702

Add internal docs for ChannelMonitor::payment_preimages


TheBlueMatt Apr 22, 2022
531d6c8

Change `Event` `amt` fields to `amount_msat` for clarity


TheBlueMatt May 28, 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


5 changes: 3 additions & 2 deletions
fuzz/src/chanmon_consistency.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -840,13 +840,14 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 events::Event::PaymentReceived { payment_hash, .. } => {
 if claim_set.insert(payment_hash.0) {
 if $fail {
assert!(nodes[$node].fail_htlc_backwards(&payment_hash));
 nodes[$node].fail_htlc_backwards(&payment_hash);
 } else {
assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0)));
 nodes[$node].claim_funds(PaymentPreimage(payment_hash.0));
 }
 }
 },
 events::Event::PaymentSent { .. } => {},
 events::Event::PaymentClaimed { .. } => {},
 events::Event::PaymentPathSuccessful { .. } => {},
 events::Event::PaymentPathFailed { .. } => {},
 events::Event::PaymentForwarded { .. } if $node == 1 => {},
Expand Down

11 changes: 7 additions & 4 deletions
lightning/src/chain/chainmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -731,7 +731,7 @@ impl<ChannelSigner: Sign, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> even
mod tests {
	use bitcoin::BlockHeader;
	use ::{check_added_monitors, check_closed_broadcast, check_closed_event};
	use ::{expect_payment_sent, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
	use ::{expect_payment_sent, expect_payment_claimed, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
	use ::{get_htlc_update_msgs, get_local_commitment_txn, get_revoke_commit_msgs, get_route_and_payment_hash, unwrap_send_err};
	use chain::{ChannelMonitorUpdateErr, Confirm, Watch};
	use chain::channelmonitor::LATENCY_GRACE_PERIOD_BLOCKS;
Expand DownExpand Up@@ -798,16 +798,18 @@ mod tests {
 create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

 // Route two payments to be claimed at the same time.
 let payment_preimage_1= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let payment_preimage_2= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
 let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

 chanmon_cfgs[1].persister.offchain_monitor_updates.lock().unwrap().clear();
 chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));

 nodes[1].node.claim_funds(payment_preimage_1);
 check_added_monitors!(nodes[1], 1);
 expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
 nodes[1].node.claim_funds(payment_preimage_2);
 check_added_monitors!(nodes[1], 1);
 expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);

 chanmon_cfgs[1].persister.set_update_ret(Ok(()));

Expand DownExpand Up@@ -877,8 +879,9 @@ mod tests {
 let (route, second_payment_hash, _, second_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);

 // First route a payment that we will claim on chain and give the recipient the preimage.
 let payment_preimage= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
 nodes[1].node.claim_funds(payment_preimage);
 expect_payment_claimed!(nodes[1], payment_hash, 1_000_000);
 nodes[1].node.get_and_clear_pending_msg_events();
 check_added_monitors!(nodes[1], 1);
 let remote_txn = get_local_commitment_txn!(nodes[1], channel.2);
Expand Down

11 changes: 10 additions & 1 deletion
lightning/src/chain/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -655,6 +655,10 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
	// deserialization
	current_holder_commitment_number: u64,

	/// The set of payment hashes from inbound payments for which we know the preimage. Payment
	/// preimages that are not included in any unrevoked local commitment transaction or unrevoked
	/// remote commitment transactions are automatically removed when commitment transactions are
	/// revoked.
	payment_preimages: HashMap<PaymentHash, PaymentPreimage>,

	// Note that `MonitorEvent`s MUST NOT be generated during update processing, only generated
Expand DownExpand Up@@ -1085,7 +1089,8 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs).map_err(|_| ())
	}

	#[cfg(test)]
	/// This is used to provide payment preimage(s) out-of-band during startup without updating the
	/// off-chain state with a new commitment transaction.
	pub(crate) fn provide_payment_preimage<B: Deref, F: Deref, L: Deref>(
 &self,
 payment_hash: &PaymentHash,
Expand DownExpand Up@@ -1631,6 +1636,10 @@ impl<Signer: Sign> ChannelMonitor<Signer> {

 res
	}

	pub(crate) fn get_stored_preimages(&self) -> HashMap<PaymentHash, PaymentPreimage> {
 self.inner.lock().unwrap().payment_preimages.clone()
Comment thread
TheBlueMatt marked this conversation as resolved.

	}
}

/// Compares a broadcasted commitment transaction's HTLCs with those in the latest state,
Expand Down

83 changes: 50 additions & 33 deletions
lightning/src/ln/chanmon_update_fail_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -89,7 +89,7 @@ fn test_monitor_and_persister_update_fail() {
	send_payment(&nodes[0], &vec!(&nodes[1])[..], 10_000_000);

	// Route an HTLC from node 0 to node 1 (but don't settle)
	let preimage= route_payment(&nodes[0], &vec!(&nodes[1])[..], 9_000_000).0;
	let (preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 9_000_000);

	// Make a copy of the ChainMonitor so we can capture the error it returns on a
	// bogus update. Note that if instead we updated the nodes[0]'s ChainMonitor
Expand DownExpand Up@@ -123,8 +123,10 @@ fn test_monitor_and_persister_update_fail() {
	persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));

	// Try to update ChannelMonitor
	assert!(nodes[1].node.claim_funds(preimage));
	nodes[1].node.claim_funds(preimage);
	expect_payment_claimed!(nodes[1], payment_hash, 9_000_000);
	check_added_monitors!(nodes[1], 1);

	let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
	assert_eq!(updates.update_fulfill_htlcs.len(), 1);
	nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
Expand DownExpand Up@@ -189,9 +191,9 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
	let events_3 = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events_3.len(), 1);
	match events_3[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_1, *payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -267,7 +269,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	// Now try to send a second payment which will fail to send
	let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand All@@ -283,8 +285,10 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {

	// Claim the previous payment, which will result in a update_fulfill_htlc/CS from nodes[1]
	// but nodes[0] won't respond since it is frozen.
assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);

	let events_2 = nodes[1].node.get_and_clear_pending_msg_events();
	assert_eq!(events_2.len(), 1);
	let (bs_initial_fulfill, bs_initial_commitment_signed) = match events_2[0] {
Expand DownExpand Up@@ -555,9 +559,9 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
	let events_5 = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events_5.len(), 1);
	match events_5[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_2, *payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -672,9 +676,9 @@ fn test_monitor_update_fail_cs() {
	let events = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events.len(), 1);
	match events[0] {
 Event::PaymentReceived { payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash, our_payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -827,7 +831,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
	let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);

	// Fail the payment backwards, failing the monitor update on nodes[1]'s receipt of the RAA
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
	nodes[2].node.fail_htlc_backwards(&payment_hash_1);
	expect_pending_htlcs_forwardable!(nodes[2]);
	check_added_monitors!(nodes[2], 1);

Expand DownExpand Up@@ -1088,13 +1092,15 @@ fn test_monitor_update_fail_reestablish() {
	let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
	create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());

	let (payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
	let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);

	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);

assert!(nodes[2].node.claim_funds(payment_preimage));
	nodes[2].node.claim_funds(payment_preimage);
	check_added_monitors!(nodes[2], 1);
	expect_payment_claimed!(nodes[2], payment_hash, 1_000_000);

	let mut updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	assert!(updates.update_add_htlcs.is_empty());
	assert!(updates.update_fail_htlcs.is_empty());
Expand DownExpand Up@@ -1292,13 +1298,14 @@ fn claim_while_disconnected_monitor_update_fail() {
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	// Forward a payment for B to claim
	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);

assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);

	nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
	nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
Expand DownExpand Up@@ -1578,10 +1585,11 @@ fn test_monitor_update_fail_claim() {
	// Rebalance a bit so that we can send backwards from 3 to 2.
	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Temporary failure claiming HTLC, treating as success: Failed to update ChannelMonitor".to_string(), 1);
	check_added_monitors!(nodes[1], 1);

Expand DownExpand Up@@ -1642,9 +1650,9 @@ fn test_monitor_update_fail_claim() {
	let events = nodes[0].node.get_and_clear_pending_events();
	assert_eq!(events.len(), 2);
	match events[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_2, *payment_hash);
 assert_eq!(1_000_000, amt);
 assert_eq!(1_000_000, amount_msat);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand All@@ -1656,9 +1664,9 @@ fn test_monitor_update_fail_claim() {
 _ => panic!("Unexpected event"),
	}
	match events[1] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_3, *payment_hash);
 assert_eq!(1_000_000, amt);
 assert_eq!(1_000_000, amount_msat);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -1688,7 +1696,7 @@ fn test_monitor_update_on_pending_forwards() {
	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

	let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
	nodes[2].node.fail_htlc_backwards(&payment_hash_1);
	expect_pending_htlcs_forwardable!(nodes[2]);
	check_added_monitors!(nodes[2], 1);

Expand DownExpand Up@@ -1754,7 +1762,7 @@ fn monitor_update_claim_fail_no_response() {
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	// Forward a payment for B to claim
	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	// Now start forwarding a second payment, skipping the last RAA so B is in AwaitingRAA
	let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand All@@ -1770,8 +1778,10 @@ fn monitor_update_claim_fail_no_response() {
	let as_raa = commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false, true, false, true);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	check_added_monitors!(nodes[1], 1);

	let events = nodes[1].node.get_and_clear_pending_msg_events();
	assert_eq!(events.len(), 0);
	nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Temporary failure claiming HTLC, treating as success: Failed to update ChannelMonitor".to_string(), 1);
Expand DownExpand Up@@ -2076,13 +2086,15 @@ fn test_fail_htlc_on_broadcast_after_claim() {
	create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
	let chan_id_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known()).2;

	let payment_preimage= route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000).0;
	let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000);

	let bs_txn = get_local_commitment_txn!(nodes[2], chan_id_2);
	assert_eq!(bs_txn.len(), 1);

	nodes[2].node.claim_funds(payment_preimage);
	check_added_monitors!(nodes[2], 1);
	expect_payment_claimed!(nodes[2], payment_hash, 2000);

	let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &cs_updates.update_fulfill_htlcs[0]);
	let bs_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
Expand DownExpand Up@@ -2235,7 +2247,7 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
	//
	// Note that because, at the end, MonitorUpdateFailed is still set, the HTLC generated in (c)
	// will not be freed from the holding cell.
	let (payment_preimage_0, _, _) = route_payment(&nodes[1], &[&nodes[0]], 100000);
	let (payment_preimage_0, payment_hash_0, _) = route_payment(&nodes[1], &[&nodes[0]], 100_000);

	nodes[0].node.send_payment(&route, payment_hash_1, &Some(payment_secret_1)).unwrap();
	check_added_monitors!(nodes[0], 1);
Expand All@@ -2246,8 +2258,9 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
	check_added_monitors!(nodes[0], 0);

	chanmon_cfgs[0].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
assert!(nodes[0].node.claim_funds(payment_preimage_0));
	nodes[0].node.claim_funds(payment_preimage_0);
	check_added_monitors!(nodes[0], 1);
	expect_payment_claimed!(nodes[0], payment_hash_0, 100_000);

	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &send.msgs[0]);
	nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &send.commitment_msg);
Expand DownExpand Up@@ -2455,13 +2468,15 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
 payment_preimage,
	};
	if second_fails {
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash));
 nodes[2].node.fail_htlc_backwards(&payment_hash);
 expect_pending_htlcs_forwardable!(nodes[2]);
 check_added_monitors!(nodes[2], 1);
 get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	} else {
assert!(nodes[2].node.claim_funds(payment_preimage));
 nodes[2].node.claim_funds(payment_preimage);
 check_added_monitors!(nodes[2], 1);
 expect_payment_claimed!(nodes[2], payment_hash, 100_000);

 let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
 assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
 // Check that the message we're about to deliver matches the one generated:
Expand DownExpand Up@@ -2630,20 +2645,22 @@ fn double_temp_error() {

	let (_, _, channel_id, _) = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_2, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
	let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	// `claim_funds` results in a ChannelMonitorUpdate.
assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	let (funding_tx, latest_update_1, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	// Previously, this would've panicked due to a double-call to `Channel::monitor_update_failed`,
	// which had some asserts that prevented it from being called twice.
assert!(nodes[1].node.claim_funds(payment_preimage_2));
	nodes[1].node.claim_funds(payment_preimage_2);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);
	chanmon_cfgs[1].persister.set_update_ret(Ok(()));

	let (_, latest_update_2, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Improve Robustness of Inbound MPP Claims Across Restart#1434

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

valentinewallace
merged 9 commits into

from

May 30, 2022
 Merged


Improve Robustness of Inbound MPP Claims Across Restart
#1434
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


9 commits


Select commit
Hold shift + click to select a range
2a42595

Enable removal of `OnionPayload::Invoice::_legacy_hop_data` later


TheBlueMatt May 4, 2022
bd1e20d

Store an `events::PaymentPurpose` with each claimable payment


TheBlueMatt May 4, 2022
28c70ac

Ensure all HTLCs for a claimed payment are claimed on startup


TheBlueMatt Apr 18, 2022
0a2a40c

Add a `PaymentClaimed` event to indicate a payment was claimed


TheBlueMatt Apr 18, 2022
0e25421

Provide a redundant `Event::PaymentClaimed` on restart if needed


TheBlueMatt May 4, 2022
11c2f12

Do additional pre-flight checks before claiming a payment


TheBlueMatt Apr 19, 2022
a12d37e

Drop return value from `fail_htlc_backwards`, clarify docs


TheBlueMatt Apr 19, 2022
8a56702

Add internal docs for ChannelMonitor::payment_preimages


TheBlueMatt Apr 22, 2022
531d6c8

Change `Event` `amt` fields to `amount_msat` for clarity


TheBlueMatt May 28, 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


5 changes: 3 additions & 2 deletions
fuzz/src/chanmon_consistency.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -840,13 +840,14 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 events::Event::PaymentReceived { payment_hash, .. } => {
 if claim_set.insert(payment_hash.0) {
 if $fail {
assert!(nodes[$node].fail_htlc_backwards(&payment_hash));
 nodes[$node].fail_htlc_backwards(&payment_hash);
 } else {
assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0)));
 nodes[$node].claim_funds(PaymentPreimage(payment_hash.0));
 }
 }
 },
 events::Event::PaymentSent { .. } => {},
 events::Event::PaymentClaimed { .. } => {},
 events::Event::PaymentPathSuccessful { .. } => {},
 events::Event::PaymentPathFailed { .. } => {},
 events::Event::PaymentForwarded { .. } if $node == 1 => {},
Expand Down

11 changes: 7 additions & 4 deletions
lightning/src/chain/chainmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -731,7 +731,7 @@ impl<ChannelSigner: Sign, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> even
mod tests {
	use bitcoin::BlockHeader;
	use ::{check_added_monitors, check_closed_broadcast, check_closed_event};
	use ::{expect_payment_sent, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
	use ::{expect_payment_sent, expect_payment_claimed, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
	use ::{get_htlc_update_msgs, get_local_commitment_txn, get_revoke_commit_msgs, get_route_and_payment_hash, unwrap_send_err};
	use chain::{ChannelMonitorUpdateErr, Confirm, Watch};
	use chain::channelmonitor::LATENCY_GRACE_PERIOD_BLOCKS;
Expand DownExpand Up@@ -798,16 +798,18 @@ mod tests {
 create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

 // Route two payments to be claimed at the same time.
 let payment_preimage_1= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let payment_preimage_2= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
 let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

 chanmon_cfgs[1].persister.offchain_monitor_updates.lock().unwrap().clear();
 chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));

 nodes[1].node.claim_funds(payment_preimage_1);
 check_added_monitors!(nodes[1], 1);
 expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
 nodes[1].node.claim_funds(payment_preimage_2);
 check_added_monitors!(nodes[1], 1);
 expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);

 chanmon_cfgs[1].persister.set_update_ret(Ok(()));

Expand DownExpand Up@@ -877,8 +879,9 @@ mod tests {
 let (route, second_payment_hash, _, second_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);

 // First route a payment that we will claim on chain and give the recipient the preimage.
 let payment_preimage= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
 nodes[1].node.claim_funds(payment_preimage);
 expect_payment_claimed!(nodes[1], payment_hash, 1_000_000);
 nodes[1].node.get_and_clear_pending_msg_events();
 check_added_monitors!(nodes[1], 1);
 let remote_txn = get_local_commitment_txn!(nodes[1], channel.2);
Expand Down

11 changes: 10 additions & 1 deletion
lightning/src/chain/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -655,6 +655,10 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
	// deserialization
	current_holder_commitment_number: u64,

	/// The set of payment hashes from inbound payments for which we know the preimage. Payment
	/// preimages that are not included in any unrevoked local commitment transaction or unrevoked
	/// remote commitment transactions are automatically removed when commitment transactions are
	/// revoked.
	payment_preimages: HashMap<PaymentHash, PaymentPreimage>,

	// Note that `MonitorEvent`s MUST NOT be generated during update processing, only generated
Expand DownExpand Up@@ -1085,7 +1089,8 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs).map_err(|_| ())
	}

	#[cfg(test)]
	/// This is used to provide payment preimage(s) out-of-band during startup without updating the
	/// off-chain state with a new commitment transaction.
	pub(crate) fn provide_payment_preimage<B: Deref, F: Deref, L: Deref>(
 &self,
 payment_hash: &PaymentHash,
Expand DownExpand Up@@ -1631,6 +1636,10 @@ impl<Signer: Sign> ChannelMonitor<Signer> {

 res
	}

	pub(crate) fn get_stored_preimages(&self) -> HashMap<PaymentHash, PaymentPreimage> {
 self.inner.lock().unwrap().payment_preimages.clone()
Comment thread
TheBlueMatt marked this conversation as resolved.

	}
}

/// Compares a broadcasted commitment transaction's HTLCs with those in the latest state,
Expand Down

83 changes: 50 additions & 33 deletions
lightning/src/ln/chanmon_update_fail_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -89,7 +89,7 @@ fn test_monitor_and_persister_update_fail() {
	send_payment(&nodes[0], &vec!(&nodes[1])[..], 10_000_000);

	// Route an HTLC from node 0 to node 1 (but don't settle)
	let preimage= route_payment(&nodes[0], &vec!(&nodes[1])[..], 9_000_000).0;
	let (preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 9_000_000);

	// Make a copy of the ChainMonitor so we can capture the error it returns on a
	// bogus update. Note that if instead we updated the nodes[0]'s ChainMonitor
Expand DownExpand Up@@ -123,8 +123,10 @@ fn test_monitor_and_persister_update_fail() {
	persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));

	// Try to update ChannelMonitor
	assert!(nodes[1].node.claim_funds(preimage));
	nodes[1].node.claim_funds(preimage);
	expect_payment_claimed!(nodes[1], payment_hash, 9_000_000);
	check_added_monitors!(nodes[1], 1);

	let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
	assert_eq!(updates.update_fulfill_htlcs.len(), 1);
	nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
Expand DownExpand Up@@ -189,9 +191,9 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
	let events_3 = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events_3.len(), 1);
	match events_3[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_1, *payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -267,7 +269,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	// Now try to send a second payment which will fail to send
	let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand All@@ -283,8 +285,10 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {

	// Claim the previous payment, which will result in a update_fulfill_htlc/CS from nodes[1]
	// but nodes[0] won't respond since it is frozen.
assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);

	let events_2 = nodes[1].node.get_and_clear_pending_msg_events();
	assert_eq!(events_2.len(), 1);
	let (bs_initial_fulfill, bs_initial_commitment_signed) = match events_2[0] {
Expand DownExpand Up@@ -555,9 +559,9 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
	let events_5 = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events_5.len(), 1);
	match events_5[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_2, *payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -672,9 +676,9 @@ fn test_monitor_update_fail_cs() {
	let events = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events.len(), 1);
	match events[0] {
 Event::PaymentReceived { payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash, our_payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -827,7 +831,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
	let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);

	// Fail the payment backwards, failing the monitor update on nodes[1]'s receipt of the RAA
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
	nodes[2].node.fail_htlc_backwards(&payment_hash_1);
	expect_pending_htlcs_forwardable!(nodes[2]);
	check_added_monitors!(nodes[2], 1);

Expand DownExpand Up@@ -1088,13 +1092,15 @@ fn test_monitor_update_fail_reestablish() {
	let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
	create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());

	let (payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
	let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);

	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);

assert!(nodes[2].node.claim_funds(payment_preimage));
	nodes[2].node.claim_funds(payment_preimage);
	check_added_monitors!(nodes[2], 1);
	expect_payment_claimed!(nodes[2], payment_hash, 1_000_000);

	let mut updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	assert!(updates.update_add_htlcs.is_empty());
	assert!(updates.update_fail_htlcs.is_empty());
Expand DownExpand Up@@ -1292,13 +1298,14 @@ fn claim_while_disconnected_monitor_update_fail() {
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	// Forward a payment for B to claim
	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);

assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);

	nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
	nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
Expand DownExpand Up@@ -1578,10 +1585,11 @@ fn test_monitor_update_fail_claim() {
	// Rebalance a bit so that we can send backwards from 3 to 2.
	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Temporary failure claiming HTLC, treating as success: Failed to update ChannelMonitor".to_string(), 1);
	check_added_monitors!(nodes[1], 1);

Expand DownExpand Up@@ -1642,9 +1650,9 @@ fn test_monitor_update_fail_claim() {
	let events = nodes[0].node.get_and_clear_pending_events();
	assert_eq!(events.len(), 2);
	match events[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_2, *payment_hash);
 assert_eq!(1_000_000, amt);
 assert_eq!(1_000_000, amount_msat);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand All@@ -1656,9 +1664,9 @@ fn test_monitor_update_fail_claim() {
 _ => panic!("Unexpected event"),
	}
	match events[1] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_3, *payment_hash);
 assert_eq!(1_000_000, amt);
 assert_eq!(1_000_000, amount_msat);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -1688,7 +1696,7 @@ fn test_monitor_update_on_pending_forwards() {
	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

	let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
	nodes[2].node.fail_htlc_backwards(&payment_hash_1);
	expect_pending_htlcs_forwardable!(nodes[2]);
	check_added_monitors!(nodes[2], 1);

Expand DownExpand Up@@ -1754,7 +1762,7 @@ fn monitor_update_claim_fail_no_response() {
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	// Forward a payment for B to claim
	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	// Now start forwarding a second payment, skipping the last RAA so B is in AwaitingRAA
	let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand All@@ -1770,8 +1778,10 @@ fn monitor_update_claim_fail_no_response() {
	let as_raa = commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false, true, false, true);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	check_added_monitors!(nodes[1], 1);

	let events = nodes[1].node.get_and_clear_pending_msg_events();
	assert_eq!(events.len(), 0);
	nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Temporary failure claiming HTLC, treating as success: Failed to update ChannelMonitor".to_string(), 1);
Expand DownExpand Up@@ -2076,13 +2086,15 @@ fn test_fail_htlc_on_broadcast_after_claim() {
	create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
	let chan_id_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known()).2;

	let payment_preimage= route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000).0;
	let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000);

	let bs_txn = get_local_commitment_txn!(nodes[2], chan_id_2);
	assert_eq!(bs_txn.len(), 1);

	nodes[2].node.claim_funds(payment_preimage);
	check_added_monitors!(nodes[2], 1);
	expect_payment_claimed!(nodes[2], payment_hash, 2000);

	let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &cs_updates.update_fulfill_htlcs[0]);
	let bs_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
Expand DownExpand Up@@ -2235,7 +2247,7 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
	//
	// Note that because, at the end, MonitorUpdateFailed is still set, the HTLC generated in (c)
	// will not be freed from the holding cell.
	let (payment_preimage_0, _, _) = route_payment(&nodes[1], &[&nodes[0]], 100000);
	let (payment_preimage_0, payment_hash_0, _) = route_payment(&nodes[1], &[&nodes[0]], 100_000);

	nodes[0].node.send_payment(&route, payment_hash_1, &Some(payment_secret_1)).unwrap();
	check_added_monitors!(nodes[0], 1);
Expand All@@ -2246,8 +2258,9 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
	check_added_monitors!(nodes[0], 0);

	chanmon_cfgs[0].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
assert!(nodes[0].node.claim_funds(payment_preimage_0));
	nodes[0].node.claim_funds(payment_preimage_0);
	check_added_monitors!(nodes[0], 1);
	expect_payment_claimed!(nodes[0], payment_hash_0, 100_000);

	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &send.msgs[0]);
	nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &send.commitment_msg);
Expand DownExpand Up@@ -2455,13 +2468,15 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
 payment_preimage,
	};
	if second_fails {
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash));
 nodes[2].node.fail_htlc_backwards(&payment_hash);
 expect_pending_htlcs_forwardable!(nodes[2]);
 check_added_monitors!(nodes[2], 1);
 get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	} else {
assert!(nodes[2].node.claim_funds(payment_preimage));
 nodes[2].node.claim_funds(payment_preimage);
 check_added_monitors!(nodes[2], 1);
 expect_payment_claimed!(nodes[2], payment_hash, 100_000);

 let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
 assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
 // Check that the message we're about to deliver matches the one generated:
Expand DownExpand Up@@ -2630,20 +2645,22 @@ fn double_temp_error() {

	let (_, _, channel_id, _) = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_2, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
	let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	// `claim_funds` results in a ChannelMonitorUpdate.
assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	let (funding_tx, latest_update_1, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	// Previously, this would've panicked due to a double-call to `Channel::monitor_update_failed`,
	// which had some asserts that prevented it from being called twice.
assert!(nodes[1].node.claim_funds(payment_preimage_2));
	nodes[1].node.claim_funds(payment_preimage_2);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);
	chanmon_cfgs[1].persister.set_update_ret(Ok(()));

	let (_, latest_update_2, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Improve Robustness of Inbound MPP Claims Across Restart#1434

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

valentinewallace
merged 9 commits into

from

May 30, 2022
 Merged


Improve Robustness of Inbound MPP Claims Across Restart
#1434
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


9 commits


Select commit
Hold shift + click to select a range
2a42595

Enable removal of `OnionPayload::Invoice::_legacy_hop_data` later


TheBlueMatt May 4, 2022
bd1e20d

Store an `events::PaymentPurpose` with each claimable payment


TheBlueMatt May 4, 2022
28c70ac

Ensure all HTLCs for a claimed payment are claimed on startup


TheBlueMatt Apr 18, 2022
0a2a40c

Add a `PaymentClaimed` event to indicate a payment was claimed


TheBlueMatt Apr 18, 2022
0e25421

Provide a redundant `Event::PaymentClaimed` on restart if needed


TheBlueMatt May 4, 2022
11c2f12

Do additional pre-flight checks before claiming a payment


TheBlueMatt Apr 19, 2022
a12d37e

Drop return value from `fail_htlc_backwards`, clarify docs


TheBlueMatt Apr 19, 2022
8a56702

Add internal docs for ChannelMonitor::payment_preimages


TheBlueMatt Apr 22, 2022
531d6c8

Change `Event` `amt` fields to `amount_msat` for clarity


TheBlueMatt May 28, 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


5 changes: 3 additions & 2 deletions
fuzz/src/chanmon_consistency.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -840,13 +840,14 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 events::Event::PaymentReceived { payment_hash, .. } => {
 if claim_set.insert(payment_hash.0) {
 if $fail {
assert!(nodes[$node].fail_htlc_backwards(&payment_hash));
 nodes[$node].fail_htlc_backwards(&payment_hash);
 } else {
assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0)));
 nodes[$node].claim_funds(PaymentPreimage(payment_hash.0));
 }
 }
 },
 events::Event::PaymentSent { .. } => {},
 events::Event::PaymentClaimed { .. } => {},
 events::Event::PaymentPathSuccessful { .. } => {},
 events::Event::PaymentPathFailed { .. } => {},
 events::Event::PaymentForwarded { .. } if $node == 1 => {},
Expand Down

11 changes: 7 additions & 4 deletions
lightning/src/chain/chainmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -731,7 +731,7 @@ impl<ChannelSigner: Sign, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> even
mod tests {
	use bitcoin::BlockHeader;
	use ::{check_added_monitors, check_closed_broadcast, check_closed_event};
	use ::{expect_payment_sent, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
	use ::{expect_payment_sent, expect_payment_claimed, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
	use ::{get_htlc_update_msgs, get_local_commitment_txn, get_revoke_commit_msgs, get_route_and_payment_hash, unwrap_send_err};
	use chain::{ChannelMonitorUpdateErr, Confirm, Watch};
	use chain::channelmonitor::LATENCY_GRACE_PERIOD_BLOCKS;
Expand DownExpand Up@@ -798,16 +798,18 @@ mod tests {
 create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

 // Route two payments to be claimed at the same time.
 let payment_preimage_1= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let payment_preimage_2= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
 let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

 chanmon_cfgs[1].persister.offchain_monitor_updates.lock().unwrap().clear();
 chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));

 nodes[1].node.claim_funds(payment_preimage_1);
 check_added_monitors!(nodes[1], 1);
 expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
 nodes[1].node.claim_funds(payment_preimage_2);
 check_added_monitors!(nodes[1], 1);
 expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);

 chanmon_cfgs[1].persister.set_update_ret(Ok(()));

Expand DownExpand Up@@ -877,8 +879,9 @@ mod tests {
 let (route, second_payment_hash, _, second_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);

 // First route a payment that we will claim on chain and give the recipient the preimage.
 let payment_preimage= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
 nodes[1].node.claim_funds(payment_preimage);
 expect_payment_claimed!(nodes[1], payment_hash, 1_000_000);
 nodes[1].node.get_and_clear_pending_msg_events();
 check_added_monitors!(nodes[1], 1);
 let remote_txn = get_local_commitment_txn!(nodes[1], channel.2);
Expand Down

11 changes: 10 additions & 1 deletion
lightning/src/chain/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -655,6 +655,10 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
	// deserialization
	current_holder_commitment_number: u64,

	/// The set of payment hashes from inbound payments for which we know the preimage. Payment
	/// preimages that are not included in any unrevoked local commitment transaction or unrevoked
	/// remote commitment transactions are automatically removed when commitment transactions are
	/// revoked.
	payment_preimages: HashMap<PaymentHash, PaymentPreimage>,

	// Note that `MonitorEvent`s MUST NOT be generated during update processing, only generated
Expand DownExpand Up@@ -1085,7 +1089,8 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs).map_err(|_| ())
	}

	#[cfg(test)]
	/// This is used to provide payment preimage(s) out-of-band during startup without updating the
	/// off-chain state with a new commitment transaction.
	pub(crate) fn provide_payment_preimage<B: Deref, F: Deref, L: Deref>(
 &self,
 payment_hash: &PaymentHash,
Expand DownExpand Up@@ -1631,6 +1636,10 @@ impl<Signer: Sign> ChannelMonitor<Signer> {

 res
	}

	pub(crate) fn get_stored_preimages(&self) -> HashMap<PaymentHash, PaymentPreimage> {
 self.inner.lock().unwrap().payment_preimages.clone()
Comment thread
TheBlueMatt marked this conversation as resolved.

	}
}

/// Compares a broadcasted commitment transaction's HTLCs with those in the latest state,
Expand Down

83 changes: 50 additions & 33 deletions
lightning/src/ln/chanmon_update_fail_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -89,7 +89,7 @@ fn test_monitor_and_persister_update_fail() {
	send_payment(&nodes[0], &vec!(&nodes[1])[..], 10_000_000);

	// Route an HTLC from node 0 to node 1 (but don't settle)
	let preimage= route_payment(&nodes[0], &vec!(&nodes[1])[..], 9_000_000).0;
	let (preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 9_000_000);

	// Make a copy of the ChainMonitor so we can capture the error it returns on a
	// bogus update. Note that if instead we updated the nodes[0]'s ChainMonitor
Expand DownExpand Up@@ -123,8 +123,10 @@ fn test_monitor_and_persister_update_fail() {
	persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));

	// Try to update ChannelMonitor
	assert!(nodes[1].node.claim_funds(preimage));
	nodes[1].node.claim_funds(preimage);
	expect_payment_claimed!(nodes[1], payment_hash, 9_000_000);
	check_added_monitors!(nodes[1], 1);

	let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
	assert_eq!(updates.update_fulfill_htlcs.len(), 1);
	nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
Expand DownExpand Up@@ -189,9 +191,9 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
	let events_3 = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events_3.len(), 1);
	match events_3[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_1, *payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -267,7 +269,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	// Now try to send a second payment which will fail to send
	let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand All@@ -283,8 +285,10 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {

	// Claim the previous payment, which will result in a update_fulfill_htlc/CS from nodes[1]
	// but nodes[0] won't respond since it is frozen.
assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);

	let events_2 = nodes[1].node.get_and_clear_pending_msg_events();
	assert_eq!(events_2.len(), 1);
	let (bs_initial_fulfill, bs_initial_commitment_signed) = match events_2[0] {
Expand DownExpand Up@@ -555,9 +559,9 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
	let events_5 = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events_5.len(), 1);
	match events_5[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_2, *payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -672,9 +676,9 @@ fn test_monitor_update_fail_cs() {
	let events = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events.len(), 1);
	match events[0] {
 Event::PaymentReceived { payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash, our_payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -827,7 +831,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
	let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);

	// Fail the payment backwards, failing the monitor update on nodes[1]'s receipt of the RAA
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
	nodes[2].node.fail_htlc_backwards(&payment_hash_1);
	expect_pending_htlcs_forwardable!(nodes[2]);
	check_added_monitors!(nodes[2], 1);

Expand DownExpand Up@@ -1088,13 +1092,15 @@ fn test_monitor_update_fail_reestablish() {
	let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
	create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());

	let (payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
	let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);

	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);

assert!(nodes[2].node.claim_funds(payment_preimage));
	nodes[2].node.claim_funds(payment_preimage);
	check_added_monitors!(nodes[2], 1);
	expect_payment_claimed!(nodes[2], payment_hash, 1_000_000);

	let mut updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	assert!(updates.update_add_htlcs.is_empty());
	assert!(updates.update_fail_htlcs.is_empty());
Expand DownExpand Up@@ -1292,13 +1298,14 @@ fn claim_while_disconnected_monitor_update_fail() {
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	// Forward a payment for B to claim
	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);

assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);

	nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
	nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
Expand DownExpand Up@@ -1578,10 +1585,11 @@ fn test_monitor_update_fail_claim() {
	// Rebalance a bit so that we can send backwards from 3 to 2.
	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Temporary failure claiming HTLC, treating as success: Failed to update ChannelMonitor".to_string(), 1);
	check_added_monitors!(nodes[1], 1);

Expand DownExpand Up@@ -1642,9 +1650,9 @@ fn test_monitor_update_fail_claim() {
	let events = nodes[0].node.get_and_clear_pending_events();
	assert_eq!(events.len(), 2);
	match events[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_2, *payment_hash);
 assert_eq!(1_000_000, amt);
 assert_eq!(1_000_000, amount_msat);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand All@@ -1656,9 +1664,9 @@ fn test_monitor_update_fail_claim() {
 _ => panic!("Unexpected event"),
	}
	match events[1] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_3, *payment_hash);
 assert_eq!(1_000_000, amt);
 assert_eq!(1_000_000, amount_msat);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -1688,7 +1696,7 @@ fn test_monitor_update_on_pending_forwards() {
	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

	let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
	nodes[2].node.fail_htlc_backwards(&payment_hash_1);
	expect_pending_htlcs_forwardable!(nodes[2]);
	check_added_monitors!(nodes[2], 1);

Expand DownExpand Up@@ -1754,7 +1762,7 @@ fn monitor_update_claim_fail_no_response() {
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	// Forward a payment for B to claim
	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	// Now start forwarding a second payment, skipping the last RAA so B is in AwaitingRAA
	let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand All@@ -1770,8 +1778,10 @@ fn monitor_update_claim_fail_no_response() {
	let as_raa = commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false, true, false, true);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	check_added_monitors!(nodes[1], 1);

	let events = nodes[1].node.get_and_clear_pending_msg_events();
	assert_eq!(events.len(), 0);
	nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Temporary failure claiming HTLC, treating as success: Failed to update ChannelMonitor".to_string(), 1);
Expand DownExpand Up@@ -2076,13 +2086,15 @@ fn test_fail_htlc_on_broadcast_after_claim() {
	create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
	let chan_id_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known()).2;

	let payment_preimage= route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000).0;
	let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000);

	let bs_txn = get_local_commitment_txn!(nodes[2], chan_id_2);
	assert_eq!(bs_txn.len(), 1);

	nodes[2].node.claim_funds(payment_preimage);
	check_added_monitors!(nodes[2], 1);
	expect_payment_claimed!(nodes[2], payment_hash, 2000);

	let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &cs_updates.update_fulfill_htlcs[0]);
	let bs_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
Expand DownExpand Up@@ -2235,7 +2247,7 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
	//
	// Note that because, at the end, MonitorUpdateFailed is still set, the HTLC generated in (c)
	// will not be freed from the holding cell.
	let (payment_preimage_0, _, _) = route_payment(&nodes[1], &[&nodes[0]], 100000);
	let (payment_preimage_0, payment_hash_0, _) = route_payment(&nodes[1], &[&nodes[0]], 100_000);

	nodes[0].node.send_payment(&route, payment_hash_1, &Some(payment_secret_1)).unwrap();
	check_added_monitors!(nodes[0], 1);
Expand All@@ -2246,8 +2258,9 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
	check_added_monitors!(nodes[0], 0);

	chanmon_cfgs[0].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
assert!(nodes[0].node.claim_funds(payment_preimage_0));
	nodes[0].node.claim_funds(payment_preimage_0);
	check_added_monitors!(nodes[0], 1);
	expect_payment_claimed!(nodes[0], payment_hash_0, 100_000);

	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &send.msgs[0]);
	nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &send.commitment_msg);
Expand DownExpand Up@@ -2455,13 +2468,15 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
 payment_preimage,
	};
	if second_fails {
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash));
 nodes[2].node.fail_htlc_backwards(&payment_hash);
 expect_pending_htlcs_forwardable!(nodes[2]);
 check_added_monitors!(nodes[2], 1);
 get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	} else {
assert!(nodes[2].node.claim_funds(payment_preimage));
 nodes[2].node.claim_funds(payment_preimage);
 check_added_monitors!(nodes[2], 1);
 expect_payment_claimed!(nodes[2], payment_hash, 100_000);

 let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
 assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
 // Check that the message we're about to deliver matches the one generated:
Expand DownExpand Up@@ -2630,20 +2645,22 @@ fn double_temp_error() {

	let (_, _, channel_id, _) = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_2, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
	let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	// `claim_funds` results in a ChannelMonitorUpdate.
assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	let (funding_tx, latest_update_1, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	// Previously, this would've panicked due to a double-call to `Channel::monitor_update_failed`,
	// which had some asserts that prevented it from being called twice.
assert!(nodes[1].node.claim_funds(payment_preimage_2));
	nodes[1].node.claim_funds(payment_preimage_2);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);
	chanmon_cfgs[1].persister.set_update_ret(Ok(()));

	let (_, latest_update_2, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Improve Robustness of Inbound MPP Claims Across Restart#1434

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

valentinewallace
merged 9 commits into

from

May 30, 2022
 Merged


Improve Robustness of Inbound MPP Claims Across Restart
#1434
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


9 commits


Select commit
Hold shift + click to select a range
2a42595

Enable removal of `OnionPayload::Invoice::_legacy_hop_data` later


TheBlueMatt May 4, 2022
bd1e20d

Store an `events::PaymentPurpose` with each claimable payment


TheBlueMatt May 4, 2022
28c70ac

Ensure all HTLCs for a claimed payment are claimed on startup


TheBlueMatt Apr 18, 2022
0a2a40c

Add a `PaymentClaimed` event to indicate a payment was claimed


TheBlueMatt Apr 18, 2022
0e25421

Provide a redundant `Event::PaymentClaimed` on restart if needed


TheBlueMatt May 4, 2022
11c2f12

Do additional pre-flight checks before claiming a payment


TheBlueMatt Apr 19, 2022
a12d37e

Drop return value from `fail_htlc_backwards`, clarify docs


TheBlueMatt Apr 19, 2022
8a56702

Add internal docs for ChannelMonitor::payment_preimages


TheBlueMatt Apr 22, 2022
531d6c8

Change `Event` `amt` fields to `amount_msat` for clarity


TheBlueMatt May 28, 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


5 changes: 3 additions & 2 deletions
fuzz/src/chanmon_consistency.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -840,13 +840,14 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 events::Event::PaymentReceived { payment_hash, .. } => {
 if claim_set.insert(payment_hash.0) {
 if $fail {
assert!(nodes[$node].fail_htlc_backwards(&payment_hash));
 nodes[$node].fail_htlc_backwards(&payment_hash);
 } else {
assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0)));
 nodes[$node].claim_funds(PaymentPreimage(payment_hash.0));
 }
 }
 },
 events::Event::PaymentSent { .. } => {},
 events::Event::PaymentClaimed { .. } => {},
 events::Event::PaymentPathSuccessful { .. } => {},
 events::Event::PaymentPathFailed { .. } => {},
 events::Event::PaymentForwarded { .. } if $node == 1 => {},
Expand Down

11 changes: 7 additions & 4 deletions
lightning/src/chain/chainmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -731,7 +731,7 @@ impl<ChannelSigner: Sign, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> even
mod tests {
	use bitcoin::BlockHeader;
	use ::{check_added_monitors, check_closed_broadcast, check_closed_event};
	use ::{expect_payment_sent, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
	use ::{expect_payment_sent, expect_payment_claimed, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
	use ::{get_htlc_update_msgs, get_local_commitment_txn, get_revoke_commit_msgs, get_route_and_payment_hash, unwrap_send_err};
	use chain::{ChannelMonitorUpdateErr, Confirm, Watch};
	use chain::channelmonitor::LATENCY_GRACE_PERIOD_BLOCKS;
Expand DownExpand Up@@ -798,16 +798,18 @@ mod tests {
 create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

 // Route two payments to be claimed at the same time.
 let payment_preimage_1= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let payment_preimage_2= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
 let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

 chanmon_cfgs[1].persister.offchain_monitor_updates.lock().unwrap().clear();
 chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));

 nodes[1].node.claim_funds(payment_preimage_1);
 check_added_monitors!(nodes[1], 1);
 expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
 nodes[1].node.claim_funds(payment_preimage_2);
 check_added_monitors!(nodes[1], 1);
 expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);

 chanmon_cfgs[1].persister.set_update_ret(Ok(()));

Expand DownExpand Up@@ -877,8 +879,9 @@ mod tests {
 let (route, second_payment_hash, _, second_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);

 // First route a payment that we will claim on chain and give the recipient the preimage.
 let payment_preimage= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
 nodes[1].node.claim_funds(payment_preimage);
 expect_payment_claimed!(nodes[1], payment_hash, 1_000_000);
 nodes[1].node.get_and_clear_pending_msg_events();
 check_added_monitors!(nodes[1], 1);
 let remote_txn = get_local_commitment_txn!(nodes[1], channel.2);
Expand Down

11 changes: 10 additions & 1 deletion
lightning/src/chain/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -655,6 +655,10 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
	// deserialization
	current_holder_commitment_number: u64,

	/// The set of payment hashes from inbound payments for which we know the preimage. Payment
	/// preimages that are not included in any unrevoked local commitment transaction or unrevoked
	/// remote commitment transactions are automatically removed when commitment transactions are
	/// revoked.
	payment_preimages: HashMap<PaymentHash, PaymentPreimage>,

	// Note that `MonitorEvent`s MUST NOT be generated during update processing, only generated
Expand DownExpand Up@@ -1085,7 +1089,8 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs).map_err(|_| ())
	}

	#[cfg(test)]
	/// This is used to provide payment preimage(s) out-of-band during startup without updating the
	/// off-chain state with a new commitment transaction.
	pub(crate) fn provide_payment_preimage<B: Deref, F: Deref, L: Deref>(
 &self,
 payment_hash: &PaymentHash,
Expand DownExpand Up@@ -1631,6 +1636,10 @@ impl<Signer: Sign> ChannelMonitor<Signer> {

 res
	}

	pub(crate) fn get_stored_preimages(&self) -> HashMap<PaymentHash, PaymentPreimage> {
 self.inner.lock().unwrap().payment_preimages.clone()
Comment thread
TheBlueMatt marked this conversation as resolved.

	}
}

/// Compares a broadcasted commitment transaction's HTLCs with those in the latest state,
Expand Down

83 changes: 50 additions & 33 deletions
lightning/src/ln/chanmon_update_fail_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -89,7 +89,7 @@ fn test_monitor_and_persister_update_fail() {
	send_payment(&nodes[0], &vec!(&nodes[1])[..], 10_000_000);

	// Route an HTLC from node 0 to node 1 (but don't settle)
	let preimage= route_payment(&nodes[0], &vec!(&nodes[1])[..], 9_000_000).0;
	let (preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 9_000_000);

	// Make a copy of the ChainMonitor so we can capture the error it returns on a
	// bogus update. Note that if instead we updated the nodes[0]'s ChainMonitor
Expand DownExpand Up@@ -123,8 +123,10 @@ fn test_monitor_and_persister_update_fail() {
	persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));

	// Try to update ChannelMonitor
	assert!(nodes[1].node.claim_funds(preimage));
	nodes[1].node.claim_funds(preimage);
	expect_payment_claimed!(nodes[1], payment_hash, 9_000_000);
	check_added_monitors!(nodes[1], 1);

	let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
	assert_eq!(updates.update_fulfill_htlcs.len(), 1);
	nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
Expand DownExpand Up@@ -189,9 +191,9 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
	let events_3 = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events_3.len(), 1);
	match events_3[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_1, *payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -267,7 +269,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	// Now try to send a second payment which will fail to send
	let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand All@@ -283,8 +285,10 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {

	// Claim the previous payment, which will result in a update_fulfill_htlc/CS from nodes[1]
	// but nodes[0] won't respond since it is frozen.
assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);

	let events_2 = nodes[1].node.get_and_clear_pending_msg_events();
	assert_eq!(events_2.len(), 1);
	let (bs_initial_fulfill, bs_initial_commitment_signed) = match events_2[0] {
Expand DownExpand Up@@ -555,9 +559,9 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
	let events_5 = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events_5.len(), 1);
	match events_5[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_2, *payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -672,9 +676,9 @@ fn test_monitor_update_fail_cs() {
	let events = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events.len(), 1);
	match events[0] {
 Event::PaymentReceived { payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash, our_payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -827,7 +831,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
	let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);

	// Fail the payment backwards, failing the monitor update on nodes[1]'s receipt of the RAA
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
	nodes[2].node.fail_htlc_backwards(&payment_hash_1);
	expect_pending_htlcs_forwardable!(nodes[2]);
	check_added_monitors!(nodes[2], 1);

Expand DownExpand Up@@ -1088,13 +1092,15 @@ fn test_monitor_update_fail_reestablish() {
	let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
	create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());

	let (payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
	let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);

	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);

assert!(nodes[2].node.claim_funds(payment_preimage));
	nodes[2].node.claim_funds(payment_preimage);
	check_added_monitors!(nodes[2], 1);
	expect_payment_claimed!(nodes[2], payment_hash, 1_000_000);

	let mut updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	assert!(updates.update_add_htlcs.is_empty());
	assert!(updates.update_fail_htlcs.is_empty());
Expand DownExpand Up@@ -1292,13 +1298,14 @@ fn claim_while_disconnected_monitor_update_fail() {
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	// Forward a payment for B to claim
	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);

assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);

	nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
	nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
Expand DownExpand Up@@ -1578,10 +1585,11 @@ fn test_monitor_update_fail_claim() {
	// Rebalance a bit so that we can send backwards from 3 to 2.
	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Temporary failure claiming HTLC, treating as success: Failed to update ChannelMonitor".to_string(), 1);
	check_added_monitors!(nodes[1], 1);

Expand DownExpand Up@@ -1642,9 +1650,9 @@ fn test_monitor_update_fail_claim() {
	let events = nodes[0].node.get_and_clear_pending_events();
	assert_eq!(events.len(), 2);
	match events[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_2, *payment_hash);
 assert_eq!(1_000_000, amt);
 assert_eq!(1_000_000, amount_msat);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand All@@ -1656,9 +1664,9 @@ fn test_monitor_update_fail_claim() {
 _ => panic!("Unexpected event"),
	}
	match events[1] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_3, *payment_hash);
 assert_eq!(1_000_000, amt);
 assert_eq!(1_000_000, amount_msat);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -1688,7 +1696,7 @@ fn test_monitor_update_on_pending_forwards() {
	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

	let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
	nodes[2].node.fail_htlc_backwards(&payment_hash_1);
	expect_pending_htlcs_forwardable!(nodes[2]);
	check_added_monitors!(nodes[2], 1);

Expand DownExpand Up@@ -1754,7 +1762,7 @@ fn monitor_update_claim_fail_no_response() {
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	// Forward a payment for B to claim
	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	// Now start forwarding a second payment, skipping the last RAA so B is in AwaitingRAA
	let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand All@@ -1770,8 +1778,10 @@ fn monitor_update_claim_fail_no_response() {
	let as_raa = commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false, true, false, true);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	check_added_monitors!(nodes[1], 1);

	let events = nodes[1].node.get_and_clear_pending_msg_events();
	assert_eq!(events.len(), 0);
	nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Temporary failure claiming HTLC, treating as success: Failed to update ChannelMonitor".to_string(), 1);
Expand DownExpand Up@@ -2076,13 +2086,15 @@ fn test_fail_htlc_on_broadcast_after_claim() {
	create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
	let chan_id_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known()).2;

	let payment_preimage= route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000).0;
	let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000);

	let bs_txn = get_local_commitment_txn!(nodes[2], chan_id_2);
	assert_eq!(bs_txn.len(), 1);

	nodes[2].node.claim_funds(payment_preimage);
	check_added_monitors!(nodes[2], 1);
	expect_payment_claimed!(nodes[2], payment_hash, 2000);

	let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &cs_updates.update_fulfill_htlcs[0]);
	let bs_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
Expand DownExpand Up@@ -2235,7 +2247,7 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
	//
	// Note that because, at the end, MonitorUpdateFailed is still set, the HTLC generated in (c)
	// will not be freed from the holding cell.
	let (payment_preimage_0, _, _) = route_payment(&nodes[1], &[&nodes[0]], 100000);
	let (payment_preimage_0, payment_hash_0, _) = route_payment(&nodes[1], &[&nodes[0]], 100_000);

	nodes[0].node.send_payment(&route, payment_hash_1, &Some(payment_secret_1)).unwrap();
	check_added_monitors!(nodes[0], 1);
Expand All@@ -2246,8 +2258,9 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
	check_added_monitors!(nodes[0], 0);

	chanmon_cfgs[0].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
assert!(nodes[0].node.claim_funds(payment_preimage_0));
	nodes[0].node.claim_funds(payment_preimage_0);
	check_added_monitors!(nodes[0], 1);
	expect_payment_claimed!(nodes[0], payment_hash_0, 100_000);

	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &send.msgs[0]);
	nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &send.commitment_msg);
Expand DownExpand Up@@ -2455,13 +2468,15 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
 payment_preimage,
	};
	if second_fails {
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash));
 nodes[2].node.fail_htlc_backwards(&payment_hash);
 expect_pending_htlcs_forwardable!(nodes[2]);
 check_added_monitors!(nodes[2], 1);
 get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	} else {
assert!(nodes[2].node.claim_funds(payment_preimage));
 nodes[2].node.claim_funds(payment_preimage);
 check_added_monitors!(nodes[2], 1);
 expect_payment_claimed!(nodes[2], payment_hash, 100_000);

 let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
 assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
 // Check that the message we're about to deliver matches the one generated:
Expand DownExpand Up@@ -2630,20 +2645,22 @@ fn double_temp_error() {

	let (_, _, channel_id, _) = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_2, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
	let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	// `claim_funds` results in a ChannelMonitorUpdate.
assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	let (funding_tx, latest_update_1, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	// Previously, this would've panicked due to a double-call to `Channel::monitor_update_failed`,
	// which had some asserts that prevented it from being called twice.
assert!(nodes[1].node.claim_funds(payment_preimage_2));
	nodes[1].node.claim_funds(payment_preimage_2);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);
	chanmon_cfgs[1].persister.set_update_ret(Ok(()));

	let (_, latest_update_2, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Improve Robustness of Inbound MPP Claims Across Restart#1434

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

valentinewallace
merged 9 commits into

from

May 30, 2022
 Merged


Improve Robustness of Inbound MPP Claims Across Restart
#1434
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


9 commits


Select commit
Hold shift + click to select a range
2a42595

Enable removal of `OnionPayload::Invoice::_legacy_hop_data` later


TheBlueMatt May 4, 2022
bd1e20d

Store an `events::PaymentPurpose` with each claimable payment


TheBlueMatt May 4, 2022
28c70ac

Ensure all HTLCs for a claimed payment are claimed on startup


TheBlueMatt Apr 18, 2022
0a2a40c

Add a `PaymentClaimed` event to indicate a payment was claimed


TheBlueMatt Apr 18, 2022
0e25421

Provide a redundant `Event::PaymentClaimed` on restart if needed


TheBlueMatt May 4, 2022
11c2f12

Do additional pre-flight checks before claiming a payment


TheBlueMatt Apr 19, 2022
a12d37e

Drop return value from `fail_htlc_backwards`, clarify docs


TheBlueMatt Apr 19, 2022
8a56702

Add internal docs for ChannelMonitor::payment_preimages


TheBlueMatt Apr 22, 2022
531d6c8

Change `Event` `amt` fields to `amount_msat` for clarity


TheBlueMatt May 28, 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


5 changes: 3 additions & 2 deletions
fuzz/src/chanmon_consistency.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -840,13 +840,14 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 events::Event::PaymentReceived { payment_hash, .. } => {
 if claim_set.insert(payment_hash.0) {
 if $fail {
assert!(nodes[$node].fail_htlc_backwards(&payment_hash));
 nodes[$node].fail_htlc_backwards(&payment_hash);
 } else {
assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0)));
 nodes[$node].claim_funds(PaymentPreimage(payment_hash.0));
 }
 }
 },
 events::Event::PaymentSent { .. } => {},
 events::Event::PaymentClaimed { .. } => {},
 events::Event::PaymentPathSuccessful { .. } => {},
 events::Event::PaymentPathFailed { .. } => {},
 events::Event::PaymentForwarded { .. } if $node == 1 => {},
Expand Down

11 changes: 7 additions & 4 deletions
lightning/src/chain/chainmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -731,7 +731,7 @@ impl<ChannelSigner: Sign, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> even
mod tests {
	use bitcoin::BlockHeader;
	use ::{check_added_monitors, check_closed_broadcast, check_closed_event};
	use ::{expect_payment_sent, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
	use ::{expect_payment_sent, expect_payment_claimed, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
	use ::{get_htlc_update_msgs, get_local_commitment_txn, get_revoke_commit_msgs, get_route_and_payment_hash, unwrap_send_err};
	use chain::{ChannelMonitorUpdateErr, Confirm, Watch};
	use chain::channelmonitor::LATENCY_GRACE_PERIOD_BLOCKS;
Expand DownExpand Up@@ -798,16 +798,18 @@ mod tests {
 create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

 // Route two payments to be claimed at the same time.
 let payment_preimage_1= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let payment_preimage_2= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
 let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

 chanmon_cfgs[1].persister.offchain_monitor_updates.lock().unwrap().clear();
 chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));

 nodes[1].node.claim_funds(payment_preimage_1);
 check_added_monitors!(nodes[1], 1);
 expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
 nodes[1].node.claim_funds(payment_preimage_2);
 check_added_monitors!(nodes[1], 1);
 expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);

 chanmon_cfgs[1].persister.set_update_ret(Ok(()));

Expand DownExpand Up@@ -877,8 +879,9 @@ mod tests {
 let (route, second_payment_hash, _, second_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);

 // First route a payment that we will claim on chain and give the recipient the preimage.
 let payment_preimage= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
 nodes[1].node.claim_funds(payment_preimage);
 expect_payment_claimed!(nodes[1], payment_hash, 1_000_000);
 nodes[1].node.get_and_clear_pending_msg_events();
 check_added_monitors!(nodes[1], 1);
 let remote_txn = get_local_commitment_txn!(nodes[1], channel.2);
Expand Down

11 changes: 10 additions & 1 deletion
lightning/src/chain/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -655,6 +655,10 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
	// deserialization
	current_holder_commitment_number: u64,

	/// The set of payment hashes from inbound payments for which we know the preimage. Payment
	/// preimages that are not included in any unrevoked local commitment transaction or unrevoked
	/// remote commitment transactions are automatically removed when commitment transactions are
	/// revoked.
	payment_preimages: HashMap<PaymentHash, PaymentPreimage>,

	// Note that `MonitorEvent`s MUST NOT be generated during update processing, only generated
Expand DownExpand Up@@ -1085,7 +1089,8 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs).map_err(|_| ())
	}

	#[cfg(test)]
	/// This is used to provide payment preimage(s) out-of-band during startup without updating the
	/// off-chain state with a new commitment transaction.
	pub(crate) fn provide_payment_preimage<B: Deref, F: Deref, L: Deref>(
 &self,
 payment_hash: &PaymentHash,
Expand DownExpand Up@@ -1631,6 +1636,10 @@ impl<Signer: Sign> ChannelMonitor<Signer> {

 res
	}

	pub(crate) fn get_stored_preimages(&self) -> HashMap<PaymentHash, PaymentPreimage> {
 self.inner.lock().unwrap().payment_preimages.clone()
Comment thread
TheBlueMatt marked this conversation as resolved.

	}
}

/// Compares a broadcasted commitment transaction's HTLCs with those in the latest state,
Expand Down

83 changes: 50 additions & 33 deletions
lightning/src/ln/chanmon_update_fail_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -89,7 +89,7 @@ fn test_monitor_and_persister_update_fail() {
	send_payment(&nodes[0], &vec!(&nodes[1])[..], 10_000_000);

	// Route an HTLC from node 0 to node 1 (but don't settle)
	let preimage= route_payment(&nodes[0], &vec!(&nodes[1])[..], 9_000_000).0;
	let (preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 9_000_000);

	// Make a copy of the ChainMonitor so we can capture the error it returns on a
	// bogus update. Note that if instead we updated the nodes[0]'s ChainMonitor
Expand DownExpand Up@@ -123,8 +123,10 @@ fn test_monitor_and_persister_update_fail() {
	persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));

	// Try to update ChannelMonitor
	assert!(nodes[1].node.claim_funds(preimage));
	nodes[1].node.claim_funds(preimage);
	expect_payment_claimed!(nodes[1], payment_hash, 9_000_000);
	check_added_monitors!(nodes[1], 1);

	let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
	assert_eq!(updates.update_fulfill_htlcs.len(), 1);
	nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
Expand DownExpand Up@@ -189,9 +191,9 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
	let events_3 = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events_3.len(), 1);
	match events_3[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_1, *payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -267,7 +269,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	// Now try to send a second payment which will fail to send
	let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand All@@ -283,8 +285,10 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {

	// Claim the previous payment, which will result in a update_fulfill_htlc/CS from nodes[1]
	// but nodes[0] won't respond since it is frozen.
assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);

	let events_2 = nodes[1].node.get_and_clear_pending_msg_events();
	assert_eq!(events_2.len(), 1);
	let (bs_initial_fulfill, bs_initial_commitment_signed) = match events_2[0] {
Expand DownExpand Up@@ -555,9 +559,9 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
	let events_5 = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events_5.len(), 1);
	match events_5[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_2, *payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -672,9 +676,9 @@ fn test_monitor_update_fail_cs() {
	let events = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events.len(), 1);
	match events[0] {
 Event::PaymentReceived { payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash, our_payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -827,7 +831,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
	let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);

	// Fail the payment backwards, failing the monitor update on nodes[1]'s receipt of the RAA
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
	nodes[2].node.fail_htlc_backwards(&payment_hash_1);
	expect_pending_htlcs_forwardable!(nodes[2]);
	check_added_monitors!(nodes[2], 1);

Expand DownExpand Up@@ -1088,13 +1092,15 @@ fn test_monitor_update_fail_reestablish() {
	let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
	create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());

	let (payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
	let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);

	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);

assert!(nodes[2].node.claim_funds(payment_preimage));
	nodes[2].node.claim_funds(payment_preimage);
	check_added_monitors!(nodes[2], 1);
	expect_payment_claimed!(nodes[2], payment_hash, 1_000_000);

	let mut updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	assert!(updates.update_add_htlcs.is_empty());
	assert!(updates.update_fail_htlcs.is_empty());
Expand DownExpand Up@@ -1292,13 +1298,14 @@ fn claim_while_disconnected_monitor_update_fail() {
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	// Forward a payment for B to claim
	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);

assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);

	nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
	nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
Expand DownExpand Up@@ -1578,10 +1585,11 @@ fn test_monitor_update_fail_claim() {
	// Rebalance a bit so that we can send backwards from 3 to 2.
	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Temporary failure claiming HTLC, treating as success: Failed to update ChannelMonitor".to_string(), 1);
	check_added_monitors!(nodes[1], 1);

Expand DownExpand Up@@ -1642,9 +1650,9 @@ fn test_monitor_update_fail_claim() {
	let events = nodes[0].node.get_and_clear_pending_events();
	assert_eq!(events.len(), 2);
	match events[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_2, *payment_hash);
 assert_eq!(1_000_000, amt);
 assert_eq!(1_000_000, amount_msat);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand All@@ -1656,9 +1664,9 @@ fn test_monitor_update_fail_claim() {
 _ => panic!("Unexpected event"),
	}
	match events[1] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_3, *payment_hash);
 assert_eq!(1_000_000, amt);
 assert_eq!(1_000_000, amount_msat);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -1688,7 +1696,7 @@ fn test_monitor_update_on_pending_forwards() {
	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

	let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
	nodes[2].node.fail_htlc_backwards(&payment_hash_1);
	expect_pending_htlcs_forwardable!(nodes[2]);
	check_added_monitors!(nodes[2], 1);

Expand DownExpand Up@@ -1754,7 +1762,7 @@ fn monitor_update_claim_fail_no_response() {
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	// Forward a payment for B to claim
	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	// Now start forwarding a second payment, skipping the last RAA so B is in AwaitingRAA
	let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand All@@ -1770,8 +1778,10 @@ fn monitor_update_claim_fail_no_response() {
	let as_raa = commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false, true, false, true);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	check_added_monitors!(nodes[1], 1);

	let events = nodes[1].node.get_and_clear_pending_msg_events();
	assert_eq!(events.len(), 0);
	nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Temporary failure claiming HTLC, treating as success: Failed to update ChannelMonitor".to_string(), 1);
Expand DownExpand Up@@ -2076,13 +2086,15 @@ fn test_fail_htlc_on_broadcast_after_claim() {
	create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
	let chan_id_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known()).2;

	let payment_preimage= route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000).0;
	let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000);

	let bs_txn = get_local_commitment_txn!(nodes[2], chan_id_2);
	assert_eq!(bs_txn.len(), 1);

	nodes[2].node.claim_funds(payment_preimage);
	check_added_monitors!(nodes[2], 1);
	expect_payment_claimed!(nodes[2], payment_hash, 2000);

	let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &cs_updates.update_fulfill_htlcs[0]);
	let bs_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
Expand DownExpand Up@@ -2235,7 +2247,7 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
	//
	// Note that because, at the end, MonitorUpdateFailed is still set, the HTLC generated in (c)
	// will not be freed from the holding cell.
	let (payment_preimage_0, _, _) = route_payment(&nodes[1], &[&nodes[0]], 100000);
	let (payment_preimage_0, payment_hash_0, _) = route_payment(&nodes[1], &[&nodes[0]], 100_000);

	nodes[0].node.send_payment(&route, payment_hash_1, &Some(payment_secret_1)).unwrap();
	check_added_monitors!(nodes[0], 1);
Expand All@@ -2246,8 +2258,9 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
	check_added_monitors!(nodes[0], 0);

	chanmon_cfgs[0].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
assert!(nodes[0].node.claim_funds(payment_preimage_0));
	nodes[0].node.claim_funds(payment_preimage_0);
	check_added_monitors!(nodes[0], 1);
	expect_payment_claimed!(nodes[0], payment_hash_0, 100_000);

	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &send.msgs[0]);
	nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &send.commitment_msg);
Expand DownExpand Up@@ -2455,13 +2468,15 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
 payment_preimage,
	};
	if second_fails {
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash));
 nodes[2].node.fail_htlc_backwards(&payment_hash);
 expect_pending_htlcs_forwardable!(nodes[2]);
 check_added_monitors!(nodes[2], 1);
 get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	} else {
assert!(nodes[2].node.claim_funds(payment_preimage));
 nodes[2].node.claim_funds(payment_preimage);
 check_added_monitors!(nodes[2], 1);
 expect_payment_claimed!(nodes[2], payment_hash, 100_000);

 let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
 assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
 // Check that the message we're about to deliver matches the one generated:
Expand DownExpand Up@@ -2630,20 +2645,22 @@ fn double_temp_error() {

	let (_, _, channel_id, _) = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_2, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
	let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	// `claim_funds` results in a ChannelMonitorUpdate.
assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	let (funding_tx, latest_update_1, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	// Previously, this would've panicked due to a double-call to `Channel::monitor_update_failed`,
	// which had some asserts that prevented it from being called twice.
assert!(nodes[1].node.claim_funds(payment_preimage_2));
	nodes[1].node.claim_funds(payment_preimage_2);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);
	chanmon_cfgs[1].persister.set_update_ret(Ok(()));

	let (_, latest_update_2, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Improve Robustness of Inbound MPP Claims Across Restart#1434

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

valentinewallace
merged 9 commits into

from

May 30, 2022
 Merged


Improve Robustness of Inbound MPP Claims Across Restart
#1434
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


9 commits


Select commit
Hold shift + click to select a range
2a42595

Enable removal of `OnionPayload::Invoice::_legacy_hop_data` later


TheBlueMatt May 4, 2022
bd1e20d

Store an `events::PaymentPurpose` with each claimable payment


TheBlueMatt May 4, 2022
28c70ac

Ensure all HTLCs for a claimed payment are claimed on startup


TheBlueMatt Apr 18, 2022
0a2a40c

Add a `PaymentClaimed` event to indicate a payment was claimed


TheBlueMatt Apr 18, 2022
0e25421

Provide a redundant `Event::PaymentClaimed` on restart if needed


TheBlueMatt May 4, 2022
11c2f12

Do additional pre-flight checks before claiming a payment


TheBlueMatt Apr 19, 2022
a12d37e

Drop return value from `fail_htlc_backwards`, clarify docs


TheBlueMatt Apr 19, 2022
8a56702

Add internal docs for ChannelMonitor::payment_preimages


TheBlueMatt Apr 22, 2022
531d6c8

Change `Event` `amt` fields to `amount_msat` for clarity


TheBlueMatt May 28, 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


5 changes: 3 additions & 2 deletions
fuzz/src/chanmon_consistency.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -840,13 +840,14 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
 events::Event::PaymentReceived { payment_hash, .. } => {
 if claim_set.insert(payment_hash.0) {
 if $fail {
assert!(nodes[$node].fail_htlc_backwards(&payment_hash));
 nodes[$node].fail_htlc_backwards(&payment_hash);
 } else {
assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0)));
 nodes[$node].claim_funds(PaymentPreimage(payment_hash.0));
 }
 }
 },
 events::Event::PaymentSent { .. } => {},
 events::Event::PaymentClaimed { .. } => {},
 events::Event::PaymentPathSuccessful { .. } => {},
 events::Event::PaymentPathFailed { .. } => {},
 events::Event::PaymentForwarded { .. } if $node == 1 => {},
Expand Down

11 changes: 7 additions & 4 deletions
lightning/src/chain/chainmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -731,7 +731,7 @@ impl<ChannelSigner: Sign, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref> even
mod tests {
	use bitcoin::BlockHeader;
	use ::{check_added_monitors, check_closed_broadcast, check_closed_event};
	use ::{expect_payment_sent, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
	use ::{expect_payment_sent, expect_payment_claimed, expect_payment_sent_without_paths, expect_payment_path_successful, get_event_msg};
	use ::{get_htlc_update_msgs, get_local_commitment_txn, get_revoke_commit_msgs, get_route_and_payment_hash, unwrap_send_err};
	use chain::{ChannelMonitorUpdateErr, Confirm, Watch};
	use chain::channelmonitor::LATENCY_GRACE_PERIOD_BLOCKS;
Expand DownExpand Up@@ -798,16 +798,18 @@ mod tests {
 create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

 // Route two payments to be claimed at the same time.
 let payment_preimage_1= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let payment_preimage_2= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
 let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

 chanmon_cfgs[1].persister.offchain_monitor_updates.lock().unwrap().clear();
 chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));

 nodes[1].node.claim_funds(payment_preimage_1);
 check_added_monitors!(nodes[1], 1);
 expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
 nodes[1].node.claim_funds(payment_preimage_2);
 check_added_monitors!(nodes[1], 1);
 expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);

 chanmon_cfgs[1].persister.set_update_ret(Ok(()));

Expand DownExpand Up@@ -877,8 +879,9 @@ mod tests {
 let (route, second_payment_hash, _, second_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);

 // First route a payment that we will claim on chain and give the recipient the preimage.
 let payment_preimage= route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
 nodes[1].node.claim_funds(payment_preimage);
 expect_payment_claimed!(nodes[1], payment_hash, 1_000_000);
 nodes[1].node.get_and_clear_pending_msg_events();
 check_added_monitors!(nodes[1], 1);
 let remote_txn = get_local_commitment_txn!(nodes[1], channel.2);
Expand Down

11 changes: 10 additions & 1 deletion
lightning/src/chain/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -655,6 +655,10 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
	// deserialization
	current_holder_commitment_number: u64,

	/// The set of payment hashes from inbound payments for which we know the preimage. Payment
	/// preimages that are not included in any unrevoked local commitment transaction or unrevoked
	/// remote commitment transactions are automatically removed when commitment transactions are
	/// revoked.
	payment_preimages: HashMap<PaymentHash, PaymentPreimage>,

	// Note that `MonitorEvent`s MUST NOT be generated during update processing, only generated
Expand DownExpand Up@@ -1085,7 +1089,8 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs).map_err(|_| ())
	}

	#[cfg(test)]
	/// This is used to provide payment preimage(s) out-of-band during startup without updating the
	/// off-chain state with a new commitment transaction.
	pub(crate) fn provide_payment_preimage<B: Deref, F: Deref, L: Deref>(
 &self,
 payment_hash: &PaymentHash,
Expand DownExpand Up@@ -1631,6 +1636,10 @@ impl<Signer: Sign> ChannelMonitor<Signer> {

 res
	}

	pub(crate) fn get_stored_preimages(&self) -> HashMap<PaymentHash, PaymentPreimage> {
 self.inner.lock().unwrap().payment_preimages.clone()
Comment thread
TheBlueMatt marked this conversation as resolved.

	}
}

/// Compares a broadcasted commitment transaction's HTLCs with those in the latest state,
Expand Down

83 changes: 50 additions & 33 deletions
lightning/src/ln/chanmon_update_fail_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -89,7 +89,7 @@ fn test_monitor_and_persister_update_fail() {
	send_payment(&nodes[0], &vec!(&nodes[1])[..], 10_000_000);

	// Route an HTLC from node 0 to node 1 (but don't settle)
	let preimage= route_payment(&nodes[0], &vec!(&nodes[1])[..], 9_000_000).0;
	let (preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], 9_000_000);

	// Make a copy of the ChainMonitor so we can capture the error it returns on a
	// bogus update. Note that if instead we updated the nodes[0]'s ChainMonitor
Expand DownExpand Up@@ -123,8 +123,10 @@ fn test_monitor_and_persister_update_fail() {
	persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));

	// Try to update ChannelMonitor
	assert!(nodes[1].node.claim_funds(preimage));
	nodes[1].node.claim_funds(preimage);
	expect_payment_claimed!(nodes[1], payment_hash, 9_000_000);
	check_added_monitors!(nodes[1], 1);

	let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
	assert_eq!(updates.update_fulfill_htlcs.len(), 1);
	nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
Expand DownExpand Up@@ -189,9 +191,9 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
	let events_3 = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events_3.len(), 1);
	match events_3[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_1, *payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -267,7 +269,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	// Now try to send a second payment which will fail to send
	let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand All@@ -283,8 +285,10 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {

	// Claim the previous payment, which will result in a update_fulfill_htlc/CS from nodes[1]
	// but nodes[0] won't respond since it is frozen.
assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);

	let events_2 = nodes[1].node.get_and_clear_pending_msg_events();
	assert_eq!(events_2.len(), 1);
	let (bs_initial_fulfill, bs_initial_commitment_signed) = match events_2[0] {
Expand DownExpand Up@@ -555,9 +559,9 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
	let events_5 = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events_5.len(), 1);
	match events_5[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_2, *payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -672,9 +676,9 @@ fn test_monitor_update_fail_cs() {
	let events = nodes[1].node.get_and_clear_pending_events();
	assert_eq!(events.len(), 1);
	match events[0] {
 Event::PaymentReceived { payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash, our_payment_hash);
 assert_eq!(amt, 1000000);
 assert_eq!(amount_msat, 1_000_000);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -827,7 +831,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
	let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);

	// Fail the payment backwards, failing the monitor update on nodes[1]'s receipt of the RAA
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
	nodes[2].node.fail_htlc_backwards(&payment_hash_1);
	expect_pending_htlcs_forwardable!(nodes[2]);
	check_added_monitors!(nodes[2], 1);

Expand DownExpand Up@@ -1088,13 +1092,15 @@ fn test_monitor_update_fail_reestablish() {
	let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
	create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known());

	let (payment_preimage, _, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
	let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);

	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);

assert!(nodes[2].node.claim_funds(payment_preimage));
	nodes[2].node.claim_funds(payment_preimage);
	check_added_monitors!(nodes[2], 1);
	expect_payment_claimed!(nodes[2], payment_hash, 1_000_000);

	let mut updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	assert!(updates.update_add_htlcs.is_empty());
	assert!(updates.update_fail_htlcs.is_empty());
Expand DownExpand Up@@ -1292,13 +1298,14 @@ fn claim_while_disconnected_monitor_update_fail() {
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	// Forward a payment for B to claim
	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);

assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);

	nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
	nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
Expand DownExpand Up@@ -1578,10 +1585,11 @@ fn test_monitor_update_fail_claim() {
	// Rebalance a bit so that we can send backwards from 3 to 2.
	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Temporary failure claiming HTLC, treating as success: Failed to update ChannelMonitor".to_string(), 1);
	check_added_monitors!(nodes[1], 1);

Expand DownExpand Up@@ -1642,9 +1650,9 @@ fn test_monitor_update_fail_claim() {
	let events = nodes[0].node.get_and_clear_pending_events();
	assert_eq!(events.len(), 2);
	match events[0] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_2, *payment_hash);
 assert_eq!(1_000_000, amt);
 assert_eq!(1_000_000, amount_msat);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand All@@ -1656,9 +1664,9 @@ fn test_monitor_update_fail_claim() {
 _ => panic!("Unexpected event"),
	}
	match events[1] {
 Event::PaymentReceived { ref payment_hash, ref purpose, amt } => {
 Event::PaymentReceived { ref payment_hash, ref purpose, amount_msat } => {
 assert_eq!(payment_hash_3, *payment_hash);
 assert_eq!(1_000_000, amt);
 assert_eq!(1_000_000, amount_msat);
 match &purpose {
 PaymentPurpose::InvoicePayment { payment_preimage, payment_secret, .. } => {
 assert!(payment_preimage.is_none());
Expand DownExpand Up@@ -1688,7 +1696,7 @@ fn test_monitor_update_on_pending_forwards() {
	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);

	let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
	nodes[2].node.fail_htlc_backwards(&payment_hash_1);
	expect_pending_htlcs_forwardable!(nodes[2]);
	check_added_monitors!(nodes[2], 1);

Expand DownExpand Up@@ -1754,7 +1762,7 @@ fn monitor_update_claim_fail_no_response() {
	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;

	// Forward a payment for B to claim
	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	// Now start forwarding a second payment, skipping the last RAA so B is in AwaitingRAA
	let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
Expand All@@ -1770,8 +1778,10 @@ fn monitor_update_claim_fail_no_response() {
	let as_raa = commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false, true, false, true);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	check_added_monitors!(nodes[1], 1);

	let events = nodes[1].node.get_and_clear_pending_msg_events();
	assert_eq!(events.len(), 0);
	nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Temporary failure claiming HTLC, treating as success: Failed to update ChannelMonitor".to_string(), 1);
Expand DownExpand Up@@ -2076,13 +2086,15 @@ fn test_fail_htlc_on_broadcast_after_claim() {
	create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
	let chan_id_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known()).2;

	let payment_preimage= route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000).0;
	let (payment_preimage, payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000);

	let bs_txn = get_local_commitment_txn!(nodes[2], chan_id_2);
	assert_eq!(bs_txn.len(), 1);

	nodes[2].node.claim_funds(payment_preimage);
	check_added_monitors!(nodes[2], 1);
	expect_payment_claimed!(nodes[2], payment_hash, 2000);

	let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &cs_updates.update_fulfill_htlcs[0]);
	let bs_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
Expand DownExpand Up@@ -2235,7 +2247,7 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
	//
	// Note that because, at the end, MonitorUpdateFailed is still set, the HTLC generated in (c)
	// will not be freed from the holding cell.
	let (payment_preimage_0, _, _) = route_payment(&nodes[1], &[&nodes[0]], 100000);
	let (payment_preimage_0, payment_hash_0, _) = route_payment(&nodes[1], &[&nodes[0]], 100_000);

	nodes[0].node.send_payment(&route, payment_hash_1, &Some(payment_secret_1)).unwrap();
	check_added_monitors!(nodes[0], 1);
Expand All@@ -2246,8 +2258,9 @@ fn do_channel_holding_cell_serialize(disconnect: bool, reload_a: bool) {
	check_added_monitors!(nodes[0], 0);

	chanmon_cfgs[0].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
assert!(nodes[0].node.claim_funds(payment_preimage_0));
	nodes[0].node.claim_funds(payment_preimage_0);
	check_added_monitors!(nodes[0], 1);
	expect_payment_claimed!(nodes[0], payment_hash_0, 100_000);

	nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &send.msgs[0]);
	nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &send.commitment_msg);
Expand DownExpand Up@@ -2455,13 +2468,15 @@ fn do_test_reconnect_dup_htlc_claims(htlc_status: HTLCStatusAtDupClaim, second_f
 payment_preimage,
	};
	if second_fails {
assert!(nodes[2].node.fail_htlc_backwards(&payment_hash));
 nodes[2].node.fail_htlc_backwards(&payment_hash);
 expect_pending_htlcs_forwardable!(nodes[2]);
 check_added_monitors!(nodes[2], 1);
 get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
	} else {
assert!(nodes[2].node.claim_funds(payment_preimage));
 nodes[2].node.claim_funds(payment_preimage);
 check_added_monitors!(nodes[2], 1);
 expect_payment_claimed!(nodes[2], payment_hash, 100_000);

 let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
 assert_eq!(cs_updates.update_fulfill_htlcs.len(), 1);
 // Check that the message we're about to deliver matches the one generated:
Expand DownExpand Up@@ -2630,20 +2645,22 @@ fn double_temp_error() {

	let (_, _, channel_id, _) = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());

	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_2, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
	let (payment_preimage_1, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
	let (payment_preimage_2, payment_hash_2, _) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	// `claim_funds` results in a ChannelMonitorUpdate.
assert!(nodes[1].node.claim_funds(payment_preimage_1));
	nodes[1].node.claim_funds(payment_preimage_1);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_1, 1_000_000);
	let (funding_tx, latest_update_1, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();

	chanmon_cfgs[1].persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
	// Previously, this would've panicked due to a double-call to `Channel::monitor_update_failed`,
	// which had some asserts that prevented it from being called twice.
assert!(nodes[1].node.claim_funds(payment_preimage_2));
	nodes[1].node.claim_funds(payment_preimage_2);
	check_added_monitors!(nodes[1], 1);
	expect_payment_claimed!(nodes[1], payment_hash_2, 1_000_000);
	chanmon_cfgs[1].persister.set_update_ret(Ok(()));

	let (_, latest_update_2, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();
Expand Down
Loading