From ac4f174b73b0231d36e3326a938df32e0d7645ca Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 8 Jan 2024 12:03:29 -0800 Subject: [PATCH] Allow holder commitment and HTLC signature requests to fail As part of the ongoing async signer work, our holder signatures must also be capable of being obtained asynchronously. We rely on our existing `ChainMonitor::rebroadcast_pending_claims` method to retry pending onchain claims by re-signing and rebroadcasting transactions. Unfortunately, we cannot retry said claims without them being registered first, so if we're not able to obtain the signature synchronously, we must return the transaction as unsigned and ensure it is not broadcast. --- lightning/src/chain/channelmonitor.rs | 50 ++++---- lightning/src/chain/onchaintx.rs | 61 ++++++---- lightning/src/chain/package.rs | 4 +- lightning/src/ln/async_signer_tests.rs | 132 ++++++++++++++++++++-- lightning/src/ln/functional_test_utils.rs | 32 ++++-- lightning/src/util/test_channel_signer.rs | 4 +- lightning/src/util/test_utils.rs | 12 +- 7 files changed, 226 insertions(+), 69 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index c81a48b78ac..ce27c8baba9 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1396,8 +1396,8 @@ impl ChannelMonitor { /// Loads the funding txo and outputs to watch into the given `chain::Filter` by repeatedly /// calling `chain::Filter::register_output` and `chain::Filter::register_tx` until all outputs /// have been registered. - pub fn load_outputs_to_watch(&self, filter: &F, logger: &L) - where + pub fn load_outputs_to_watch(&self, filter: &F, logger: &L) + where F::Target: chain::Filter, L::Target: Logger, { let lock = self.inner.lock().unwrap(); @@ -1542,21 +1542,16 @@ impl ChannelMonitor { self.inner.lock().unwrap().counterparty_node_id } - /// Used by [`ChannelManager`] deserialization to broadcast the latest holder state if its copy - /// of the channel state was out-of-date. - /// - /// You may also use this to broadcast the latest local commitment transaction, either because + /// You may use this to broadcast the latest local commitment transaction, either because /// a monitor update failed or because we've fallen behind (i.e. we've received proof that our /// counterparty side knows a revocation secret we gave them that they shouldn't know). /// - /// Broadcasting these transactions in the second case is UNSAFE, as they allow counterparty + /// Broadcasting these transactions in this manner is UNSAFE, as they allow counterparty /// side to punish you. Nevertheless you may want to broadcast them if counterparty doesn't /// close channel with their commitment transaction after a substantial amount of time. Best /// may be to contact the other node operator out-of-band to coordinate other options available /// to you. - /// - /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager - pub fn get_latest_holder_commitment_txn(&self, logger: &L) -> Vec + pub fn get_latest_holder_commitment_txn(&self, logger: &L) -> Result, ()> where L::Target: Logger { let mut inner = self.inner.lock().unwrap(); let logger = WithChannelMonitor::from_impl(logger, &*inner); @@ -1789,6 +1784,12 @@ impl ChannelMonitor { pub fn set_counterparty_payment_script(&self, script: ScriptBuf) { self.inner.lock().unwrap().counterparty_payment_script = script; } + + #[cfg(test)] + pub fn do_signer_call ()>(&self, mut f: F) { + let inner = self.inner.lock().unwrap(); + f(&inner.onchain_tx_handler.signer); + } } impl ChannelMonitorImpl { @@ -3462,16 +3463,19 @@ impl ChannelMonitorImpl { fn get_latest_holder_commitment_txn( &mut self, logger: &WithChannelMonitor, - ) -> Vec where L::Target: Logger { + ) -> Result, ()> where L::Target: Logger { log_debug!(logger, "Getting signed latest holder commitment transaction!"); self.holder_tx_signed = true; - let commitment_tx = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript); + let commitment_tx = self.onchain_tx_handler.get_maybe_signed_holder_tx(&self.funding_redeemscript); + if commitment_tx.input.iter().any(|input| input.witness.is_empty()) { + return Err(()); + } let txid = commitment_tx.txid(); let mut holder_transactions = vec![commitment_tx]; // When anchor outputs are present, the HTLC transactions are only valid once the commitment // transaction confirms. if self.onchain_tx_handler.channel_type_features().supports_anchors_zero_fee_htlc_tx() { - return holder_transactions; + return Ok(holder_transactions); } for htlc in self.current_holder_commitment_tx.htlc_outputs.iter() { if let Some(vout) = htlc.0.transaction_output_index { @@ -3488,15 +3492,18 @@ impl ChannelMonitorImpl { // confirmed in the next block. continue; } else { None }; - if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx( - &::bitcoin::OutPoint { txid, vout }, &preimage) { - holder_transactions.push(htlc_tx); + if let Some(htlc_tx) = self.onchain_tx_handler.get_maybe_signed_htlc_tx( + &::bitcoin::OutPoint { txid, vout }, &preimage + ) { + if !htlc_tx.input.iter().any(|input| input.witness.is_empty()) { + holder_transactions.push(htlc_tx); + } } } } // We throw away the generated waiting_first_conf data as we aren't (yet) confirmed and we don't actually know what the caller wants to do. // The data will be re-generated and tracked in check_spend_holder_transaction if we get a confirmation. - holder_transactions + Ok(holder_transactions) } #[cfg(any(test,feature = "unsafe_revoked_tx_signing"))] @@ -3521,9 +3528,12 @@ impl ChannelMonitorImpl { continue; } } else { None }; - if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx( - &::bitcoin::OutPoint { txid, vout }, &preimage) { - holder_transactions.push(htlc_tx); + if let Some(htlc_tx) = self.onchain_tx_handler.get_maybe_signed_htlc_tx( + &::bitcoin::OutPoint { txid, vout }, &preimage + ) { + if !htlc_tx.input.iter().any(|input| input.witness.is_empty()) { + holder_transactions.push(htlc_tx); + } } } } diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 59c98f05ebc..ebe4458b4ce 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -497,9 +497,13 @@ impl OnchainTxHandler } match claim { OnchainClaim::Tx(tx) => { - let log_start = if bumped_feerate { "Broadcasting RBF-bumped" } else { "Rebroadcasting" }; - log_info!(logger, "{} onchain {}", log_start, log_tx!(tx)); - broadcaster.broadcast_transactions(&[&tx]); + if tx.input.iter().any(|input| input.witness.is_empty()) { + log_info!(logger, "Waiting for signature of unsigned onchain transaction {}", tx.txid()); + } else { + let log_start = if bumped_feerate { "Broadcasting RBF-bumped" } else { "Rebroadcasting" }; + log_info!(logger, "{} onchain {}", log_start, log_tx!(tx)); + broadcaster.broadcast_transactions(&[&tx]); + } }, OnchainClaim::Event(event) => { let log_start = if bumped_feerate { "Yielding fee-bumped" } else { "Replaying" }; @@ -636,8 +640,8 @@ impl OnchainTxHandler let commitment_tx_feerate_sat_per_1000_weight = compute_feerate_sat_per_1000_weight(fee_sat, tx.weight().to_wu()); if commitment_tx_feerate_sat_per_1000_weight >= package_target_feerate_sat_per_1000_weight { - log_debug!(logger, "Pre-signed {} already has feerate {} sat/kW above required {} sat/kW", - log_tx!(tx), commitment_tx_feerate_sat_per_1000_weight, + log_debug!(logger, "Pre-signed commitment {} already has feerate {} sat/kW above required {} sat/kW", + tx.txid(), commitment_tx_feerate_sat_per_1000_weight, package_target_feerate_sat_per_1000_weight); return Some((new_timer, 0, OnchainClaim::Tx(tx.clone()))); } @@ -776,8 +780,12 @@ impl OnchainTxHandler // `OnchainClaim`. let claim_id = match claim { OnchainClaim::Tx(tx) => { - log_info!(logger, "Broadcasting onchain {}", log_tx!(tx)); - broadcaster.broadcast_transactions(&[&tx]); + if tx.input.iter().any(|input| input.witness.is_empty()) { + log_info!(logger, "Waiting for signature of unsigned onchain transaction {}", tx.txid()); + } else { + log_info!(logger, "Broadcasting onchain {}", log_tx!(tx)); + broadcaster.broadcast_transactions(&[&tx]); + } ClaimId(tx.txid().to_byte_array()) }, OnchainClaim::Event(claim_event) => { @@ -969,8 +977,13 @@ impl OnchainTxHandler ) { match bump_claim { OnchainClaim::Tx(bump_tx) => { - log_info!(logger, "Broadcasting RBF-bumped onchain {}", log_tx!(bump_tx)); - broadcaster.broadcast_transactions(&[&bump_tx]); + if bump_tx.input.iter().any(|input| input.witness.is_empty()) { + log_info!(logger, "Waiting for signature of RBF-bumped unsigned onchain transaction {}", + bump_tx.txid()); + } else { + log_info!(logger, "Broadcasting RBF-bumped onchain {}", log_tx!(bump_tx)); + broadcaster.broadcast_transactions(&[&bump_tx]); + } }, OnchainClaim::Event(claim_event) => { log_info!(logger, "Yielding RBF-bumped onchain event to spend inputs {:?}", request.outpoints()); @@ -1052,8 +1065,12 @@ impl OnchainTxHandler request.set_feerate(new_feerate); match bump_claim { OnchainClaim::Tx(bump_tx) => { - log_info!(logger, "Broadcasting onchain {}", log_tx!(bump_tx)); - broadcaster.broadcast_transactions(&[&bump_tx]); + if bump_tx.input.iter().any(|input| input.witness.is_empty()) { + log_info!(logger, "Waiting for signature of unsigned onchain transaction {}", bump_tx.txid()); + } else { + log_info!(logger, "Broadcasting onchain {}", log_tx!(bump_tx)); + broadcaster.broadcast_transactions(&[&bump_tx]); + } }, OnchainClaim::Event(claim_event) => { log_info!(logger, "Yielding onchain event after reorg to spend inputs {:?}", request.outpoints()); @@ -1106,13 +1123,10 @@ impl OnchainTxHandler &self.holder_commitment.trust().built_transaction().transaction } - //TODO: getting lastest holder transactions should be infallible and result in us "force-closing the channel", but we may - // have empty holder commitment transaction if a ChannelMonitor is asked to force-close just after OutboundV1Channel::get_funding_created, - // before providing a initial commitment transaction. For outbound channel, init ChannelMonitor at Channel::funding_signed, there is nothing - // to monitor before. - pub(crate) fn get_fully_signed_holder_tx(&mut self, funding_redeemscript: &Script) -> Transaction { - let sig = self.signer.sign_holder_commitment(&self.holder_commitment, &self.secp_ctx).expect("signing holder commitment"); - self.holder_commitment.add_holder_sig(funding_redeemscript, sig) + pub(crate) fn get_maybe_signed_holder_tx(&mut self, funding_redeemscript: &Script) -> Transaction { + self.signer.sign_holder_commitment(&self.holder_commitment, &self.secp_ctx) + .map(|sig| self.holder_commitment.add_holder_sig(funding_redeemscript, sig)) + .unwrap_or_else(|_| self.get_unsigned_holder_commitment_tx().clone()) } #[cfg(any(test, feature="unsafe_revoked_tx_signing"))] @@ -1121,7 +1135,7 @@ impl OnchainTxHandler self.holder_commitment.add_holder_sig(funding_redeemscript, sig) } - pub(crate) fn get_fully_signed_htlc_tx(&mut self, outp: &::bitcoin::OutPoint, preimage: &Option) -> Option { + pub(crate) fn get_maybe_signed_htlc_tx(&mut self, outp: &::bitcoin::OutPoint, preimage: &Option) -> Option { let get_signed_htlc_tx = |holder_commitment: &HolderCommitmentTransaction| { let trusted_tx = holder_commitment.trust(); if trusted_tx.txid() != outp.txid { @@ -1149,10 +1163,11 @@ impl OnchainTxHandler preimage: preimage.clone(), counterparty_sig: counterparty_htlc_sig.clone(), }; - let htlc_sig = self.signer.sign_holder_htlc_transaction(&htlc_tx, 0, &htlc_descriptor, &self.secp_ctx).unwrap(); - htlc_tx.input[0].witness = trusted_tx.build_htlc_input_witness( - htlc_idx, &counterparty_htlc_sig, &htlc_sig, preimage, - ); + if let Ok(htlc_sig) = self.signer.sign_holder_htlc_transaction(&htlc_tx, 0, &htlc_descriptor, &self.secp_ctx) { + htlc_tx.input[0].witness = trusted_tx.build_htlc_input_witness( + htlc_idx, &counterparty_htlc_sig, &htlc_sig, preimage, + ); + } Some(htlc_tx) }; diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs index efc32bf7d40..7c33f47d91e 100644 --- a/lightning/src/chain/package.rs +++ b/lightning/src/chain/package.rs @@ -637,10 +637,10 @@ impl PackageSolvingData { match self { PackageSolvingData::HolderHTLCOutput(ref outp) => { debug_assert!(!outp.channel_type_features.supports_anchors_zero_fee_htlc_tx()); - return onchain_handler.get_fully_signed_htlc_tx(outpoint, &outp.preimage); + return onchain_handler.get_maybe_signed_htlc_tx(outpoint, &outp.preimage); } PackageSolvingData::HolderFundingOutput(ref outp) => { - return Some(onchain_handler.get_fully_signed_holder_tx(&outp.funding_redeemscript)); + Some(onchain_handler.get_maybe_signed_holder_tx(&outp.funding_redeemscript)) } _ => { panic!("API Error!"); } } diff --git a/lightning/src/ln/async_signer_tests.rs b/lightning/src/ln/async_signer_tests.rs index 0f51bea0d36..b9ba9480667 100644 --- a/lightning/src/ln/async_signer_tests.rs +++ b/lightning/src/ln/async_signer_tests.rs @@ -10,7 +10,12 @@ //! Tests for asynchronous signing. These tests verify that the channel state machine behaves //! properly with a signer implementation that asynchronously derives signatures. -use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider}; +use bitcoin::{Transaction, TxOut, TxIn, Amount}; +use bitcoin::blockdata::locktime::absolute::LockTime; + +use crate::chain::channelmonitor::LATENCY_GRACE_PERIOD_BLOCKS; +use crate::events::bump_transaction::WalletSource; +use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, ClosureReason}; use crate::ln::functional_test_utils::*; use crate::ln::msgs::ChannelMessageHandler; use crate::ln::channelmanager::{PaymentId, RecipientOnionFields}; @@ -37,7 +42,7 @@ fn test_async_commitment_signature_for_funding_created() { // But! Let's make node[0]'s signer be unavailable: we should *not* broadcast a funding_created // message... let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42); - nodes[0].set_channel_signer_available(&nodes[1].node.get_our_node_id(), &temporary_channel_id, false); + nodes[0].set_channel_signer_available(&nodes[1].node.get_our_node_id(), &temporary_channel_id, None, false); nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap(); check_added_monitors(&nodes[0], 0); @@ -51,7 +56,7 @@ fn test_async_commitment_signature_for_funding_created() { channels[0].channel_id }; - nodes[0].set_channel_signer_available(&nodes[1].node.get_our_node_id(), &chan_id, true); + nodes[0].set_channel_signer_available(&nodes[1].node.get_our_node_id(), &chan_id, None, true); nodes[0].node.signer_unblocked(Some((nodes[1].node.get_our_node_id(), chan_id))); let mut funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); @@ -92,7 +97,7 @@ fn test_async_commitment_signature_for_funding_signed() { // Now let's make node[1]'s signer be unavailable while handling the `funding_created`. It should // *not* broadcast a `funding_signed`... - nodes[1].set_channel_signer_available(&nodes[0].node.get_our_node_id(), &temporary_channel_id, false); + nodes[1].set_channel_signer_available(&nodes[0].node.get_our_node_id(), &temporary_channel_id, None, false); nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created_msg); check_added_monitors(&nodes[1], 1); @@ -105,7 +110,7 @@ fn test_async_commitment_signature_for_funding_signed() { assert_eq!(channels.len(), 1, "expected one channel, not {}", channels.len()); channels[0].channel_id }; - nodes[1].set_channel_signer_available(&nodes[0].node.get_our_node_id(), &chan_id, true); + nodes[1].set_channel_signer_available(&nodes[0].node.get_our_node_id(), &chan_id, None, true); nodes[1].node.signer_unblocked(Some((nodes[0].node.get_our_node_id(), chan_id))); expect_channel_pending_event(&nodes[1], &nodes[0].node.get_our_node_id()); @@ -146,14 +151,14 @@ fn test_async_commitment_signature_for_commitment_signed() { // Mark dst's signer as unavailable and handle src's commitment_signed: while dst won't yet have a // `commitment_signed` of its own to offer, it should publish a `revoke_and_ack`. - dst.set_channel_signer_available(&src.node.get_our_node_id(), &chan_id, false); + dst.set_channel_signer_available(&src.node.get_our_node_id(), &chan_id, None, false); dst.node.handle_commitment_signed(&src.node.get_our_node_id(), &payment_event.commitment_msg); check_added_monitors(dst, 1); get_event_msg!(dst, MessageSendEvent::SendRevokeAndACK, src.node.get_our_node_id()); // Mark dst's signer as available and retry: we now expect to see dst's `commitment_signed`. - dst.set_channel_signer_available(&src.node.get_our_node_id(), &chan_id, true); + dst.set_channel_signer_available(&src.node.get_our_node_id(), &chan_id, None, true); dst.node.signer_unblocked(Some((src.node.get_our_node_id(), chan_id))); let events = dst.node.get_and_clear_pending_msg_events(); @@ -209,7 +214,7 @@ fn test_async_commitment_signature_for_funding_signed_0conf() { // Now let's make node[1]'s signer be unavailable while handling the `funding_created`. It should // *not* broadcast a `funding_signed`... - nodes[1].set_channel_signer_available(&nodes[0].node.get_our_node_id(), &temporary_channel_id, false); + nodes[1].set_channel_signer_available(&nodes[0].node.get_our_node_id(), &temporary_channel_id, None, false); nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created_msg); check_added_monitors(&nodes[1], 1); @@ -224,7 +229,7 @@ fn test_async_commitment_signature_for_funding_signed_0conf() { }; // At this point, we basically expect the channel to open like a normal zero-conf channel. - nodes[1].set_channel_signer_available(&nodes[0].node.get_our_node_id(), &chan_id, true); + nodes[1].set_channel_signer_available(&nodes[0].node.get_our_node_id(), &chan_id, None, true); nodes[1].node.signer_unblocked(Some((nodes[0].node.get_our_node_id(), chan_id))); let (funding_signed, channel_ready_1) = { @@ -293,7 +298,7 @@ fn test_async_commitment_signature_for_peer_disconnect() { // Mark dst's signer as unavailable and handle src's commitment_signed: while dst won't yet have a // `commitment_signed` of its own to offer, it should publish a `revoke_and_ack`. - dst.set_channel_signer_available(&src.node.get_our_node_id(), &chan_id, false); + dst.set_channel_signer_available(&src.node.get_our_node_id(), &chan_id, None, false); dst.node.handle_commitment_signed(&src.node.get_our_node_id(), &payment_event.commitment_msg); check_added_monitors(dst, 1); @@ -308,7 +313,7 @@ fn test_async_commitment_signature_for_peer_disconnect() { reconnect_nodes(reconnect_args); // Mark dst's signer as available and retry: we now expect to see dst's `commitment_signed`. - dst.set_channel_signer_available(&src.node.get_our_node_id(), &chan_id, true); + dst.set_channel_signer_available(&src.node.get_our_node_id(), &chan_id, None, true); dst.node.signer_unblocked(Some((src.node.get_our_node_id(), chan_id))); { @@ -321,3 +326,108 @@ fn test_async_commitment_signature_for_peer_disconnect() { }; } } + +fn do_test_async_holder_signatures(anchors: bool) { + // Ensures that we can obtain holder signatures for commitment and HTLC transactions + // asynchronously by allowing their retrieval to fail and retrying via + // `ChainMonitor::rebroadcast_pending_claims`. + let mut config = test_default_channel_config(); + if anchors { + config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + config.manually_accept_inbound_channels = true; + } + + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config), Some(config)]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let coinbase_tx = Transaction { + version: 2, + lock_time: LockTime::ZERO, + input: vec![TxIn { ..Default::default() }], + output: vec![ + TxOut { + value: Amount::ONE_BTC.to_sat(), + script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), + }, + ], + }; + if anchors { + *nodes[0].fee_estimator.sat_per_kw.lock().unwrap() *= 2; + nodes[0].wallet_source.add_utxo(bitcoin::OutPoint { txid: coinbase_tx.txid(), vout: 0 }, coinbase_tx.output[0].value); + } + + // Route an HTLC and set the signer as unavailable. + let (_, _, chan_id, funding_tx) = create_announced_chan_between_nodes(&nodes, 0, 1); + let funding_txo = crate::chain::transaction::OutPoint { txid: funding_tx.txid(), index: 0 }; + route_payment(&nodes[0], &[&nodes[1]], 1_000_000); + + nodes[0].set_channel_signer_available(&nodes[1].node.get_our_node_id(), &chan_id, Some(funding_txo), false); + + // We'll connect blocks until the sender has to go onchain to time out the HTLC. + connect_blocks(&nodes[0], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1); + + // No transaction should be broadcast since the signer is not available yet. + assert!(nodes[0].tx_broadcaster.txn_broadcast().is_empty()); + assert!(nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events().is_empty()); + + // Mark it as available now, we should see the signed commitment transaction. + nodes[0].set_channel_signer_available(&nodes[1].node.get_our_node_id(), &chan_id, Some(funding_txo), true); + nodes[0].chain_monitor.chain_monitor.rebroadcast_pending_claims(); + + let commitment_tx = { + let mut txn = nodes[0].tx_broadcaster.txn_broadcast(); + if anchors { + assert_eq!(txn.len(), 1); + check_spends!(txn[0], funding_tx); + txn.remove(0) + } else { + assert_eq!(txn.len(), 2); + if txn[0].input[0].previous_output.txid == funding_tx.txid() { + check_spends!(txn[0], funding_tx); + check_spends!(txn[1], txn[0]); + txn.remove(0) + } else { + check_spends!(txn[1], funding_tx); + check_spends!(txn[0], txn[1]); + txn.remove(1) + } + } + }; + + // Mark it as unavailable again to now test the HTLC transaction. We'll mine the commitment such + // that the HTLC transaction is retried. + nodes[0].set_channel_signer_available(&nodes[1].node.get_our_node_id(), &chan_id, Some(funding_txo), false); + mine_transaction(&nodes[0], &commitment_tx); + + check_added_monitors(&nodes[0], 1); + check_closed_broadcast(&nodes[0], 1, true); + check_closed_event(&nodes[0], 1, ClosureReason::CommitmentTxConfirmed, false, &[nodes[1].node.get_our_node_id()], 100_000); + + // No HTLC transaction should be broadcast as the signer is not available yet. + if anchors { + handle_bump_htlc_event(&nodes[0], 1); + } + assert!(nodes[0].tx_broadcaster.txn_broadcast().is_empty()); + + // Mark it as available now, we should see the signed HTLC transaction. + nodes[0].set_channel_signer_available(&nodes[1].node.get_our_node_id(), &chan_id, Some(funding_txo), true); + nodes[0].chain_monitor.chain_monitor.rebroadcast_pending_claims(); + + if anchors { + handle_bump_htlc_event(&nodes[0], 1); + } + { + let mut txn = nodes[0].tx_broadcaster.txn_broadcast(); + assert_eq!(txn.len(), 1); + check_spends!(txn[0], commitment_tx, coinbase_tx); + txn.remove(0) + }; +} + +#[test] +fn test_async_holder_signatures() { + do_test_async_holder_signatures(false); + do_test_async_holder_signatures(true); +} diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index d153a0a4c2d..adf3a6471d9 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -453,17 +453,33 @@ impl<'a, 'b, 'c> Node<'a, 'b, 'c> { /// several of the signing methods. Currently, only `get_per_commitment_point` and /// `release_commitment_secret` are affected by this setting. #[cfg(test)] - pub fn set_channel_signer_available(&self, peer_id: &PublicKey, chan_id: &ChannelId, available: bool) { + pub fn set_channel_signer_available(&self, peer_id: &PublicKey, chan_id: &ChannelId, funding_txo: Option, available: bool) { + use crate::sign::ChannelSigner; + log_debug!(self.logger, "Setting channel signer for {} as available={}", chan_id, available); + let per_peer_state = self.node.per_peer_state.read().unwrap(); let chan_lock = per_peer_state.get(peer_id).unwrap().lock().unwrap(); - let signer = (|| { - match chan_lock.channel_by_id.get(chan_id) { - Some(phase) => phase.context().get_signer(), - None => panic!("Couldn't find a channel with id {}", chan_id), + if let Some(chan) = chan_lock.channel_by_id.get(chan_id).map(|phase| phase.context()) { + chan.get_signer().as_ecdsa().unwrap().set_available(available); + } + + if let Some(funding_txo) = funding_txo { + if let Ok(monitor) = self.chain_monitor.chain_monitor.get_monitor(funding_txo) { + let mut channel_keys_id = None; + monitor.do_signer_call(|signer| { + channel_keys_id = Some(signer.inner.channel_keys_id()); + signer.set_available(available) + }); + + if available { + self.keys_manager.unavailable_signers.lock().unwrap() + .remove(channel_keys_id.as_ref().unwrap()); + } else { + self.keys_manager.unavailable_signers.lock().unwrap() + .insert(channel_keys_id.unwrap()); + } } - })(); - log_debug!(self.logger, "Setting channel signer for {} as available={}", chan_id, available); - signer.as_ecdsa().unwrap().set_available(available); + } } } diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs index a2cbf78b700..3d00d2e602e 100644 --- a/lightning/src/util/test_channel_signer.rs +++ b/lightning/src/util/test_channel_signer.rs @@ -117,7 +117,6 @@ impl TestChannelSigner { /// When `true`, methods are forwarded to the underlying signer as normal. When `false`, some /// methods will return `Err` indicating that the signer is unavailable. Intended to be used for /// testing asynchronous signing. - #[cfg(test)] pub fn set_available(&self, available: bool) { *self.available.lock().unwrap() = available; } @@ -221,6 +220,9 @@ impl EcdsaChannelSigner for TestChannelSigner { &self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor, secp_ctx: &Secp256k1 ) -> Result { + if !*self.available.lock().unwrap() { + return Err(()); + } let state = self.state.lock().unwrap(); if state.last_holder_revoked_commitment - 1 != htlc_descriptor.per_commitment_number && state.last_holder_revoked_commitment - 2 != htlc_descriptor.per_commitment_number diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 1c45c4a4c5d..0d5133be178 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -1124,6 +1124,7 @@ pub struct TestKeysInterface { pub disable_revocation_policy_check: bool, enforcement_states: Mutex>>>, expectations: Mutex>>, + pub unavailable_signers: Mutex>, } impl EntropySource for TestKeysInterface { @@ -1182,7 +1183,11 @@ impl SignerProvider for TestKeysInterface { fn derive_channel_signer(&self, channel_value_satoshis: u64, channel_keys_id: [u8; 32]) -> TestChannelSigner { let keys = self.backing.derive_channel_signer(channel_value_satoshis, channel_keys_id); let state = self.make_enforcement_state_cell(keys.commitment_seed); - TestChannelSigner::new_with_revoked(keys, state, self.disable_revocation_policy_check) + let signer = TestChannelSigner::new_with_revoked(keys, state, self.disable_revocation_policy_check); + if self.unavailable_signers.lock().unwrap().contains(&channel_keys_id) { + signer.set_available(false); + } + signer } fn read_chan_signer(&self, buffer: &[u8]) -> Result { @@ -1220,6 +1225,7 @@ impl TestKeysInterface { disable_revocation_policy_check: false, enforcement_states: Mutex::new(HashMap::new()), expectations: Mutex::new(None), + unavailable_signers: Mutex::new(HashSet::new()), } } @@ -1233,9 +1239,7 @@ impl TestKeysInterface { } pub fn derive_channel_keys(&self, channel_value_satoshis: u64, id: &[u8; 32]) -> TestChannelSigner { - let keys = self.backing.derive_channel_keys(channel_value_satoshis, id); - let state = self.make_enforcement_state_cell(keys.commitment_seed); - TestChannelSigner::new_with_revoked(keys, state, self.disable_revocation_policy_check) + self.derive_channel_signer(channel_value_satoshis, *id) } fn make_enforcement_state_cell(&self, commitment_seed: [u8; 32]) -> Arc> {