X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fchain%2Fchannelmonitor.rs;h=681d895f27e05fa6ef42105f468bae7a1a258250;hb=62edee568985e3362bd1609c6089d05428023925;hp=1699e4827236c89ba6e1dc6fef6b7ff726db6366;hpb=19191b450c8f07e7b703ec83eabaa0d268757bd7;p=rust-lightning diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 1699e482..681d895f 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -20,7 +20,7 @@ //! security-domain-separated system design, you should consider having multiple paths for //! ChannelMonitors to get out of the HSM and onto monitoring devices. -use bitcoin::blockdata::block::{Block, BlockHeader}; +use bitcoin::blockdata::block::BlockHeader; use bitcoin::blockdata::transaction::{TxOut,Transaction}; use bitcoin::blockdata::script::{Script, Builder}; use bitcoin::blockdata::opcodes; @@ -59,7 +59,7 @@ use sync::Mutex; /// An update generated by the underlying Channel itself which contains some new information the /// ChannelMonitor should be made aware of. -#[cfg_attr(any(test, feature = "fuzztarget", feature = "_test_utils"), derive(PartialEq))] +#[cfg_attr(any(test, fuzzing, feature = "_test_utils"), derive(PartialEq))] #[derive(Clone)] #[must_use] pub struct ChannelMonitorUpdate { @@ -115,14 +115,6 @@ impl Readable for ChannelMonitorUpdate { } } -/// General Err type for ChannelMonitor actions. Generally, this implies that the data provided is -/// inconsistent with the ChannelMonitor being called. eg for ChannelMonitor::update_monitor this -/// means you tried to update a monitor for a different channel or the ChannelMonitorUpdate was -/// corrupted. -/// Contains a developer-readable error message. -#[derive(Clone, Debug)] -pub struct MonitorUpdateError(pub &'static str); - /// An event to be processed by the ChannelManager. #[derive(Clone, PartialEq)] pub enum MonitorEvent { @@ -449,7 +441,7 @@ impl_writeable_tlv_based_enum_upgradable!(OnchainEvent, ); -#[cfg_attr(any(test, feature = "fuzztarget", feature = "_test_utils"), derive(PartialEq))] +#[cfg_attr(any(test, fuzzing, feature = "_test_utils"), derive(PartialEq))] #[derive(Clone)] pub(crate) enum ChannelMonitorUpdateStep { LatestHolderCommitmentTXInfo { @@ -481,6 +473,19 @@ pub(crate) enum ChannelMonitorUpdateStep { }, } +impl ChannelMonitorUpdateStep { + fn variant_name(&self) -> &'static str { + match self { + ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { .. } => "LatestHolderCommitmentTXInfo", + ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. } => "LatestCounterpartyCommitmentTXInfo", + ChannelMonitorUpdateStep::PaymentPreimage { .. } => "PaymentPreimage", + ChannelMonitorUpdateStep::CommitmentSecret { .. } => "CommitmentSecret", + ChannelMonitorUpdateStep::ChannelForceClosed { .. } => "ChannelForceClosed", + ChannelMonitorUpdateStep::ShutdownScript { .. } => "ShutdownScript", + } + } +} + impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep, (0, LatestHolderCommitmentTXInfo) => { (0, commitment_tx, required), @@ -695,6 +700,11 @@ pub(crate) struct ChannelMonitorImpl { // remote monitor out-of-order with regards to the block view. holder_tx_signed: bool, + // If a spend of the funding output is seen, we set this to true and reject any further + // updates. This prevents any further changes in the offchain state no matter the order + // of block connection between ChannelMonitors and the ChannelManager. + funding_spend_seen: bool, + funding_spend_confirmed: Option, /// The set of HTLCs which have been either claimed or failed on chain and have reached /// the requisite confirmations on the claim/fail transaction (either ANTI_REORG_DELAY or the @@ -714,9 +724,9 @@ pub(crate) struct ChannelMonitorImpl { /// Transaction outputs to watch for on-chain spends. pub type TransactionOutputs = (Txid, Vec<(u32, TxOut)>); -#[cfg(any(test, feature = "fuzztarget", feature = "_test_utils"))] -/// Used only in testing and fuzztarget to check serialization roundtrips don't change the -/// underlying object +#[cfg(any(test, fuzzing, feature = "_test_utils"))] +/// Used only in testing and fuzzing to check serialization roundtrips don't change the underlying +/// object impl PartialEq for ChannelMonitor { fn eq(&self, other: &Self) -> bool { let inner = self.inner.lock().unwrap(); @@ -725,9 +735,9 @@ impl PartialEq for ChannelMonitor { } } -#[cfg(any(test, feature = "fuzztarget", feature = "_test_utils"))] -/// Used only in testing and fuzztarget to check serialization roundtrips don't change the -/// underlying object +#[cfg(any(test, fuzzing, feature = "_test_utils"))] +/// Used only in testing and fuzzing to check serialization roundtrips don't change the underlying +/// object impl PartialEq for ChannelMonitorImpl { fn eq(&self, other: &Self) -> bool { if self.latest_update_id != other.latest_update_id || @@ -760,6 +770,7 @@ impl PartialEq for ChannelMonitorImpl { self.outputs_to_watch != other.outputs_to_watch || self.lockdown_from_offchain != other.lockdown_from_offchain || self.holder_tx_signed != other.holder_tx_signed || + self.funding_spend_seen != other.funding_spend_seen || self.funding_spend_confirmed != other.funding_spend_confirmed || self.htlcs_resolved_on_chain != other.htlcs_resolved_on_chain { @@ -935,6 +946,7 @@ impl Writeable for ChannelMonitorImpl { (1, self.funding_spend_confirmed, option), (3, self.htlcs_resolved_on_chain, vec_type), (5, self.pending_monitor_events, vec_type), + (7, self.funding_spend_seen, required), }); Ok(()) @@ -1033,6 +1045,7 @@ impl ChannelMonitor { lockdown_from_offchain: false, holder_tx_signed: false, + funding_spend_seen: false, funding_spend_confirmed: None, htlcs_resolved_on_chain: Vec::new(), @@ -1044,7 +1057,7 @@ impl ChannelMonitor { } #[cfg(test)] - fn provide_secret(&self, idx: u64, secret: [u8; 32]) -> Result<(), MonitorUpdateError> { + fn provide_secret(&self, idx: u64, secret: [u8; 32]) -> Result<(), &'static str> { self.inner.lock().unwrap().provide_secret(idx, secret) } @@ -1066,12 +1079,10 @@ impl ChannelMonitor { #[cfg(test)] fn provide_latest_holder_commitment_tx( - &self, - holder_commitment_tx: HolderCommitmentTransaction, + &self, holder_commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option, Option)>, - ) -> Result<(), MonitorUpdateError> { - self.inner.lock().unwrap().provide_latest_holder_commitment_tx( - holder_commitment_tx, htlc_outputs) + ) -> Result<(), ()> { + self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs).map_err(|_| ()) } #[cfg(test)] @@ -1112,7 +1123,7 @@ impl ChannelMonitor { broadcaster: &B, fee_estimator: &F, logger: &L, - ) -> Result<(), MonitorUpdateError> + ) -> Result<(), ()> where B::Target: BroadcasterInterface, F::Target: FeeEstimator, @@ -1381,8 +1392,23 @@ impl ChannelMonitor { ($holder_commitment: expr, $htlc_iter: expr) => { for htlc in $htlc_iter { if let Some(htlc_input_idx) = htlc.transaction_output_index { - if us.htlcs_resolved_on_chain.iter().any(|v| v.input_idx == htlc_input_idx) { - assert!(us.funding_spend_confirmed.is_some()); + if let Some(conf_thresh) = us.onchain_events_awaiting_threshold_conf.iter().find_map(|event| { + if let OnchainEvent::MaturingOutput { descriptor: SpendableOutputDescriptor::DelayedPaymentOutput(descriptor) } = &event.event { + if descriptor.outpoint.index as u32 == htlc_input_idx { Some(event.confirmation_threshold()) } else { None } + } else { None } + }) { + debug_assert!($holder_commitment); + res.push(Balance::ClaimableAwaitingConfirmations { + claimable_amount_satoshis: htlc.amount_msat / 1000, + confirmation_height: conf_thresh, + }); + } else if us.htlcs_resolved_on_chain.iter().any(|v| v.input_idx == htlc_input_idx) { + // Funding transaction spends should be fully confirmed by the time any + // HTLC transactions are resolved, unless we're talking about a holder + // commitment tx, whose resolution is delayed until the CSV timeout is + // reached, even though HTLCs may be resolved after only + // ANTI_REORG_DELAY confirmations. + debug_assert!($holder_commitment || us.funding_spend_confirmed.is_some()); } else if htlc.offered == $holder_commitment { // If the payment was outbound, check if there's an HTLCUpdate // indicating we have spent this HTLC with a timeout, claiming it back @@ -1687,9 +1713,9 @@ impl ChannelMonitorImpl { /// Inserts a revocation secret into this channel monitor. Prunes old preimages if neither /// needed by holder commitment transactions HTCLs nor by counterparty ones. Unless we haven't already seen /// counterparty commitment transaction's secret, they are de facto pruned (we can use revocation key). - fn provide_secret(&mut self, idx: u64, secret: [u8; 32]) -> Result<(), MonitorUpdateError> { + fn provide_secret(&mut self, idx: u64, secret: [u8; 32]) -> Result<(), &'static str> { if let Err(()) = self.commitment_secrets.provide_secret(idx, secret) { - return Err(MonitorUpdateError("Previous secret did not match new one")); + return Err("Previous secret did not match new one"); } // Prune HTLCs from the previous counterparty commitment tx so we don't generate failure/fulfill @@ -1781,7 +1807,7 @@ impl ChannelMonitorImpl { /// is important that any clones of this channel monitor (including remote clones) by kept /// up-to-date as our holder commitment transaction is updated. /// Panics if set_on_holder_tx_csv has never been called. - fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option, Option)>) -> Result<(), MonitorUpdateError> { + fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option, Option)>) -> Result<(), &'static str> { // block for Rust 1.34 compat let mut new_holder_commitment_tx = { let trusted_tx = holder_commitment_tx.trust(); @@ -1804,7 +1830,7 @@ impl ChannelMonitorImpl { mem::swap(&mut new_holder_commitment_tx, &mut self.current_holder_commitment_tx); self.prev_holder_signed_commitment_tx = Some(new_holder_commitment_tx); if self.holder_tx_signed { - return Err(MonitorUpdateError("Latest holder commitment signed has already been signed, update is rejected")); + return Err("Latest holder commitment signed has already been signed, update is rejected"); } Ok(()) } @@ -1868,30 +1894,40 @@ impl ChannelMonitorImpl { self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0)); } - pub fn update_monitor(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &L) -> Result<(), MonitorUpdateError> + pub fn update_monitor(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &L) -> Result<(), ()> where B::Target: BroadcasterInterface, F::Target: FeeEstimator, L::Target: Logger, { + log_info!(logger, "Applying update to monitor {}, bringing update_id from {} to {} with {} changes.", + log_funding_info!(self), self.latest_update_id, updates.update_id, updates.updates.len()); // ChannelMonitor updates may be applied after force close if we receive a // preimage for a broadcasted commitment transaction HTLC output that we'd // like to claim on-chain. If this is the case, we no longer have guaranteed // access to the monitor's update ID, so we use a sentinel value instead. if updates.update_id == CLOSED_CHANNEL_UPDATE_ID { + assert_eq!(updates.updates.len(), 1); match updates.updates[0] { ChannelMonitorUpdateStep::PaymentPreimage { .. } => {}, - _ => panic!("Attempted to apply post-force-close ChannelMonitorUpdate that wasn't providing a payment preimage"), + _ => { + log_error!(logger, "Attempted to apply post-force-close ChannelMonitorUpdate of type {}", updates.updates[0].variant_name()); + panic!("Attempted to apply post-force-close ChannelMonitorUpdate that wasn't providing a payment preimage"); + }, } - assert_eq!(updates.updates.len(), 1); } else if self.latest_update_id + 1 != updates.update_id { panic!("Attempted to apply ChannelMonitorUpdates out of order, check the update_id before passing an update to update_monitor!"); } + let mut ret = Ok(()); for update in updates.updates.iter() { match update { ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs } => { log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction info"); if self.lockdown_from_offchain { panic!(); } - self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone())? + if let Err(e) = self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone()) { + log_error!(logger, "Providing latest holder commitment transaction failed/was refused:"); + log_error!(logger, " {}", e); + ret = Err(()); + } } ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_revocation_point } => { log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info"); @@ -1903,7 +1939,11 @@ impl ChannelMonitorImpl { }, ChannelMonitorUpdateStep::CommitmentSecret { idx, secret } => { log_trace!(logger, "Updating ChannelMonitor with commitment secret"); - self.provide_secret(*idx, *secret)? + if let Err(e) = self.provide_secret(*idx, *secret) { + log_error!(logger, "Providing latest counterparty commitment secret failed/was refused:"); + log_error!(logger, " {}", e); + ret = Err(()); + } }, ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast } => { log_trace!(logger, "Updating ChannelMonitor: channel force closed, should broadcast: {}", should_broadcast); @@ -1928,7 +1968,11 @@ impl ChannelMonitorImpl { } } self.latest_update_id = updates.update_id; - Ok(()) + + if ret.is_ok() && self.funding_spend_seen { + log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent"); + Err(()) + } else { ret } } pub fn get_latest_update_id(&self) -> u64 { @@ -2030,7 +2074,7 @@ impl ChannelMonitorImpl { 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 } - let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone()); + let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), self.onchain_tx_handler.channel_transaction_parameters.opt_anchors.is_some()); let justice_package = PackageTemplate::build_package(commitment_txid, transaction_output_index, PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp), htlc.cltv_expiry, true, height); claimable_outpoints.push(justice_package); } @@ -2357,6 +2401,7 @@ impl ChannelMonitorImpl { let mut balance_spendable_csv = None; log_info!(logger, "Channel {} closed by funding output spend in txid {}.", log_bytes!(self.funding_info.0.to_channel_id()), tx.txid()); + self.funding_spend_seen = true; if (tx.input[0].sequence >> 8*3) as u8 == 0x80 && (tx.lock_time >> 8*3) as u8 == 0x20 { let (mut new_outpoints, new_outputs) = self.check_spend_counterparty_transaction(&tx, height, &logger); if !new_outputs.1.is_empty() { @@ -2962,9 +3007,8 @@ where F::Target: FeeEstimator, L::Target: Logger, { - fn block_connected(&self, block: &Block, height: u32) { - let txdata: Vec<_> = block.txdata.iter().enumerate().collect(); - self.0.block_connected(&block.header, &txdata, height, &*self.1, &*self.2, &*self.3); + fn filtered_block_connected(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) { + self.0.block_connected(header, txdata, height, &*self.1, &*self.2, &*self.3); } fn block_disconnected(&self, header: &BlockHeader, height: u32) { @@ -3206,10 +3250,12 @@ impl<'a, Signer: Sign, K: KeysInterface> ReadableArgs<&'a K> let mut funding_spend_confirmed = None; let mut htlcs_resolved_on_chain = Some(Vec::new()); + let mut funding_spend_seen = Some(false); read_tlv_fields!(reader, { (1, funding_spend_confirmed, option), (3, htlcs_resolved_on_chain, vec_type), (5, pending_monitor_events, vec_type), + (7, funding_spend_seen, option), }); let mut secp_ctx = Secp256k1::new(); @@ -3259,6 +3305,7 @@ impl<'a, Signer: Sign, K: KeysInterface> ReadableArgs<&'a K> lockdown_from_offchain, holder_tx_signed, + funding_spend_seen: funding_spend_seen.unwrap(), funding_spend_confirmed, htlcs_resolved_on_chain: htlcs_resolved_on_chain.unwrap(), @@ -3272,6 +3319,7 @@ impl<'a, Signer: Sign, K: KeysInterface> ReadableArgs<&'a K> #[cfg(test)] mod tests { + use bitcoin::blockdata::block::BlockHeader; use bitcoin::blockdata::script::{Script, Builder}; use bitcoin::blockdata::opcodes; use bitcoin::blockdata::transaction::{Transaction, TxIn, TxOut, SigHashType}; @@ -3280,24 +3328,128 @@ mod tests { use bitcoin::hashes::Hash; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::hex::FromHex; - use bitcoin::hash_types::Txid; + use bitcoin::hash_types::{BlockHash, Txid}; use bitcoin::network::constants::Network; + use bitcoin::secp256k1::key::{SecretKey,PublicKey}; + use bitcoin::secp256k1::Secp256k1; + use hex; - use chain::BestBlock; + + use super::ChannelMonitorUpdateStep; + use ::{check_added_monitors, check_closed_broadcast, check_closed_event, check_spends, get_local_commitment_txn, get_monitor, get_route_and_payment_hash, unwrap_send_err}; + use chain::{BestBlock, Confirm}; use chain::channelmonitor::ChannelMonitor; - use chain::package::{WEIGHT_OFFERED_HTLC, WEIGHT_RECEIVED_HTLC, WEIGHT_REVOKED_OFFERED_HTLC, WEIGHT_REVOKED_RECEIVED_HTLC, WEIGHT_REVOKED_OUTPUT}; + use chain::package::{weight_offered_htlc, weight_received_htlc, weight_revoked_offered_htlc, weight_revoked_received_htlc, WEIGHT_REVOKED_OUTPUT}; use chain::transaction::OutPoint; + use chain::keysinterface::InMemorySigner; use ln::{PaymentPreimage, PaymentHash}; use ln::chan_utils; use ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, ChannelTransactionParameters, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters}; + use ln::channelmanager::PaymentSendFailure; + use ln::features::InitFeatures; + use ln::functional_test_utils::*; use ln::script::ShutdownScript; + use util::errors::APIError; + use util::events::{ClosureReason, MessageSendEventsProvider}; use util::test_utils::{TestLogger, TestBroadcaster, TestFeeEstimator}; - use bitcoin::secp256k1::key::{SecretKey,PublicKey}; - use bitcoin::secp256k1::Secp256k1; + use util::ser::{ReadableArgs, Writeable}; use sync::{Arc, Mutex}; - use chain::keysinterface::InMemorySigner; + use io; use prelude::*; + fn do_test_funding_spend_refuses_updates(use_local_txn: bool) { + // Previously, monitor updates were allowed freely even after a funding-spend transaction + // confirmed. This would allow a race condition where we could receive a payment (including + // the counterparty revoking their broadcasted state!) and accept it without recourse as + // long as the ChannelMonitor receives the block first, the full commitment update dance + // occurs after the block is connected, and before the ChannelManager receives the block. + // Obviously this is an incredibly contrived race given the counterparty would be risking + // their full channel balance for it, but its worth fixing nonetheless as it makes the + // potential ChannelMonitor states simpler to reason about. + // + // This test checks said behavior, as well as ensuring a ChannelMonitorUpdate with multiple + // updates is handled correctly in such conditions. + 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); + let channel = create_announced_chan_between_nodes( + &nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); + create_announced_chan_between_nodes( + &nodes, 1, 2, InitFeatures::known(), InitFeatures::known()); + + // Rebalance somewhat + send_payment(&nodes[0], &[&nodes[1]], 10_000_000); + + // First route two payments for testing at the end + let payment_preimage_1 = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000).0; + let payment_preimage_2 = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000).0; + + let local_txn = get_local_commitment_txn!(nodes[1], channel.2); + assert_eq!(local_txn.len(), 1); + let remote_txn = get_local_commitment_txn!(nodes[0], channel.2); + assert_eq!(remote_txn.len(), 3); // Commitment and two HTLC-Timeouts + check_spends!(remote_txn[1], remote_txn[0]); + check_spends!(remote_txn[2], remote_txn[0]); + let broadcast_tx = if use_local_txn { &local_txn[0] } else { &remote_txn[0] }; + + // Connect a commitment transaction, but only to the ChainMonitor/ChannelMonitor. The + // channel is now closed, but the ChannelManager doesn't know that yet. + let new_header = BlockHeader { + version: 2, time: 0, bits: 0, nonce: 0, + prev_blockhash: nodes[0].best_block_info().0, + merkle_root: Default::default() }; + let conf_height = nodes[0].best_block_info().1 + 1; + nodes[1].chain_monitor.chain_monitor.transactions_confirmed(&new_header, + &[(0, broadcast_tx)], conf_height); + + let (_, pre_update_monitor) = <(BlockHash, ChannelMonitor)>::read( + &mut io::Cursor::new(&get_monitor!(nodes[1], channel.2).encode()), + &nodes[1].keys_manager.backing).unwrap(); + + // If the ChannelManager tries to update the channel, however, the ChainMonitor will pass + // the update through to the ChannelMonitor which will refuse it (as the channel is closed). + let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 100_000); + unwrap_send_err!(nodes[1].node.send_payment(&route, payment_hash, &Some(payment_secret)), + true, APIError::ChannelUnavailable { ref err }, + assert!(err.contains("ChannelMonitor storage failure"))); + check_added_monitors!(nodes[1], 2); // After the failure we generate a close-channel monitor update + check_closed_broadcast!(nodes[1], true); + check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() }); + + // Build a new ChannelMonitorUpdate which contains both the failing commitment tx update + // and provides the claim preimages for the two pending HTLCs. The first update generates + // an error, but the point of this test is to ensure the later updates are still applied. + let monitor_updates = nodes[1].chain_monitor.monitor_updates.lock().unwrap(); + let mut replay_update = monitor_updates.get(&channel.2).unwrap().iter().rev().skip(1).next().unwrap().clone(); + assert_eq!(replay_update.updates.len(), 1); + if let ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. } = replay_update.updates[0] { + } else { panic!(); } + replay_update.updates.push(ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage: payment_preimage_1 }); + replay_update.updates.push(ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage: payment_preimage_2 }); + + let broadcaster = TestBroadcaster::new(Arc::clone(&nodes[1].blocks)); + assert!( + pre_update_monitor.update_monitor(&replay_update, &&broadcaster, &&chanmon_cfgs[1].fee_estimator, &nodes[1].logger) + .is_err()); + // Even though we error'd on the first update, we should still have generated an HTLC claim + // transaction + let txn_broadcasted = broadcaster.txn_broadcasted.lock().unwrap().split_off(0); + assert!(txn_broadcasted.len() >= 2); + let htlc_txn = txn_broadcasted.iter().filter(|tx| { + assert_eq!(tx.input.len(), 1); + tx.input[0].previous_output.txid == broadcast_tx.txid() + }).collect::>(); + assert_eq!(htlc_txn.len(), 2); + check_spends!(htlc_txn[0], broadcast_tx); + check_spends!(htlc_txn[1], broadcast_tx); + } + #[test] + fn test_funding_spend_refuses_updates() { + do_test_funding_spend_refuses_updates(true); + do_test_funding_spend_refuses_updates(false); + } + #[test] fn test_prune_preimages() { let secp_ctx = Secp256k1::new(); @@ -3359,6 +3511,7 @@ mod tests { SecretKey::from_slice(&[41; 32]).unwrap(), SecretKey::from_slice(&[41; 32]).unwrap(), SecretKey::from_slice(&[41; 32]).unwrap(), + SecretKey::from_slice(&[41; 32]).unwrap(), [41; 32], 0, [0; 32] @@ -3444,12 +3597,11 @@ mod tests { let secp_ctx = Secp256k1::new(); let privkey = SecretKey::from_slice(&hex::decode("0101010101010101010101010101010101010101010101010101010101010101").unwrap()[..]).unwrap(); let pubkey = PublicKey::from_secret_key(&secp_ctx, &privkey); - let mut sum_actual_sigs = 0; macro_rules! sign_input { ($sighash_parts: expr, $idx: expr, $amount: expr, $weight: expr, $sum_actual_sigs: expr, $opt_anchors: expr) => { let htlc = HTLCOutputInCommitment { - offered: if *$weight == WEIGHT_REVOKED_OFFERED_HTLC || *$weight == WEIGHT_OFFERED_HTLC { true } else { false }, + offered: if *$weight == weight_revoked_offered_htlc($opt_anchors) || *$weight == weight_offered_htlc($opt_anchors) { true } else { false }, amount_msat: 0, cltv_expiry: 2 << 16, payment_hash: PaymentHash([1; 32]), @@ -3460,12 +3612,12 @@ mod tests { let sig = secp_ctx.sign(&sighash, &privkey); $sighash_parts.access_witness($idx).push(sig.serialize_der().to_vec()); $sighash_parts.access_witness($idx)[0].push(SigHashType::All as u8); - sum_actual_sigs += $sighash_parts.access_witness($idx)[0].len(); + $sum_actual_sigs += $sighash_parts.access_witness($idx)[0].len(); if *$weight == WEIGHT_REVOKED_OUTPUT { $sighash_parts.access_witness($idx).push(vec!(1)); - } else if *$weight == WEIGHT_REVOKED_OFFERED_HTLC || *$weight == WEIGHT_REVOKED_RECEIVED_HTLC { + } else if *$weight == weight_revoked_offered_htlc($opt_anchors) || *$weight == weight_revoked_received_htlc($opt_anchors) { $sighash_parts.access_witness($idx).push(pubkey.clone().serialize().to_vec()); - } else if *$weight == WEIGHT_RECEIVED_HTLC { + } else if *$weight == weight_received_htlc($opt_anchors) { $sighash_parts.access_witness($idx).push(vec![0]); } else { $sighash_parts.access_witness($idx).push(PaymentPreimage([1; 32]).0.to_vec()); @@ -3481,83 +3633,98 @@ mod tests { let txid = Txid::from_hex("56944c5d3f98413ef45cf54545538103cc9f298e0575820ad3591376e2e0f65d").unwrap(); // Justice tx with 1 to_holder, 2 revoked offered HTLCs, 1 revoked received HTLCs - let mut claim_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() }; - for i in 0..4 { - claim_tx.input.push(TxIn { - previous_output: BitcoinOutPoint { - txid, - vout: i, - }, - script_sig: Script::new(), - sequence: 0xfffffffd, - witness: Vec::new(), + for &opt_anchors in [false, true].iter() { + let mut claim_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() }; + let mut sum_actual_sigs = 0; + for i in 0..4 { + claim_tx.input.push(TxIn { + previous_output: BitcoinOutPoint { + txid, + vout: i, + }, + script_sig: Script::new(), + sequence: 0xfffffffd, + witness: Vec::new(), + }); + } + claim_tx.output.push(TxOut { + script_pubkey: script_pubkey.clone(), + value: 0, }); - } - claim_tx.output.push(TxOut { - script_pubkey: script_pubkey.clone(), - value: 0, - }); - let base_weight = claim_tx.get_weight(); - let inputs_weight = vec![WEIGHT_REVOKED_OUTPUT, WEIGHT_REVOKED_OFFERED_HTLC, WEIGHT_REVOKED_OFFERED_HTLC, WEIGHT_REVOKED_RECEIVED_HTLC]; - let mut inputs_total_weight = 2; // count segwit flags - { - let mut sighash_parts = bip143::SigHashCache::new(&mut claim_tx); - for (idx, inp) in inputs_weight.iter().enumerate() { - sign_input!(sighash_parts, idx, 0, inp, sum_actual_sigs, false); - inputs_total_weight += inp; + let base_weight = claim_tx.get_weight(); + let inputs_weight = vec![WEIGHT_REVOKED_OUTPUT, weight_revoked_offered_htlc(opt_anchors), weight_revoked_offered_htlc(opt_anchors), weight_revoked_received_htlc(opt_anchors)]; + let mut inputs_total_weight = 2; // count segwit flags + { + let mut sighash_parts = bip143::SigHashCache::new(&mut claim_tx); + for (idx, inp) in inputs_weight.iter().enumerate() { + sign_input!(sighash_parts, idx, 0, inp, sum_actual_sigs, opt_anchors); + inputs_total_weight += inp; + } } + assert_eq!(base_weight + inputs_total_weight as usize, claim_tx.get_weight() + /* max_length_sig */ (73 * inputs_weight.len() - sum_actual_sigs)); } - assert_eq!(base_weight + inputs_total_weight as usize, claim_tx.get_weight() + /* max_length_sig */ (73 * inputs_weight.len() - sum_actual_sigs)); // Claim tx with 1 offered HTLCs, 3 received HTLCs - claim_tx.input.clear(); - sum_actual_sigs = 0; - for i in 0..4 { + for &opt_anchors in [false, true].iter() { + let mut claim_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() }; + let mut sum_actual_sigs = 0; + for i in 0..4 { + claim_tx.input.push(TxIn { + previous_output: BitcoinOutPoint { + txid, + vout: i, + }, + script_sig: Script::new(), + sequence: 0xfffffffd, + witness: Vec::new(), + }); + } + claim_tx.output.push(TxOut { + script_pubkey: script_pubkey.clone(), + value: 0, + }); + let base_weight = claim_tx.get_weight(); + let inputs_weight = vec![weight_offered_htlc(opt_anchors), weight_received_htlc(opt_anchors), weight_received_htlc(opt_anchors), weight_received_htlc(opt_anchors)]; + let mut inputs_total_weight = 2; // count segwit flags + { + let mut sighash_parts = bip143::SigHashCache::new(&mut claim_tx); + for (idx, inp) in inputs_weight.iter().enumerate() { + sign_input!(sighash_parts, idx, 0, inp, sum_actual_sigs, opt_anchors); + inputs_total_weight += inp; + } + } + assert_eq!(base_weight + inputs_total_weight as usize, claim_tx.get_weight() + /* max_length_sig */ (73 * inputs_weight.len() - sum_actual_sigs)); + } + + // Justice tx with 1 revoked HTLC-Success tx output + for &opt_anchors in [false, true].iter() { + let mut claim_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() }; + let mut sum_actual_sigs = 0; claim_tx.input.push(TxIn { previous_output: BitcoinOutPoint { txid, - vout: i, + vout: 0, }, script_sig: Script::new(), sequence: 0xfffffffd, witness: Vec::new(), }); - } - let base_weight = claim_tx.get_weight(); - let inputs_weight = vec![WEIGHT_OFFERED_HTLC, WEIGHT_RECEIVED_HTLC, WEIGHT_RECEIVED_HTLC, WEIGHT_RECEIVED_HTLC]; - let mut inputs_total_weight = 2; // count segwit flags - { - let mut sighash_parts = bip143::SigHashCache::new(&mut claim_tx); - for (idx, inp) in inputs_weight.iter().enumerate() { - sign_input!(sighash_parts, idx, 0, inp, sum_actual_sigs, false); - inputs_total_weight += inp; - } - } - assert_eq!(base_weight + inputs_total_weight as usize, claim_tx.get_weight() + /* max_length_sig */ (73 * inputs_weight.len() - sum_actual_sigs)); - - // Justice tx with 1 revoked HTLC-Success tx output - claim_tx.input.clear(); - sum_actual_sigs = 0; - claim_tx.input.push(TxIn { - previous_output: BitcoinOutPoint { - txid, - vout: 0, - }, - script_sig: Script::new(), - sequence: 0xfffffffd, - witness: Vec::new(), - }); - let base_weight = claim_tx.get_weight(); - let inputs_weight = vec![WEIGHT_REVOKED_OUTPUT]; - let mut inputs_total_weight = 2; // count segwit flags - { - let mut sighash_parts = bip143::SigHashCache::new(&mut claim_tx); - for (idx, inp) in inputs_weight.iter().enumerate() { - sign_input!(sighash_parts, idx, 0, inp, sum_actual_sigs, false); - inputs_total_weight += inp; + claim_tx.output.push(TxOut { + script_pubkey: script_pubkey.clone(), + value: 0, + }); + let base_weight = claim_tx.get_weight(); + let inputs_weight = vec![WEIGHT_REVOKED_OUTPUT]; + let mut inputs_total_weight = 2; // count segwit flags + { + let mut sighash_parts = bip143::SigHashCache::new(&mut claim_tx); + for (idx, inp) in inputs_weight.iter().enumerate() { + sign_input!(sighash_parts, idx, 0, inp, sum_actual_sigs, opt_anchors); + inputs_total_weight += inp; + } } + assert_eq!(base_weight + inputs_total_weight as usize, claim_tx.get_weight() + /* max_length_isg */ (73 * inputs_weight.len() - sum_actual_sigs)); } - assert_eq!(base_weight + inputs_total_weight as usize, claim_tx.get_weight() + /* max_length_isg */ (73 * inputs_weight.len() - sum_actual_sigs)); } // Further testing is done in the ChannelManager integration tests.