From: Valentine Wallace Date: Thu, 5 Nov 2020 21:37:24 +0000 (-0500) Subject: Claim HTLC output on-chain if preimage is recv'd after force-close X-Git-Tag: v0.0.12~6^2 X-Git-Url: http://git.bitcoin.ninja/index.cgi?p=rust-lightning;a=commitdiff_plain;h=6f1a0bf0e4088a1935bfcece06f479c26f825ee1 Claim HTLC output on-chain if preimage is recv'd after force-close If we receive a preimage for an outgoing HTLC that solves an output on a backwards force-closed channel, we need to claim the output on-chain. Note that this commit also gets rid of the channel monitor redundantly setting `self.counterparty_payment_script` in `check_spend_counterparty_transaction`. Co-authored-by: Antoine Riard Co-authored-by: Valentine Wallace --- diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 425baedd..bb8d8c32 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1165,6 +1165,41 @@ impl ChannelMonitor { L::Target: Logger, { self.payment_preimages.insert(payment_hash.clone(), payment_preimage.clone()); + + // If the channel is force closed, try to claim the output from this preimage. + // First check if a counterparty commitment transaction has been broadcasted: + macro_rules! claim_htlcs { + ($commitment_number: expr, $txid: expr) => { + let htlc_claim_reqs = self.get_counterparty_htlc_output_claim_reqs($commitment_number, $txid, None); + self.onchain_tx_handler.update_claims_view(&Vec::new(), htlc_claim_reqs, None, broadcaster, fee_estimator, logger); + } + } + if let Some(txid) = self.current_counterparty_commitment_txid { + if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) { + claim_htlcs!(*commitment_number, txid); + return; + } + } + if let Some(txid) = self.prev_counterparty_commitment_txid { + if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) { + claim_htlcs!(*commitment_number, txid); + return; + } + } + + // Then if a holder commitment transaction has been seen on-chain, broadcast transactions + // claiming the HTLC output from each of the holder commitment transactions. + // Note that we can't just use `self.holder_tx_signed`, because that only covers the case where + // *we* sign a holder commitment transaction, not when e.g. a watchtower broadcasts one of our + // holder commitment transactions. + if self.broadcasted_holder_revokable_script.is_some() { + let (claim_reqs, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx); + self.onchain_tx_handler.update_claims_view(&Vec::new(), claim_reqs, None, broadcaster, fee_estimator, logger); + if let Some(ref tx) = self.prev_holder_signed_commitment_tx { + let (claim_reqs, _) = self.get_broadcasted_holder_claims(&tx); + self.onchain_tx_handler.update_claims_view(&Vec::new(), claim_reqs, None, broadcaster, fee_estimator, logger); + } + } } pub(crate) fn broadcast_latest_holder_commitment_txn(&mut self, broadcaster: &B, logger: &L) @@ -1463,39 +1498,55 @@ impl ChannelMonitor { check_htlc_fails!(txid, "previous", 'prev_loop); } + let htlc_claim_reqs = self.get_counterparty_htlc_output_claim_reqs(commitment_number, commitment_txid, Some(tx)); + for req in htlc_claim_reqs { + claimable_outpoints.push(req); + } + + } + (claimable_outpoints, (commitment_txid, watch_outputs)) + } + + fn get_counterparty_htlc_output_claim_reqs(&self, commitment_number: u64, commitment_txid: Txid, tx: Option<&Transaction>) -> Vec { + let mut claims = Vec::new(); + if let Some(htlc_outputs) = self.counterparty_claimable_outpoints.get(&commitment_txid) { if let Some(revocation_points) = self.their_cur_revocation_points { let revocation_point_option = + // If the counterparty commitment tx is the latest valid state, use their latest + // per-commitment point if revocation_points.0 == commitment_number { Some(&revocation_points.1) } else if let Some(point) = revocation_points.2.as_ref() { + // If counterparty commitment tx is the state previous to the latest valid state, use + // their previous per-commitment point (non-atomicity of revocation means it's valid for + // them to temporarily have two valid commitment txns from our viewpoint) if revocation_points.0 == commitment_number + 1 { Some(point) } else { None } } else { None }; if let Some(revocation_point) = revocation_point_option { - self.counterparty_payment_script = { - // Note that the Network here is ignored as we immediately drop the address for the - // script_pubkey version - let payment_hash160 = WPubkeyHash::hash(&self.keys.pubkeys().payment_point.serialize()); - Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script() - }; - - // Then, try to find htlc outputs - for (_, &(ref htlc, _)) in per_commitment_data.iter().enumerate() { + for (_, &(ref htlc, _)) in htlc_outputs.iter().enumerate() { if let Some(transaction_output_index) = htlc.transaction_output_index { - if transaction_output_index as usize >= tx.output.len() || - tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 { - return (claimable_outpoints, (commitment_txid, watch_outputs)); // Corrupted per_commitment_data, fuck this user + if let Some(transaction) = tx { + if transaction_output_index as usize >= transaction.output.len() || + transaction.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 { + return claims; // Corrupted per_commitment_data, fuck this user + } } - let preimage = if htlc.offered { if let Some(p) = self.payment_preimages.get(&htlc.payment_hash) { Some(*p) } else { None } } else { None }; + let preimage = + if htlc.offered { + if let Some(p) = self.payment_preimages.get(&htlc.payment_hash) { + Some(*p) + } else { None } + } else { None }; let aggregable = if !htlc.offered { false } else { true }; if preimage.is_some() || !htlc.offered { let witness_data = InputMaterial::CounterpartyHTLC { per_commitment_point: *revocation_point, counterparty_delayed_payment_base_key: self.counterparty_tx_cache.counterparty_delayed_payment_base_key, counterparty_htlc_base_key: self.counterparty_tx_cache.counterparty_htlc_base_key, preimage, htlc: htlc.clone() }; - claimable_outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data }); + claims.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data }); } } } } } } - (claimable_outpoints, (commitment_txid, watch_outputs)) + claims } /// Attempts to claim a counterparty HTLC-Success/HTLC-Timeout's outputs using the revocation key @@ -1816,7 +1867,7 @@ impl ChannelMonitor { } } - self.onchain_tx_handler.block_connected(&txn_matched, claimable_outpoints, height, &*broadcaster, &*fee_estimator, &*logger); + self.onchain_tx_handler.update_claims_view(&txn_matched, claimable_outpoints, Some(height), &&*broadcaster, &&*fee_estimator, &&*logger); self.last_block_hash = block_hash; // Determine new outputs to watch by comparing against previously known outputs to watch, diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index d243ab28..b3e5697c 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -8503,3 +8503,187 @@ fn test_htlc_no_detection() { connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1, 201, true, header_201.block_hash()); expect_payment_failed!(nodes[0], our_payment_hash, true); } + +fn do_test_onchain_htlc_settlement_after_close(broadcast_alice: bool, go_onchain_before_fulfill: bool) { + // If we route an HTLC, then learn the HTLC's preimage after the upstream channel has been + // force-closed, we must claim that HTLC on-chain. (Given an HTLC forwarded from Alice --> Bob --> + // Carol, Alice would be the upstream node, and Carol the downstream.) + // + // Steps of the test: + // 1) Alice sends a HTLC to Carol through Bob. + // 2) Carol doesn't settle the HTLC. + // 3) If broadcast_alice is true, Alice force-closes her channel with Bob. Else Bob force closes. + // Steps 4 and 5 may be reordered depending on go_onchain_before_fulfill. + // 4) Bob sees the Alice's commitment on his chain or vice versa. An offered output is present + // but can't be claimed as Bob doesn't have yet knowledge of the preimage. + // 5) Carol release the preimage to Bob off-chain. + // 6) Bob claims the offered output on the broadcasted commitment. + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + // Create some initial channels + let chan_ab = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 10001, InitFeatures::known(), InitFeatures::known()); + create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 100000, 10001, InitFeatures::known(), InitFeatures::known()); + + // Steps (1) and (2): + // Send an HTLC Alice --> Bob --> Carol, but Carol doesn't settle the HTLC back. + let (payment_preimage, _payment_hash) = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[2]), 3_000_000); + + // Check that Alice's commitment transaction now contains an output for this HTLC. + let alice_txn = get_local_commitment_txn!(nodes[0], chan_ab.2); + check_spends!(alice_txn[0], chan_ab.3); + assert_eq!(alice_txn[0].output.len(), 2); + check_spends!(alice_txn[1], alice_txn[0]); // 2nd transaction is a non-final HTLC-timeout + assert_eq!(alice_txn[1].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); + assert_eq!(alice_txn.len(), 2); + + // Steps (3) and (4): + // If `go_onchain_before_fufill`, broadcast the relevant commitment transaction and check that Bob + // responds by (1) broadcasting a channel update and (2) adding a new ChannelMonitor. + let mut force_closing_node = 0; // Alice force-closes + if !broadcast_alice { force_closing_node = 1; } // Bob force-closes + nodes[force_closing_node].node.force_close_channel(&chan_ab.2); + check_closed_broadcast!(nodes[force_closing_node], false); + check_added_monitors!(nodes[force_closing_node], 1); + if go_onchain_before_fulfill { + let txn_to_broadcast = match broadcast_alice { + true => alice_txn.clone(), + false => get_local_commitment_txn!(nodes[1], chan_ab.2) + }; + let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42}; + connect_block(&nodes[1], &Block { header, txdata: vec![txn_to_broadcast[0].clone()]}, 1); + let mut bob_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); + if broadcast_alice { + check_closed_broadcast!(nodes[1], false); + check_added_monitors!(nodes[1], 1); + } + assert_eq!(bob_txn.len(), 1); + check_spends!(bob_txn[0], chan_ab.3); + } + + // Step (5): + // Carol then claims the funds and sends an update_fulfill message to Bob, and they go through the + // process of removing the HTLC from their commitment transactions. + assert!(nodes[2].node.claim_funds(payment_preimage, &None, 3_000_000)); + check_added_monitors!(nodes[2], 1); + let carol_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id()); + assert!(carol_updates.update_add_htlcs.is_empty()); + assert!(carol_updates.update_fail_htlcs.is_empty()); + assert!(carol_updates.update_fail_malformed_htlcs.is_empty()); + assert!(carol_updates.update_fee.is_none()); + assert_eq!(carol_updates.update_fulfill_htlcs.len(), 1); + + nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &carol_updates.update_fulfill_htlcs[0]); + // If Alice broadcasted but Bob doesn't know yet, here he prepares to tell her about the preimage. + if !go_onchain_before_fulfill && broadcast_alice { + let events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match events[0] { + MessageSendEvent::UpdateHTLCs { ref node_id, .. } => { + assert_eq!(*node_id, nodes[0].node.get_our_node_id()); + }, + _ => panic!("Unexpected event"), + }; + } + nodes[1].node.handle_commitment_signed(&nodes[2].node.get_our_node_id(), &carol_updates.commitment_signed); + // One monitor update for the preimage to update the Bob<->Alice channel, one monitor update + // Carol<->Bob's updated commitment transaction info. + check_added_monitors!(nodes[1], 2); + + let events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 2); + let bob_revocation = match events[0] { + MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => { + assert_eq!(*node_id, nodes[2].node.get_our_node_id()); + (*msg).clone() + }, + _ => panic!("Unexpected event"), + }; + let bob_updates = match events[1] { + MessageSendEvent::UpdateHTLCs { ref node_id, ref updates } => { + assert_eq!(*node_id, nodes[2].node.get_our_node_id()); + (*updates).clone() + }, + _ => panic!("Unexpected event"), + }; + + nodes[2].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bob_revocation); + check_added_monitors!(nodes[2], 1); + nodes[2].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bob_updates.commitment_signed); + check_added_monitors!(nodes[2], 1); + + let events = nodes[2].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + let carol_revocation = match events[0] { + MessageSendEvent::SendRevokeAndACK { ref node_id, ref msg } => { + assert_eq!(*node_id, nodes[1].node.get_our_node_id()); + (*msg).clone() + }, + _ => panic!("Unexpected event"), + }; + nodes[1].node.handle_revoke_and_ack(&nodes[2].node.get_our_node_id(), &carol_revocation); + check_added_monitors!(nodes[1], 1); + + // If this test requires the force-closed channel to not be on-chain until after the fulfill, + // here's where we put said channel's commitment tx on-chain. + let mut txn_to_broadcast = alice_txn.clone(); + if !broadcast_alice { txn_to_broadcast = get_local_commitment_txn!(nodes[1], chan_ab.2); } + if !go_onchain_before_fulfill { + let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42}; + connect_block(&nodes[1], &Block { header, txdata: vec![txn_to_broadcast[0].clone()]}, 1); + // If Bob was the one to force-close, he will have already passed these checks earlier. + if broadcast_alice { + check_closed_broadcast!(nodes[1], false); + check_added_monitors!(nodes[1], 1); + } + let mut bob_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); + if broadcast_alice { + // In `connect_block()`, the ChainMonitor and ChannelManager are separately notified about a + // new block being connected. The ChannelManager being notified triggers a monitor update, + // which triggers broadcasting our commitment tx and an HTLC-claiming tx. The ChainMonitor + // being notified triggers the HTLC-claiming tx redundantly, resulting in 3 total txs being + // broadcasted. + assert_eq!(bob_txn.len(), 3); + check_spends!(bob_txn[1], chan_ab.3); + } else { + assert_eq!(bob_txn.len(), 2); + check_spends!(bob_txn[0], chan_ab.3); + } + } + + // Step (6): + // Finally, check that Bob broadcasted a preimage-claiming transaction for the HTLC output on the + // broadcasted commitment transaction. + { + let bob_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); + if go_onchain_before_fulfill { + // Bob should now have an extra broadcasted tx, for the preimage-claiming transaction. + assert_eq!(bob_txn.len(), 2); + } + let script_weight = match broadcast_alice { + true => OFFERED_HTLC_SCRIPT_WEIGHT, + false => ACCEPTED_HTLC_SCRIPT_WEIGHT + }; + // If Alice force-closed and Bob didn't receive her commitment transaction until after he + // received Carol's fulfill, he broadcasts the HTLC-output-claiming transaction first. Else if + // Bob force closed or if he found out about Alice's commitment tx before receiving Carol's + // fulfill, then he broadcasts the HTLC-output-claiming transaction second. + if broadcast_alice && !go_onchain_before_fulfill { + check_spends!(bob_txn[0], txn_to_broadcast[0]); + assert_eq!(bob_txn[0].input[0].witness.last().unwrap().len(), script_weight); + } else { + check_spends!(bob_txn[1], txn_to_broadcast[0]); + assert_eq!(bob_txn[1].input[0].witness.last().unwrap().len(), script_weight); + } + } +} + +#[test] +fn test_onchain_htlc_settlement_after_close() { + do_test_onchain_htlc_settlement_after_close(true, true); + do_test_onchain_htlc_settlement_after_close(false, true); // Technically redundant, but may as well + do_test_onchain_htlc_settlement_after_close(true, false); + do_test_onchain_htlc_settlement_after_close(false, false); +} diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs index 2f156563..3484d898 100644 --- a/lightning/src/ln/onchaintx.rs +++ b/lightning/src/ln/onchaintx.rs @@ -282,6 +282,8 @@ pub struct OnchainTxHandler { onchain_events_waiting_threshold_conf: HashMap>, + latest_height: u32, + secp_ctx: Secp256k1, } @@ -328,6 +330,7 @@ impl OnchainTxHandler { } } } + self.latest_height.write(writer)?; Ok(()) } } @@ -387,6 +390,7 @@ impl Readable for OnchainTxHandler Readable for OnchainTxHandler OnchainTxHandler { pending_claim_requests: HashMap::new(), claimable_outpoints: HashMap::new(), onchain_events_waiting_threshold_conf: HashMap::new(), + latest_height: 0, secp_ctx: Secp256k1::new(), } @@ -471,7 +477,7 @@ impl OnchainTxHandler { /// Lightning security model (i.e being able to redeem/timeout HTLC or penalize coutnerparty onchain) lays on the assumption of claim transactions getting confirmed before timelock expiration /// (CSV or CLTV following cases). In case of high-fee spikes, claim tx may stuck in the mempool, so you need to bump its feerate quickly using Replace-By-Fee or Child-Pay-For-Parent. - fn generate_claim_tx(&mut self, height: u32, cached_claim_datas: &ClaimTxBumpMaterial, fee_estimator: F, logger: L) -> Option<(Option, u32, Transaction)> + fn generate_claim_tx(&mut self, height: u32, cached_claim_datas: &ClaimTxBumpMaterial, fee_estimator: &F, logger: &L) -> Option<(Option, u32, Transaction)> where F::Target: FeeEstimator, L::Target: Logger, { @@ -657,12 +663,20 @@ impl OnchainTxHandler { None } - pub(crate) fn block_connected(&mut self, txn_matched: &[&Transaction], claimable_outpoints: Vec, height: u32, broadcaster: B, fee_estimator: F, logger: L) + /// Upon channelmonitor.block_connected(..) or upon provision of a preimage on the forward link + /// for this channel, provide new relevant on-chain transactions and/or new claim requests. + /// Formerly this was named `block_connected`, but it is now also used for claiming an HTLC output + /// if we receive a preimage after force-close. + pub(crate) fn update_claims_view(&mut self, txn_matched: &[&Transaction], claimable_outpoints: Vec, latest_height: Option, broadcaster: &B, fee_estimator: &F, logger: &L) where B::Target: BroadcasterInterface, F::Target: FeeEstimator, L::Target: Logger, { - log_trace!(logger, "Block at height {} connected with {} claim requests", height, claimable_outpoints.len()); + let height = match latest_height { + Some(h) => h, + None => self.latest_height, + }; + log_trace!(logger, "Updating claims view at height {} with {} matched transactions and {} claim requests", height, txn_matched.len(), claimable_outpoints.len()); let mut new_claims = Vec::new(); let mut aggregated_claim = HashMap::new(); let mut aggregated_soonest = ::std::u32::MAX; @@ -855,7 +869,7 @@ impl OnchainTxHandler { } } for (_, claim_material) in bump_candidates.iter_mut() { - if let Some((new_timer, new_feerate, bump_tx)) = self.generate_claim_tx(height, &claim_material, &*fee_estimator, &*logger) { + if let Some((new_timer, new_feerate, bump_tx)) = self.generate_claim_tx(height, &claim_material, &&*fee_estimator, &&*logger) { claim_material.height_timer = new_timer; claim_material.feerate_previous = new_feerate; broadcaster.broadcast_transaction(&bump_tx);