From ed8f36520d7a5e8ba375719798906b884b483cc3 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 25 Feb 2022 05:18:29 +0000 Subject: [PATCH] Ensure `get_claimable_balances` never panicks in tests ... by calling it both before and after every chain event in testing and fuzzing. This requires fixing some blockchain inconsistencies in `do_test_onchain_htlc_reorg`, `do_retry_with_no_persist`, and `do_test_dup_htlc_onchain_fails_on_reload` where we'd connect conflicting transactions in the same chain. --- lightning/src/ln/functional_test_utils.rs | 13 +++++++++++++ lightning/src/ln/payment_tests.rs | 10 ++++++++-- lightning/src/ln/reorg_tests.rs | 10 ++++------ 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 1d9e5afc4..a908cd416 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -123,19 +123,29 @@ pub fn connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: &Block) do_connect_block(node, block, false); } +fn call_claimable_balances<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>) { + // Ensure `get_claimable_balances`' self-tests never panic + for funding_outpoint in node.chain_monitor.chain_monitor.list_monitors() { + node.chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances(); + } +} + fn do_connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: &Block, skip_intermediaries: bool) { + call_claimable_balances(node); let height = node.best_block_info().1 + 1; if !skip_intermediaries { let txdata: Vec<_> = block.txdata.iter().enumerate().collect(); match *node.connect_style.borrow() { ConnectStyle::BestBlockFirst|ConnectStyle::BestBlockFirstSkippingBlocks => { node.chain_monitor.chain_monitor.best_block_updated(&block.header, height); + call_claimable_balances(node); node.chain_monitor.chain_monitor.transactions_confirmed(&block.header, &txdata, height); node.node.best_block_updated(&block.header, height); node.node.transactions_confirmed(&block.header, &txdata, height); }, ConnectStyle::TransactionsFirst|ConnectStyle::TransactionsFirstSkippingBlocks => { node.chain_monitor.chain_monitor.transactions_confirmed(&block.header, &txdata, height); + call_claimable_balances(node); node.chain_monitor.chain_monitor.best_block_updated(&block.header, height); node.node.transactions_confirmed(&block.header, &txdata, height); node.node.best_block_updated(&block.header, height); @@ -146,11 +156,13 @@ fn do_connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: &Block, s } } } + call_claimable_balances(node); node.node.test_process_background_events(); node.blocks.lock().unwrap().push((block.header, height)); } pub fn disconnect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, count: u32) { + call_claimable_balances(node); for i in 0..count { let orig_header = node.blocks.lock().unwrap().pop().unwrap(); assert!(orig_header.1 > 0); // Cannot disconnect genesis @@ -172,6 +184,7 @@ pub fn disconnect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, count: u32) node.node.best_block_updated(&prev_header.0, prev_header.1); }, } + call_claimable_balances(node); } } diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 2435c3b7a..6557295b5 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -423,7 +423,9 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) { check_spends!(bs_htlc_claim_txn[0], as_commitment_tx); expect_payment_forwarded!(nodes[1], None, false); - mine_transaction(&nodes[0], &as_commitment_tx); + if !confirm_before_reload { + mine_transaction(&nodes[0], &as_commitment_tx); + } mine_transaction(&nodes[0], &bs_htlc_claim_txn[0]); expect_payment_sent!(nodes[0], payment_preimage_1); connect_blocks(&nodes[0], TEST_FINAL_CLTV*4 + 20); @@ -515,6 +517,10 @@ fn do_test_dup_htlc_onchain_fails_on_reload(persist_manager_post_event: bool, co check_added_monitors!(nodes[1], 1); check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed); let claim_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0); + assert_eq!(claim_txn.len(), 3); + check_spends!(claim_txn[0], node_txn[1]); + check_spends!(claim_txn[1], funding_tx); + check_spends!(claim_txn[2], claim_txn[1]); header.prev_blockhash = nodes[0].best_block_hash(); connect_block(&nodes[0], &Block { header, txdata: vec![node_txn[1].clone()]}); @@ -524,7 +530,7 @@ fn do_test_dup_htlc_onchain_fails_on_reload(persist_manager_post_event: bool, co } header.prev_blockhash = nodes[0].best_block_hash(); - let claim_block = Block { header, txdata: if payment_timeout { timeout_txn } else { claim_txn } }; + let claim_block = Block { header, txdata: if payment_timeout { timeout_txn } else { vec![claim_txn[0].clone()] } }; if payment_timeout { assert!(confirm_commitment_tx); // Otherwise we're spending below our CSV! diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs index fe8fd6827..6c39efb87 100644 --- a/lightning/src/ln/reorg_tests.rs +++ b/lightning/src/ln/reorg_tests.rs @@ -98,7 +98,7 @@ fn do_test_onchain_htlc_reorg(local_commitment: bool, claim: bool) { vec![node_1_commitment_txn[0].clone(), node_2_commitment_txn[0].clone()] } else { // Broadcast node 2 commitment txn - let node_2_commitment_txn = get_local_commitment_txn!(nodes[2], chan_2.2); + let mut node_2_commitment_txn = get_local_commitment_txn!(nodes[2], chan_2.2); assert_eq!(node_2_commitment_txn.len(), 2); // 1 local commitment tx, 1 Received HTLC-Claim assert_eq!(node_2_commitment_txn[0].output.len(), 2); // to-remote and Received HTLC (to-self is dust) check_spends!(node_2_commitment_txn[0], chan_2.3); @@ -113,12 +113,10 @@ fn do_test_onchain_htlc_reorg(local_commitment: bool, claim: bool) { check_spends!(node_1_commitment_txn[0], chan_2.3); check_spends!(node_1_commitment_txn[1], node_2_commitment_txn[0]); - // Confirm node 2's commitment txn (and node 1's HTLC-Timeout) on node 1 - header.prev_blockhash = nodes[1].best_block_hash(); - let block = Block { header, txdata: vec![node_2_commitment_txn[0].clone(), node_1_commitment_txn[1].clone()] }; - connect_block(&nodes[1], &block); + // Confirm node 1's HTLC-Timeout on node 1 + mine_transaction(&nodes[1], &node_1_commitment_txn[1]); // ...but return node 2's commitment tx (and claim) in case claim is set and we're preparing to reorg - node_2_commitment_txn + vec![node_2_commitment_txn.pop().unwrap()] }; check_added_monitors!(nodes[1], 1); check_closed_broadcast!(nodes[1], true); // We should get a BroadcastChannelUpdate (and *only* a BroadcstChannelUpdate) -- 2.39.5