diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 5128600163a..18c53dc3584 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -42,7 +42,7 @@ use chain; use chain::{BestBlock, WatchedOutput}; use chain::chaininterface::{BroadcasterInterface, FeeEstimator}; use chain::transaction::{OutPoint, TransactionData}; -use chain::keysinterface::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, Sign, KeysInterface}; +use chain::keysinterface::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, Sign, KeysInterface, SignError}; use chain::onchaintx::OnchainTxHandler; use chain::package::{CounterpartyOfferedHTLCOutput, CounterpartyReceivedHTLCOutput, HolderFundingOutput, HolderHTLCOutput, PackageSolvingData, PackageTemplate, RevokedOutput, RevokedHTLCOutput}; use chain::Filter; @@ -1098,7 +1098,7 @@ impl ChannelMonitor { broadcaster: &B, fee_estimator: &F, logger: &L, - ) where + ) -> Result<(), SignError> where B::Target: BroadcasterInterface, F::Target: FeeEstimator, L::Target: Logger, @@ -1111,7 +1111,8 @@ impl ChannelMonitor { &self, broadcaster: &B, logger: &L, - ) where + ) + where B::Target: BroadcasterInterface, L::Target: Logger, { @@ -1210,7 +1211,7 @@ impl ChannelMonitor { /// substantial amount of time (a month or even a year) to get back funds. Best may be to contact /// out-of-band the other node operator to coordinate with him if option is available to you. /// In any-case, choice is up to the user. - pub fn get_latest_holder_commitment_txn(&self, logger: &L) -> Vec + pub fn get_latest_holder_commitment_txn(&self, logger: &L) -> Result, SignError> where L::Target: Logger { self.inner.lock().unwrap().get_latest_holder_commitment_txn(logger) } @@ -1847,7 +1848,7 @@ impl ChannelMonitorImpl { /// Provides a payment_hash->payment_preimage mapping. Will be automatically pruned when all /// commitment_tx_infos which contain the payment hash have been revoked. - fn provide_payment_preimage(&mut self, payment_hash: &PaymentHash, payment_preimage: &PaymentPreimage, broadcaster: &B, fee_estimator: &F, logger: &L) + fn provide_payment_preimage(&mut self, payment_hash: &PaymentHash, payment_preimage: &PaymentPreimage, broadcaster: &B, fee_estimator: &F, logger: &L) -> Result<(), SignError> where B::Target: BroadcasterInterface, F::Target: FeeEstimator, L::Target: Logger, @@ -1859,19 +1860,19 @@ impl ChannelMonitorImpl { 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, self.best_block.height(), self.best_block.height(), broadcaster, fee_estimator, logger); + self.onchain_tx_handler.update_claims_view(&Vec::new(), htlc_claim_reqs, self.best_block.height(), self.best_block.height(), 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; + return Ok(()); } } 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; + return Ok(()); } } @@ -1885,21 +1886,33 @@ impl ChannelMonitorImpl { // block. Even if not, its a reasonable metric for the bump criteria on the HTLC // transactions. let (claim_reqs, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, self.best_block.height()); - self.onchain_tx_handler.update_claims_view(&Vec::new(), claim_reqs, self.best_block.height(), self.best_block.height(), broadcaster, fee_estimator, logger); + if self.onchain_tx_handler.update_claims_view(&Vec::new(), claim_reqs, self.best_block.height(), self.best_block.height(), broadcaster, fee_estimator, logger).is_err() { + log_warn!(logger, "Unable to broadcast claims because signer is unavailable, will retry"); + } if let Some(ref tx) = self.prev_holder_signed_commitment_tx { let (claim_reqs, _) = self.get_broadcasted_holder_claims(&tx, self.best_block.height()); - self.onchain_tx_handler.update_claims_view(&Vec::new(), claim_reqs, self.best_block.height(), self.best_block.height(), broadcaster, fee_estimator, logger); + if self.onchain_tx_handler.update_claims_view(&Vec::new(), claim_reqs, self.best_block.height(), self.best_block.height(), broadcaster, fee_estimator, logger).is_err() { + log_warn!(logger, "Unable to broadcast claims for prev tx because signer is unavailable, will retry"); + } } } + Ok(()) } pub(crate) fn broadcast_latest_holder_commitment_txn(&mut self, broadcaster: &B, logger: &L) where B::Target: BroadcasterInterface, L::Target: Logger, { - for tx in self.get_latest_holder_commitment_txn(logger).iter() { - log_info!(logger, "Broadcasting local {}", log_tx!(tx)); - broadcaster.broadcast_transaction(tx); + match self.get_latest_holder_commitment_txn(logger) { + Ok(txs) => { + for tx in txs.iter() { + log_info!(logger, "Broadcasting local {}", log_tx!(tx)); + broadcaster.broadcast_transaction(tx); + } + } + Err(_) => { + log_warn!(logger, "Unable to broadcast holder tx because signer is unavailable, will retry"); + } } self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0)); } @@ -1945,7 +1958,8 @@ impl ChannelMonitorImpl { }, ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage } => { log_trace!(logger, "Updating ChannelMonitor with payment preimage"); - self.provide_payment_preimage(&PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner()), &payment_preimage, broadcaster, fee_estimator, logger) + // No further error handling needed + let _ = self.provide_payment_preimage(&PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner()), &payment_preimage, broadcaster, fee_estimator, logger); }, ChannelMonitorUpdateStep::CommitmentSecret { idx, secret } => { log_trace!(logger, "Updating ChannelMonitor with commitment secret"); @@ -2291,10 +2305,11 @@ impl ChannelMonitorImpl { } } - pub fn get_latest_holder_commitment_txn(&mut self, logger: &L) -> Vec where L::Target: Logger { + pub fn get_latest_holder_commitment_txn(&mut self, logger: &L) -> Result, SignError> + 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_fully_signed_holder_tx(&self.funding_redeemscript)?; let txid = commitment_tx.txid(); let mut holder_transactions = vec![commitment_tx]; for htlc in self.current_holder_commitment_tx.htlc_outputs.iter() { @@ -2313,14 +2328,14 @@ impl ChannelMonitorImpl { continue; } else { None }; if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx( - &::bitcoin::OutPoint { txid, vout }, &preimage) { + &::bitcoin::OutPoint { txid, vout }, &preimage)? { 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"))] @@ -2504,17 +2519,24 @@ impl ChannelMonitorImpl { let commitment_package = PackageTemplate::build_package(self.funding_info.0.txid.clone(), self.funding_info.0.index as u32, PackageSolvingData::HolderFundingOutput(funding_outp), self.best_block.height(), false, self.best_block.height()); claimable_outpoints.push(commitment_package); self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0)); - let commitment_tx = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript); self.holder_tx_signed = true; - // Because we're broadcasting a commitment transaction, we should construct the package - // assuming it gets confirmed in the next block. Sadly, we have code which considers - // "not yet confirmed" things as discardable, so we cannot do that here. - let (mut new_outpoints, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, self.best_block.height()); - let new_outputs = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, &commitment_tx); - if !new_outputs.is_empty() { - watch_outputs.push((self.current_holder_commitment_tx.txid.clone(), new_outputs)); + match self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript) { + Ok(commitment_tx) => { + // Because we're broadcasting a commitment transaction, we should construct the package + // assuming it gets confirmed in the next block. Sadly, we have code which considers + // "not yet confirmed" things as discardable, so we cannot do that here. + let (mut new_outpoints, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, self.best_block.height()); + let new_outputs = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, &commitment_tx); + if !new_outputs.is_empty() { + watch_outputs.push((self.current_holder_commitment_tx.txid.clone(), new_outputs)); + } + claimable_outpoints.append(&mut new_outpoints); + + } + Err(_) => { + log_warn!(logger, "Unable to broadcast holder commitment tx because the signer is not available, will retry"); + } } - claimable_outpoints.append(&mut new_outpoints); } // Find which on-chain events have reached their confirmation threshold. @@ -2587,7 +2609,9 @@ impl ChannelMonitorImpl { } } - self.onchain_tx_handler.update_claims_view(&txn_matched, claimable_outpoints, conf_height, self.best_block.height(), broadcaster, fee_estimator, logger); + if self.onchain_tx_handler.update_claims_view(&txn_matched, claimable_outpoints, conf_height, self.best_block.height(), broadcaster, fee_estimator, logger).is_err() { + log_warn!(logger, "Unable to broadcast claims because signer was not available, will retry"); + } // Determine new outputs to watch by comparing against previously known outputs to watch, // updating the latter in the process. @@ -3580,7 +3604,7 @@ mod tests { monitor.provide_latest_counterparty_commitment_tx(dummy_txid, preimages_slice_to_htlc_outputs!(preimages[17..20]), 281474976710653, dummy_key, &logger); monitor.provide_latest_counterparty_commitment_tx(dummy_txid, preimages_slice_to_htlc_outputs!(preimages[18..20]), 281474976710652, dummy_key, &logger); for &(ref preimage, ref hash) in preimages.iter() { - monitor.provide_payment_preimage(hash, preimage, &broadcaster, &fee_estimator, &logger); + monitor.provide_payment_preimage(hash, preimage, &broadcaster, &fee_estimator, &logger).unwrap(); } // Now provide a secret, pruning preimages 10-15 diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 33c88cf11e3..a26de7c2762 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -187,6 +187,15 @@ impl_writeable_tlv_based_enum!(SpendableOutputDescriptor, (2, StaticPaymentOutput), ); +/// An error while signing +#[derive(Debug)] +pub enum SignError { + /// The signer is temporarily unavailable + Temporary, + /// A signer internal error + Internal +} + /// A trait to sign lightning channel transactions as described in BOLT 3. /// /// Signing services could be implemented on a hardware wallet. In this case, @@ -278,7 +287,7 @@ pub trait BaseSign { // // TODO: Document the things someone using this interface should enforce before signing. // TODO: Key derivation failure should panic rather than Err - fn sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()>; + fn sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), SignError>; /// Same as sign_holder_commitment, but exists only for tests to get access to holder commitment /// transactions which will be broadcasted later, after the channel has moved on to a newer @@ -301,7 +310,7 @@ pub trait BaseSign { /// revoked the state which they eventually broadcast. It's not a _holder_ secret key and does /// not allow the spending of any funds by itself (you need our holder revocation_secret to do /// so). - fn sign_justice_revoked_output(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1) -> Result; + fn sign_justice_revoked_output(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1) -> Result; /// Create a signature for the given input in a transaction spending a commitment transaction /// HTLC output when our counterparty broadcasts an old state. @@ -320,7 +329,7 @@ pub trait BaseSign { /// /// htlc holds HTLC elements (hash, timelock), thus changing the format of the witness script /// (which is committed to in the BIP 143 signatures). - fn sign_justice_revoked_htlc(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1) -> Result; + fn sign_justice_revoked_htlc(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1) -> Result; /// Create a signature for a claiming transaction for a HTLC output on a counterparty's commitment /// transaction, either offered or received. @@ -683,13 +692,14 @@ impl BaseSign for InMemorySigner { Ok(()) } - fn sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { + fn sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), SignError> { let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key); let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey); let trusted_tx = commitment_tx.trust(); let sig = trusted_tx.built_transaction().sign(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, secp_ctx); let channel_parameters = self.get_channel_parameters(); - let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), secp_ctx)?; + let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), secp_ctx) + .map_err(|()| SignError::Internal)?; Ok((sig, htlc_sigs)) } @@ -704,12 +714,12 @@ impl BaseSign for InMemorySigner { Ok((sig, htlc_sigs)) } - fn sign_justice_revoked_output(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1) -> Result { - let revocation_key = chan_utils::derive_private_revocation_key(&secp_ctx, &per_commitment_key, &self.revocation_base_key).map_err(|_| ())?; + fn sign_justice_revoked_output(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1) -> Result { + let revocation_key = chan_utils::derive_private_revocation_key(&secp_ctx, &per_commitment_key, &self.revocation_base_key).map_err(|_| SignError::Internal)?; let per_commitment_point = PublicKey::from_secret_key(secp_ctx, &per_commitment_key); - let revocation_pubkey = chan_utils::derive_public_revocation_key(&secp_ctx, &per_commitment_point, &self.pubkeys().revocation_basepoint).map_err(|_| ())?; + let revocation_pubkey = chan_utils::derive_public_revocation_key(&secp_ctx, &per_commitment_point, &self.pubkeys().revocation_basepoint).map_err(|_| SignError::Internal)?; let witness_script = { - let counterparty_delayedpubkey = chan_utils::derive_public_key(&secp_ctx, &per_commitment_point, &self.counterparty_pubkeys().delayed_payment_basepoint).map_err(|_| ())?; + let counterparty_delayedpubkey = chan_utils::derive_public_key(&secp_ctx, &per_commitment_point, &self.counterparty_pubkeys().delayed_payment_basepoint).map_err(|_| SignError::Internal)?; chan_utils::get_revokeable_redeemscript(&revocation_pubkey, self.holder_selected_contest_delay(), &counterparty_delayedpubkey) }; let mut sighash_parts = sighash::SighashCache::new(justice_tx); @@ -717,13 +727,13 @@ impl BaseSign for InMemorySigner { return Ok(sign(secp_ctx, &sighash, &revocation_key)) } - fn sign_justice_revoked_htlc(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1) -> Result { - let revocation_key = chan_utils::derive_private_revocation_key(&secp_ctx, &per_commitment_key, &self.revocation_base_key).map_err(|_| ())?; + fn sign_justice_revoked_htlc(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1) -> Result { + let revocation_key = chan_utils::derive_private_revocation_key(&secp_ctx, &per_commitment_key, &self.revocation_base_key).map_err(|_| SignError::Internal)?; let per_commitment_point = PublicKey::from_secret_key(secp_ctx, &per_commitment_key); - let revocation_pubkey = chan_utils::derive_public_revocation_key(&secp_ctx, &per_commitment_point, &self.pubkeys().revocation_basepoint).map_err(|_| ())?; + let revocation_pubkey = chan_utils::derive_public_revocation_key(&secp_ctx, &per_commitment_point, &self.pubkeys().revocation_basepoint).map_err(|_| SignError::Internal)?; let witness_script = { - let counterparty_htlcpubkey = chan_utils::derive_public_key(&secp_ctx, &per_commitment_point, &self.counterparty_pubkeys().htlc_basepoint).map_err(|_| ())?; - let holder_htlcpubkey = chan_utils::derive_public_key(&secp_ctx, &per_commitment_point, &self.pubkeys().htlc_basepoint).map_err(|_| ())?; + let counterparty_htlcpubkey = chan_utils::derive_public_key(&secp_ctx, &per_commitment_point, &self.counterparty_pubkeys().htlc_basepoint).map_err(|_| SignError::Internal)?; + let holder_htlcpubkey = chan_utils::derive_public_key(&secp_ctx, &per_commitment_point, &self.pubkeys().htlc_basepoint).map_err(|_| SignError::Internal)?; chan_utils::get_htlc_redeemscript_with_explicit_keys(&htlc, self.opt_anchors(), &counterparty_htlcpubkey, &holder_htlcpubkey, &revocation_pubkey) }; let mut sighash_parts = sighash::SighashCache::new(justice_tx); diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 1ca3effabd7..6f291bd1da7 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -26,7 +26,7 @@ use ln::PaymentPreimage; use ln::chan_utils::{ChannelTransactionParameters, HolderCommitmentTransaction}; use chain::chaininterface::{FeeEstimator, BroadcasterInterface}; use chain::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER}; -use chain::keysinterface::{Sign, KeysInterface}; +use chain::keysinterface::{Sign, KeysInterface, SignError}; use chain::package::PackageTemplate; use util::logger::Logger; use util::ser::{Readable, ReadableArgs, MaybeReadable, Writer, Writeable, VecWriter}; @@ -377,7 +377,7 @@ impl OnchainTxHandler { /// (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. /// Panics if there are signing errors, because signing operations in reaction to on-chain events /// are not expected to fail, and if they do, we may lose funds. - fn generate_claim_tx(&mut self, cur_height: u32, cached_request: &PackageTemplate, fee_estimator: &F, logger: &L) -> Option<(Option, u64, Transaction)> + fn generate_claim_tx(&mut self, cur_height: u32, cached_request: &PackageTemplate, fee_estimator: &F, logger: &L) -> Option<(Option, u64, u64)> where F::Target: FeeEstimator, L::Target: Logger, { @@ -391,23 +391,34 @@ impl OnchainTxHandler { if let Some((output_value, new_feerate)) = cached_request.compute_package_output(predicted_weight, self.destination_script.dust_value().as_sat(), fee_estimator, logger) { assert!(new_feerate != 0); - - let transaction = cached_request.finalize_package(self, output_value, self.destination_script.clone(), logger).unwrap(); log_trace!(logger, "...with timer {} and feerate {}", new_timer.unwrap(), new_feerate); - assert!(predicted_weight >= transaction.weight()); - return Some((new_timer, new_feerate, transaction)) + return Some((new_timer, new_feerate, output_value)) } } else { // Note: Currently, amounts of holder outputs spending witnesses aren't used // as we can't malleate spending package to increase their feerate. This // should change with the remaining anchor output patchset. - if let Some(transaction) = cached_request.finalize_package(self, 0, self.destination_script.clone(), logger) { - return Some((None, 0, transaction)); - } + return Some((None, 0, 0)); } None } + fn finalize_claim_tx(&mut self, output_value: u64, cached_request: &PackageTemplate, logger: &L) -> Result, SignError> + where L::Target: Logger, + { + let transaction = cached_request.finalize_package(self, output_value, self.destination_script.clone(), logger) + .map_err(|e| { + log_warn!(logger, "Unable to sign claims because signer was not available, will retry"); + e + })?; + if cached_request.is_malleable() { + let predicted_weight = cached_request.package_weight(&self.destination_script, self.channel_transaction_parameters.opt_anchors.is_some()); + // If the request is malleable, a transaction must have been finalized, so the unwrap is safe + assert!(predicted_weight >= transaction.as_ref().unwrap().weight()); + } + Ok(transaction) + } + /// 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 @@ -415,7 +426,7 @@ impl OnchainTxHandler { /// `conf_height` represents the height at which the transactions in `txn_matched` were /// confirmed. This does not need to equal the current blockchain tip height, which should be /// provided via `cur_height`, however it must never be higher than `cur_height`. - pub(crate) fn update_claims_view(&mut self, txn_matched: &[&Transaction], requests: Vec, conf_height: u32, cur_height: u32, broadcaster: &B, fee_estimator: &F, logger: &L) + pub(crate) fn update_claims_view(&mut self, txn_matched: &[&Transaction], requests: Vec, conf_height: u32, cur_height: u32, broadcaster: &B, fee_estimator: &F, logger: &L) -> Result<(), SignError> where B::Target: BroadcasterInterface, F::Target: FeeEstimator, L::Target: Logger, @@ -471,20 +482,15 @@ impl OnchainTxHandler { } self.locktimed_packages = remaining_locked_packages; + let mut claims = Vec::new(); + // Generate claim transactions and track them to bump if necessary at // height timer expiration (i.e in how many blocks we're going to take action). for mut req in preprocessed_requests { - if let Some((new_timer, new_feerate, tx)) = self.generate_claim_tx(cur_height, &req, &*fee_estimator, &*logger) { + if let Some((new_timer, new_feerate, output_value)) = self.generate_claim_tx(cur_height, &req, &*fee_estimator, &*logger) { req.set_timer(new_timer); req.set_feerate(new_feerate); - let txid = tx.txid(); - for k in req.outpoints() { - log_info!(logger, "Registering claiming request for {}:{}", k.txid, k.vout); - self.claimable_outpoints.insert(k.clone(), (txid, conf_height)); - } - self.pending_claim_requests.insert(txid, req); - log_info!(logger, "Broadcasting onchain {}", log_tx!(tx)); - broadcaster.broadcast_transaction(&tx); + claims.push((output_value, req)); } } @@ -563,6 +569,26 @@ impl OnchainTxHandler { } } + for (output_value, mut req) in claims { + match self.finalize_claim_tx(output_value, &req, &*logger) { + Ok(Some(tx)) => { + let txid = tx.txid(); + for k in req.outpoints() { + log_info!(logger, "Registering claiming request for {}:{}", k.txid, k.vout); + // XXX this cannot be reordered to later than previous block because of data dependency - tests are failing + self.claimable_outpoints.insert(k.clone(), (txid, conf_height)); + } + self.pending_claim_requests.insert(txid, req); + log_info!(logger, "Broadcasting onchain {}", log_tx!(tx)); + broadcaster.broadcast_transaction(&tx); + } + Ok(None) => {} + Err(_) => { + req.set_timer(Some(cur_height + 1)); + } + } + } + // After security delay, either our claim tx got enough confs or outpoint is definetely out of reach let onchain_events_awaiting_threshold_conf = self.onchain_events_awaiting_threshold_conf.drain(..).collect::>(); @@ -601,16 +627,29 @@ impl OnchainTxHandler { // Build, bump and rebroadcast tx accordingly log_trace!(logger, "Bumping {} candidates", bump_candidates.len()); - for (first_claim_txid, request) in bump_candidates.iter() { - if let Some((new_timer, new_feerate, bump_tx)) = self.generate_claim_tx(cur_height, &request, &*fee_estimator, &*logger) { - log_info!(logger, "Broadcasting RBF-bumped onchain {}", log_tx!(bump_tx)); - broadcaster.broadcast_transaction(&bump_tx); - if let Some(request) = self.pending_claim_requests.get_mut(first_claim_txid) { - request.set_timer(new_timer); - request.set_feerate(new_feerate); + for (first_claim_txid, req) in bump_candidates.drain() { + if let Some((new_timer, new_feerate, output_value)) = self.generate_claim_tx(cur_height, &req, &*fee_estimator, &*logger) { + match self.finalize_claim_tx(output_value, &req, &*logger) { + Ok(Some(bump_tx)) => { + log_info!(logger, "Broadcasting RBF-bumped onchain {}", log_tx!(bump_tx)); + broadcaster.broadcast_transaction(&bump_tx); + if let Some(request) = self.pending_claim_requests.get_mut(&first_claim_txid) { + request.set_timer(new_timer); + request.set_feerate(new_feerate); + } + } + Ok(None) => {} + Err(_) => { + if let Some(request) = self.pending_claim_requests.get_mut(&first_claim_txid) { + request.set_timer(Some(cur_height + 1)); + } + } } + } } + + Ok(()) } pub(crate) fn transaction_unconfirmed( @@ -648,7 +687,7 @@ impl OnchainTxHandler { for entry in onchain_events_awaiting_threshold_conf { if entry.height >= height { //- our claim tx on a commitment tx output - //- resurect outpoint back in its claimable set and regenerate tx + //- resurrect outpoint back in its claimable set and regenerate tx match entry.event { OnchainEvent::ContentiousOutpoint { package } => { if let Some(ancestor_claimable_txid) = self.claimable_outpoints.get(&package.outpoints()[0]) { @@ -667,11 +706,14 @@ impl OnchainTxHandler { } } for (_, request) in bump_candidates.iter_mut() { - if let Some((new_timer, new_feerate, bump_tx)) = self.generate_claim_tx(height, &request, &&*fee_estimator, &&*logger) { + if let Some((new_timer, new_feerate, output_value)) = self.generate_claim_tx(height, request, &&*fee_estimator, &&*logger) { + let bump_tx = self.finalize_claim_tx(output_value, request, &&*logger).unwrap().unwrap(); request.set_timer(new_timer); request.set_feerate(new_feerate); log_info!(logger, "Broadcasting onchain {}", log_tx!(bump_tx)); broadcaster.broadcast_transaction(&bump_tx); + // log_warn!(logger, "Unable to generate claim tx because signer is unavailable, will retry next block"); + // request.set_timer(Some(height + 1)); } } for (ancestor_claim_txid, request) in bump_candidates.drain() { @@ -682,8 +724,8 @@ impl OnchainTxHandler { let mut remove_request = Vec::new(); self.claimable_outpoints.retain(|_, ref v| if v.1 >= height { - remove_request.push(v.0.clone()); - false + remove_request.push(v.0.clone()); + false } else { true }); for req in remove_request { self.pending_claim_requests.remove(&req); @@ -708,23 +750,25 @@ impl OnchainTxHandler { // Normally holder HTLCs are signed at the same time as the holder commitment tx. However, // in some configurations, the holder commitment tx has been signed and broadcast by a // ChannelMonitor replica, so we handle that case here. - fn sign_latest_holder_htlcs(&mut self) { + fn sign_latest_holder_htlcs(&mut self) -> Result<(), SignError> { if self.holder_htlc_sigs.is_none() { - let (_sig, sigs) = self.signer.sign_holder_commitment_and_htlcs(&self.holder_commitment, &self.secp_ctx).expect("sign holder commitment"); + let (_sig, sigs) = self.signer.sign_holder_commitment_and_htlcs(&self.holder_commitment, &self.secp_ctx)?; self.holder_htlc_sigs = Some(Self::extract_holder_sigs(&self.holder_commitment, sigs)); } + Ok(()) } // Normally only the latest commitment tx and HTLCs need to be signed. However, in some // configurations we may have updated our holder commitment but a replica of the ChannelMonitor // broadcast the previous one before we sync with it. We handle that case here. - fn sign_prev_holder_htlcs(&mut self) { + fn sign_prev_holder_htlcs(&mut self) -> Result<(), SignError> { if self.prev_holder_htlc_sigs.is_none() { if let Some(ref holder_commitment) = self.prev_holder_commitment { - let (_sig, sigs) = self.signer.sign_holder_commitment_and_htlcs(holder_commitment, &self.secp_ctx).expect("sign previous holder commitment"); + let (_sig, sigs) = self.signer.sign_holder_commitment_and_htlcs(holder_commitment, &self.secp_ctx)?; self.prev_holder_htlc_sigs = Some(Self::extract_holder_sigs(holder_commitment, sigs)); } } + Ok(()) } fn extract_holder_sigs(holder_commitment: &HolderCommitmentTransaction, sigs: Vec) -> Vec> { @@ -737,14 +781,14 @@ impl OnchainTxHandler { ret } - //TODO: getting lastest holder transactions should be infallible and result in us "force-closing the channel", but we may + //TODO: getting latest 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 Channel::get_outbound_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, htlc_sigs) = self.signer.sign_holder_commitment_and_htlcs(&self.holder_commitment, &self.secp_ctx).expect("signing holder commitment"); + pub(crate) fn get_fully_signed_holder_tx(&mut self, funding_redeemscript: &Script) -> Result { + let (sig, htlc_sigs) = self.signer.sign_holder_commitment_and_htlcs(&self.holder_commitment, &self.secp_ctx)?; self.holder_htlc_sigs = Some(Self::extract_holder_sigs(&self.holder_commitment, htlc_sigs)); - self.holder_commitment.add_holder_sig(funding_redeemscript, sig) + Ok(self.holder_commitment.add_holder_sig(funding_redeemscript, sig)) } #[cfg(any(test, feature="unsafe_revoked_tx_signing"))] @@ -754,12 +798,12 @@ 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_fully_signed_htlc_tx(&mut self, outp: &::bitcoin::OutPoint, preimage: &Option) -> Result, SignError> { let mut htlc_tx = None; let commitment_txid = self.holder_commitment.trust().txid(); // Check if the HTLC spends from the current holder commitment if commitment_txid == outp.txid { - self.sign_latest_holder_htlcs(); + self.sign_latest_holder_htlcs()?; if let &Some(ref htlc_sigs) = &self.holder_htlc_sigs { let &(ref htlc_idx, ref htlc_sig) = htlc_sigs[outp.vout as usize].as_ref().unwrap(); let trusted_tx = self.holder_commitment.trust(); @@ -772,7 +816,7 @@ impl OnchainTxHandler { if htlc_tx.is_none() && self.prev_holder_commitment.is_some() { let commitment_txid = self.prev_holder_commitment.as_ref().unwrap().trust().txid(); if commitment_txid == outp.txid { - self.sign_prev_holder_htlcs(); + self.sign_prev_holder_htlcs()?; if let &Some(ref htlc_sigs) = &self.prev_holder_htlc_sigs { let &(ref htlc_idx, ref htlc_sig) = htlc_sigs[outp.vout as usize].as_ref().unwrap(); let holder_commitment = self.prev_holder_commitment.as_ref().unwrap(); @@ -783,7 +827,7 @@ impl OnchainTxHandler { } } } - htlc_tx + Ok(htlc_tx) } pub(crate) fn opt_anchors(&self) -> bool { @@ -794,7 +838,7 @@ impl OnchainTxHandler { pub(crate) fn unsafe_get_fully_signed_htlc_tx(&mut self, outp: &::bitcoin::OutPoint, preimage: &Option) -> Option { let latest_had_sigs = self.holder_htlc_sigs.is_some(); let prev_had_sigs = self.prev_holder_htlc_sigs.is_some(); - let ret = self.get_fully_signed_htlc_tx(outp, preimage); + let ret = self.get_fully_signed_htlc_tx(outp, preimage).unwrap(); if !latest_had_sigs { self.holder_htlc_sigs = None; } diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs index b0961293c07..dc8b4fbd199 100644 --- a/lightning/src/chain/package.rs +++ b/lightning/src/chain/package.rs @@ -25,7 +25,7 @@ use ln::chan_utils::{TxCreationKeys, HTLCOutputInCommitment}; use ln::chan_utils; use ln::msgs::DecodeError; use chain::chaininterface::{FeeEstimator, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT}; -use chain::keysinterface::Sign; +use chain::keysinterface::{Sign, SignError}; use chain::onchaintx::OnchainTxHandler; use util::byte_utils; use util::logger::Logger; @@ -346,7 +346,7 @@ impl PackageSolvingData { _ => { mem::discriminant(self) == mem::discriminant(&input) } } } - fn finalize_input(&self, bumped_tx: &mut Transaction, i: usize, onchain_handler: &mut OnchainTxHandler) -> bool { + fn finalize_input(&self, bumped_tx: &mut Transaction, i: usize, onchain_handler: &mut OnchainTxHandler) -> Result { match self { PackageSolvingData::RevokedOutput(ref outp) => { if let Ok(chan_keys) = TxCreationKeys::derive_new(&onchain_handler.secp_ctx, &outp.per_commitment_point, &outp.counterparty_delayed_payment_base_key, &outp.counterparty_htlc_base_key, &onchain_handler.signer.pubkeys().revocation_basepoint, &onchain_handler.signer.pubkeys().htlc_basepoint) { @@ -358,7 +358,7 @@ impl PackageSolvingData { bumped_tx.input[i].witness.push(ser_sig); bumped_tx.input[i].witness.push(vec!(1)); bumped_tx.input[i].witness.push(witness_script.clone().into_bytes()); - } else { return false; } + } else { return Ok(false); } } }, PackageSolvingData::RevokedHTLCOutput(ref outp) => { @@ -371,7 +371,7 @@ impl PackageSolvingData { bumped_tx.input[i].witness.push(ser_sig); bumped_tx.input[i].witness.push(chan_keys.revocation_key.clone().serialize().to_vec()); bumped_tx.input[i].witness.push(witness_script.clone().into_bytes()); - } else { return false; } + } else { return Ok(false); } } }, PackageSolvingData::CounterpartyOfferedHTLCOutput(ref outp) => { @@ -404,12 +404,12 @@ impl PackageSolvingData { }, _ => { panic!("API Error!"); } } - true + Ok(true) } - fn get_finalized_tx(&self, outpoint: &BitcoinOutPoint, onchain_handler: &mut OnchainTxHandler) -> Option { + fn get_finalized_tx(&self, outpoint: &BitcoinOutPoint, onchain_handler: &mut OnchainTxHandler) -> Result, SignError> { match self { PackageSolvingData::HolderHTLCOutput(ref outp) => { return onchain_handler.get_fully_signed_htlc_tx(outpoint, &outp.preimage); } - PackageSolvingData::HolderFundingOutput(ref outp) => { return Some(onchain_handler.get_fully_signed_holder_tx(&outp.funding_redeemscript)); } + PackageSolvingData::HolderFundingOutput(ref outp) => { return Ok(Some(onchain_handler.get_fully_signed_holder_tx(&outp.funding_redeemscript)?)); } _ => { panic!("API Error!"); } } } @@ -606,7 +606,7 @@ impl PackageTemplate { let output_weight = (8 + 1 + destination_script.len()) * WITNESS_SCALE_FACTOR; inputs_weight + witnesses_weight + transaction_weight + output_weight } - pub(crate) fn finalize_package(&self, onchain_handler: &mut OnchainTxHandler, value: u64, destination_script: Script, logger: &L) -> Option + pub(crate) fn finalize_package(&self, onchain_handler: &mut OnchainTxHandler, value: u64, destination_script: Script, logger: &L) -> Result, SignError> where L::Target: Logger, { match self.malleability { @@ -630,20 +630,20 @@ impl PackageTemplate { } for (i, (outpoint, out)) in self.inputs.iter().enumerate() { log_debug!(logger, "Adding claiming input for outpoint {}:{}", outpoint.txid, outpoint.vout); - if !out.finalize_input(&mut bumped_tx, i, onchain_handler) { return None; } + if !out.finalize_input(&mut bumped_tx, i, onchain_handler)? { return Ok(None); } } log_debug!(logger, "Finalized transaction {} ready to broadcast", bumped_tx.txid()); - return Some(bumped_tx); + return Ok(Some(bumped_tx)); }, PackageMalleability::Untractable => { debug_assert_eq!(value, 0, "value is ignored for non-malleable packages, should be zero to ensure callsites are correct"); if let Some((outpoint, outp)) = self.inputs.first() { - if let Some(final_tx) = outp.get_finalized_tx(outpoint, onchain_handler) { + if let Some(final_tx) = outp.get_finalized_tx(outpoint, onchain_handler)? { log_debug!(logger, "Adding claiming input for outpoint {}:{}", outpoint.txid, outpoint.vout); log_debug!(logger, "Finalized transaction {} ready to broadcast", final_tx.txid()); - return Some(final_tx); + return Ok(Some(final_tx)); } - return None; + return Ok(None); } else { panic!("API Error: Package must not be inputs empty"); } }, } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 3fa136e9af8..f9c869f0f8f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -7015,7 +7015,8 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> channel.claim_htlc_while_disconnected_dropping_mon_update(claimable_htlc.prev_hop.htlc_id, payment_preimage, &args.logger); } if let Some(previous_hop_monitor) = args.channel_monitors.get(&claimable_htlc.prev_hop.outpoint) { - previous_hop_monitor.provide_payment_preimage(&payment_hash, &payment_preimage, &args.tx_broadcaster, &args.fee_estimator, &args.logger); + // FIXME + previous_hop_monitor.provide_payment_preimage(&payment_hash, &payment_preimage, &args.tx_broadcaster, &args.fee_estimator, &args.logger).unwrap(); } } pending_events_read.push(events::Event::PaymentClaimed { diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 3298db0dbd4..438173082fb 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -3473,7 +3473,7 @@ fn test_force_close_fail_back() { // Now check that if we add the preimage to ChannelMonitor it broadcasts our HTLC-Success.. { get_monitor!(nodes[2], payment_event.commitment_msg.channel_id) - .provide_payment_preimage(&our_payment_hash, &our_payment_preimage, &node_cfgs[2].tx_broadcaster, &node_cfgs[2].fee_estimator, &node_cfgs[2].logger); + .provide_payment_preimage(&our_payment_hash, &our_payment_preimage, &node_cfgs[2].tx_broadcaster, &node_cfgs[2].fee_estimator, &node_cfgs[2].logger).unwrap(); } mine_transaction(&nodes[2], &tx); let node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap(); diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index b4b66e8f5fe..ccd249cbef1 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -9,7 +9,7 @@ use ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, HolderCommitmentTransaction, CommitmentTransaction, ChannelTransactionParameters, TrustedCommitmentTransaction, ClosingTransaction}; use ln::{chan_utils, msgs, PaymentPreimage}; -use chain::keysinterface::{Sign, InMemorySigner, BaseSign}; +use chain::keysinterface::{Sign, InMemorySigner, BaseSign, SignError}; use prelude::*; use core::cmp; @@ -139,7 +139,7 @@ impl BaseSign for EnforcingSigner { Ok(()) } - fn sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { + fn sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), SignError> { let trusted_tx = self.verify_holder_commitment_tx(commitment_tx, secp_ctx); let commitment_txid = trusted_tx.txid(); let holder_csv = self.inner.counterparty_selected_contest_delay(); @@ -164,7 +164,7 @@ impl BaseSign for EnforcingSigner { secp_ctx.verify_ecdsa(&sighash, sig, &keys.countersignatory_htlc_key).unwrap(); } - Ok(self.inner.sign_holder_commitment_and_htlcs(commitment_tx, secp_ctx).unwrap()) + self.inner.sign_holder_commitment_and_htlcs(commitment_tx, secp_ctx) } #[cfg(any(test,feature = "unsafe_revoked_tx_signing"))] @@ -172,11 +172,11 @@ impl BaseSign for EnforcingSigner { Ok(self.inner.unsafe_sign_holder_commitment_and_htlcs(commitment_tx, secp_ctx).unwrap()) } - fn sign_justice_revoked_output(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1) -> Result { + fn sign_justice_revoked_output(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1) -> Result { Ok(self.inner.sign_justice_revoked_output(justice_tx, input, amount, per_commitment_key, secp_ctx).unwrap()) } - fn sign_justice_revoked_htlc(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1) -> Result { + fn sign_justice_revoked_htlc(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1) -> Result { Ok(self.inner.sign_justice_revoked_htlc(justice_tx, input, amount, per_commitment_key, htlc, secp_ctx).unwrap()) }