X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannel.rs;h=94ce779ff895f15ffc3836025d03b9fb607d1f65;hb=d7ff37edb4fc52023d7e96b725fa586d2f64e179;hp=4051ebf3fbf6cc0d07dd29c4132c2dab3510bb43;hpb=0c57018f2fb5618f976542a4d24adee29cf49c96;p=rust-lightning diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 4051ebf3..94ce779f 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -9,15 +9,15 @@ use bitcoin::blockdata::script::{Script,Builder}; use bitcoin::blockdata::transaction::{TxIn, TxOut, Transaction, SigHashType}; -use bitcoin::blockdata::opcodes; use bitcoin::util::bip143; use bitcoin::consensus::encode; use bitcoin::hashes::Hash; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::sha256d::Hash as Sha256d; -use bitcoin::hash_types::{Txid, BlockHash, WPubkeyHash}; +use bitcoin::hash_types::{Txid, BlockHash}; +use bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE; use bitcoin::secp256k1::key::{PublicKey,SecretKey}; use bitcoin::secp256k1::{Secp256k1,Signature}; use bitcoin::secp256k1; @@ -26,9 +26,11 @@ use ln::{PaymentPreimage, PaymentHash}; use ln::features::{ChannelFeatures, InitFeatures}; use ln::msgs; use ln::msgs::{DecodeError, OptionalField, DataLossProtect}; -use ln::channelmanager::{BestBlock, PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT}; +use ln::script::ShutdownScript; +use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT}; use ln::chan_utils::{CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS, get_commitment_transaction_number_obscure_factor}; use ln::chan_utils; +use chain::BestBlock; use chain::chaininterface::{FeeEstimator,ConfirmationTarget}; use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER}; use chain::transaction::{OutPoint, TransactionData}; @@ -42,11 +44,11 @@ use util::scid_utils::scid_from_parts; use prelude::*; use core::{cmp,mem,fmt}; +use core::convert::TryFrom; use core::ops::Deref; -#[cfg(any(test, feature = "fuzztarget"))] -use std::sync::Mutex; +#[cfg(any(test, feature = "fuzztarget", debug_assertions))] +use sync::Mutex; use bitcoin::hashes::hex::ToHex; -use bitcoin::blockdata::opcodes::all::OP_PUSHBYTES_0; #[cfg(test)] pub struct ChannelValueStat { @@ -300,6 +302,33 @@ pub struct CounterpartyForwardingInfo { pub cltv_expiry_delta: u16, } +/// A return value enum for get_update_fulfill_htlc. See UpdateFulfillCommitFetch variants for +/// description +enum UpdateFulfillFetch { + NewClaim { + monitor_update: ChannelMonitorUpdate, + msg: Option, + }, + DuplicateClaim {}, +} + +/// The return type of get_update_fulfill_htlc_and_commit. +pub enum UpdateFulfillCommitFetch { + /// Indicates the HTLC fulfill is new, and either generated an update_fulfill message, placed + /// it in the holding cell, or re-generated the update_fulfill message after the same claim was + /// previously placed in the holding cell (and has since been removed). + NewClaim { + /// The ChannelMonitorUpdate which places the new payment preimage in the channel monitor + monitor_update: ChannelMonitorUpdate, + /// The update_fulfill message and commitment_signed message (if the claim was not placed + /// in the holding cell). + msgs: Option<(msgs::UpdateFulfillHTLC, msgs::CommitmentSigned)>, + }, + /// Indicates the HTLC fulfill is duplicative and already existed either in the holding cell + /// or has been forgotten (presumably previously claimed). + DuplicateClaim {}, +} + // TODO: We should refactor this to be an Inbound/OutboundChannel until initial setup handshaking // has been completed, and then turn into a Channel to get compiler-time enforcement of things like // calling channel_id() before we're set up or things like get_outbound_funding_signed on an @@ -320,7 +349,7 @@ pub(super) struct Channel { latest_monitor_update_id: u64, holder_signer: Signer, - shutdown_pubkey: PublicKey, + shutdown_scriptpubkey: Option, destination_script: Script, // Our commitment numbers start at 2^48-1 and count down, whereas the ones used in transaction @@ -373,10 +402,10 @@ pub(super) struct Channel { #[cfg(debug_assertions)] /// Max to_local and to_remote outputs in a locally-generated commitment transaction - holder_max_commitment_tx_output: ::std::sync::Mutex<(u64, u64)>, + holder_max_commitment_tx_output: Mutex<(u64, u64)>, #[cfg(debug_assertions)] /// Max to_local and to_remote outputs in a remote-generated commitment transaction - counterparty_max_commitment_tx_output: ::std::sync::Mutex<(u64, u64)>, + counterparty_max_commitment_tx_output: Mutex<(u64, u64)>, last_sent_closing_fee: Option<(u32, u64, Signature)>, // (feerate, fee, holder_sig) @@ -396,7 +425,7 @@ pub(super) struct Channel { counterparty_max_htlc_value_in_flight_msat: u64, //get_holder_max_htlc_value_in_flight_msat(): u64, /// minimum channel reserve for self to maintain - set by them. - counterparty_selected_channel_reserve_satoshis: u64, + counterparty_selected_channel_reserve_satoshis: Option, // get_holder_selected_channel_reserve_satoshis(channel_value_sats: u64): u64 counterparty_htlc_minimum_msat: u64, holder_htlc_minimum_msat: u64, @@ -405,7 +434,7 @@ pub(super) struct Channel { #[cfg(not(test))] counterparty_max_accepted_htlcs: u16, //implied by OUR_MAX_HTLCS: max_accepted_htlcs: u16, - minimum_depth: u32, + minimum_depth: Option, counterparty_forwarding_info: Option, @@ -443,6 +472,15 @@ pub(super) struct Channel { /// /// See-also pub workaround_lnd_bug_4006: Option, + + #[cfg(any(test, feature = "fuzztarget"))] + // When we receive an HTLC fulfill on an outbound path, we may immediately fulfill the + // corresponding HTLC on the inbound path. If, then, the outbound path channel is + // disconnected and reconnected (before we've exchange commitment_signed and revoke_and_ack + // messages), they may re-broadcast their update_fulfill_htlc, causing a duplicate claim. This + // is fine, but as a sanity check in our failure to generate the second claim, we check here + // that the original was a claim, and that we aren't now trying to fulfill a failed HTLC. + historical_inbound_htlc_fulfills: HashSet, } #[cfg(any(test, feature = "fuzztarget"))] @@ -455,7 +493,6 @@ struct CommitmentTxInfoCached { } pub const OUR_MAX_HTLCS: u16 = 50; //TODO -const SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT: u64 = 79; // prevout: 36, nSequence: 4, script len: 1, witness lengths: (3+1)/4, sig: 73/4, if-selector: 1, redeemScript: (6 ops + 2*33 pubkeys + 1*2 delay)/4 #[cfg(not(test))] const COMMITMENT_TX_BASE_WEIGHT: u64 = 724; @@ -530,7 +567,7 @@ impl Channel { } // Constructors: - pub fn new_outbound(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig) -> Result, APIError> + pub fn new_outbound(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: InitFeatures, channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig) -> Result, APIError> where K::Target: KeysInterface, F::Target: FeeEstimator, { @@ -558,6 +595,16 @@ impl Channel { let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes()); + let shutdown_scriptpubkey = if config.channel_options.commit_upfront_shutdown_pubkey { + Some(keys_provider.get_shutdown_scriptpubkey()) + } else { None }; + + if let Some(shutdown_scriptpubkey) = &shutdown_scriptpubkey { + if !shutdown_scriptpubkey.is_compatible(&their_features) { + return Err(APIError::APIMisuseError { err: format!("Provided a scriptpubkey format not accepted by peer. script: ({})", shutdown_scriptpubkey.clone().into_inner().to_bytes().to_hex()) }); + } + } + Ok(Channel { user_id, config: config.channel_options.clone(), @@ -570,7 +617,7 @@ impl Channel { latest_monitor_update_id: 0, holder_signer, - shutdown_pubkey: keys_provider.get_shutdown_pubkey(), + shutdown_scriptpubkey, destination_script: keys_provider.get_destination_script(), cur_holder_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER, @@ -595,9 +642,9 @@ impl Channel { monitor_pending_failures: Vec::new(), #[cfg(debug_assertions)] - holder_max_commitment_tx_output: ::std::sync::Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)), + holder_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)), #[cfg(debug_assertions)] - counterparty_max_commitment_tx_output: ::std::sync::Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)), + counterparty_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)), last_sent_closing_fee: None, @@ -609,11 +656,11 @@ impl Channel { counterparty_dust_limit_satoshis: 0, holder_dust_limit_satoshis: MIN_DUST_LIMIT_SATOSHIS, counterparty_max_htlc_value_in_flight_msat: 0, - counterparty_selected_channel_reserve_satoshis: 0, + counterparty_selected_channel_reserve_satoshis: None, // Filled in in accept_channel counterparty_htlc_minimum_msat: 0, holder_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat }, counterparty_max_accepted_htlcs: 0, - minimum_depth: 0, // Filled in in accept_channel + minimum_depth: None, // Filled in in accept_channel counterparty_forwarding_info: None, @@ -644,6 +691,9 @@ impl Channel { next_remote_commitment_tx_fee_info_cached: Mutex::new(None), workaround_lnd_bug_4006: None, + + #[cfg(any(test, feature = "fuzztarget"))] + historical_inbound_htlc_fulfills: HashSet::new(), }) } @@ -783,11 +833,11 @@ impl Channel { // Peer is signaling upfront_shutdown and has opt-out with a 0-length script. We don't enforce anything if script.len() == 0 { None - // Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. Fail the channel - } else if is_unsupported_shutdown_script(&their_features, script) { - return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: ({})", script.to_bytes().to_hex()))); } else { - Some(script.clone()) + match ShutdownScript::try_from((script.clone(), &their_features)) { + Ok(shutdown_script) => Some(shutdown_script.into_inner()), + Err(_) => return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: ({})", script.to_bytes().to_hex()))), + } } }, // Peer is signaling upfront shutdown but don't opt-out with correct mechanism (a.k.a 0-length script). Peer looks buggy, we fail the channel @@ -797,6 +847,16 @@ impl Channel { } } else { None }; + let shutdown_scriptpubkey = if config.channel_options.commit_upfront_shutdown_pubkey { + Some(keys_provider.get_shutdown_scriptpubkey()) + } else { None }; + + if let Some(shutdown_scriptpubkey) = &shutdown_scriptpubkey { + if !shutdown_scriptpubkey.is_compatible(&their_features) { + return Err(ChannelError::Close(format!("Provided a scriptpubkey format not accepted by peer. script: ({})", shutdown_scriptpubkey.clone().into_inner().to_bytes().to_hex()))); + } + } + let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes()); @@ -811,7 +871,7 @@ impl Channel { latest_monitor_update_id: 0, holder_signer, - shutdown_pubkey: keys_provider.get_shutdown_pubkey(), + shutdown_scriptpubkey, destination_script: keys_provider.get_destination_script(), cur_holder_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER, @@ -836,9 +896,9 @@ impl Channel { monitor_pending_failures: Vec::new(), #[cfg(debug_assertions)] - holder_max_commitment_tx_output: ::std::sync::Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)), + holder_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)), #[cfg(debug_assertions)] - counterparty_max_commitment_tx_output: ::std::sync::Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)), + counterparty_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)), last_sent_closing_fee: None, @@ -851,11 +911,11 @@ impl Channel { counterparty_dust_limit_satoshis: msg.dust_limit_satoshis, holder_dust_limit_satoshis: MIN_DUST_LIMIT_SATOSHIS, counterparty_max_htlc_value_in_flight_msat: cmp::min(msg.max_htlc_value_in_flight_msat, msg.funding_satoshis * 1000), - counterparty_selected_channel_reserve_satoshis: msg.channel_reserve_satoshis, + counterparty_selected_channel_reserve_satoshis: Some(msg.channel_reserve_satoshis), counterparty_htlc_minimum_msat: msg.htlc_minimum_msat, holder_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat }, counterparty_max_accepted_htlcs: msg.max_accepted_htlcs, - minimum_depth: config.own_channel_config.minimum_depth, + minimum_depth: Some(config.own_channel_config.minimum_depth), counterparty_forwarding_info: None, @@ -889,6 +949,9 @@ impl Channel { next_remote_commitment_tx_fee_info_cached: Mutex::new(None), workaround_lnd_bug_4006: None, + + #[cfg(any(test, feature = "fuzztarget"))] + historical_inbound_htlc_fulfills: HashSet::new(), }; Ok(chan) @@ -1036,7 +1099,7 @@ impl Channel { } else { self.counterparty_max_commitment_tx_output.lock().unwrap() }; - debug_assert!(broadcaster_max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= self.counterparty_selected_channel_reserve_satoshis as i64); + debug_assert!(broadcaster_max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= self.counterparty_selected_channel_reserve_satoshis.unwrap() as i64); broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, value_to_self_msat as u64); debug_assert!(broadcaster_max_commitment_tx_output.1 <= value_to_remote_msat as u64 || value_to_remote_msat / 1000 >= Channel::::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) as i64); broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat as u64); @@ -1087,8 +1150,7 @@ impl Channel { #[inline] fn get_closing_scriptpubkey(&self) -> Script { - let channel_close_key_hash = WPubkeyHash::hash(&self.shutdown_pubkey.serialize()); - Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&channel_close_key_hash[..]).into_script() + self.shutdown_scriptpubkey.clone().unwrap().into_inner() } #[inline] @@ -1154,6 +1216,7 @@ impl Channel { }, ())); } + assert!(self.shutdown_scriptpubkey.is_some()); if value_to_self as u64 > self.holder_dust_limit_satoshis { txouts.push((TxOut { script_pubkey: self.get_closing_scriptpubkey(), @@ -1216,20 +1279,7 @@ impl Channel { make_funding_redeemscript(&self.get_holder_pubkeys().funding_pubkey, self.counterparty_funding_pubkey()) } - /// Builds the htlc-success or htlc-timeout transaction which spends a given HTLC output - /// @local is used only to convert relevant internal structures which refer to remote vs local - /// to decide value of outputs and direction of HTLCs. - fn build_htlc_transaction(&self, prev_hash: &Txid, htlc: &HTLCOutputInCommitment, local: bool, keys: &TxCreationKeys, feerate_per_kw: u32) -> Transaction { - chan_utils::build_htlc_transaction(prev_hash, feerate_per_kw, if local { self.get_counterparty_selected_contest_delay() } else { self.get_holder_selected_contest_delay() }, htlc, &keys.broadcaster_delayed_payment_key, &keys.revocation_key) - } - - /// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made. - /// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always - /// return Ok(_) if debug assertions are turned on or preconditions are met. - /// - /// Note that it is still possible to hit these assertions in case we find a preimage on-chain - /// but then have a reorg which settles on an HTLC-failure on chain. - fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage, logger: &L) -> Result<(Option, Option), ChannelError> where L::Target: Logger { + fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage, logger: &L) -> UpdateFulfillFetch where L::Target: Logger { // Either ChannelFunded got set (which means it won't be unset) or there is no way any // caller thought we could have something claimed (cause we wouldn't have accepted in an // incoming HTLC anyway). If we got to ShutdownComplete, callers aren't allowed to call us, @@ -1255,9 +1305,9 @@ impl Channel { if let &InboundHTLCRemovalReason::Fulfill(_) = reason { } else { log_warn!(logger, "Have preimage and want to fulfill HTLC with payment hash {} we already failed against channel {}", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id())); + debug_assert!(false, "Tried to fulfill an HTLC that was already failed"); } - debug_assert!(false, "Tried to fulfill an HTLC that was already fail/fulfilled"); - return Ok((None, None)); + return UpdateFulfillFetch::DuplicateClaim {}; }, _ => { debug_assert!(false, "Have an inbound HTLC we tried to claim before it was fully committed to"); @@ -1269,7 +1319,11 @@ impl Channel { } } if pending_idx == core::usize::MAX { - return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned())); + #[cfg(any(test, feature = "fuzztarget"))] + // If we failed to find an HTLC to fulfill, make sure it was previously fulfilled and + // this is simply a duplicate claim, not previously failed and we lost funds. + debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg)); + return UpdateFulfillFetch::DuplicateClaim {}; } // Now update local state: @@ -1291,8 +1345,9 @@ impl Channel { if htlc_id_arg == htlc_id { // Make sure we don't leave latest_monitor_update_id incremented here: self.latest_monitor_update_id -= 1; - debug_assert!(false, "Tried to fulfill an HTLC that was already fulfilled"); - return Ok((None, None)); + #[cfg(any(test, feature = "fuzztarget"))] + debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg)); + return UpdateFulfillFetch::DuplicateClaim {}; } }, &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => { @@ -1301,7 +1356,7 @@ impl Channel { // TODO: We may actually be able to switch to a fulfill here, though its // rare enough it may not be worth the complexity burden. debug_assert!(false, "Tried to fulfill an HTLC that was already failed"); - return Ok((None, Some(monitor_update))); + return UpdateFulfillFetch::NewClaim { monitor_update, msg: None }; } }, _ => {} @@ -1311,52 +1366,58 @@ impl Channel { self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::ClaimHTLC { payment_preimage: payment_preimage_arg, htlc_id: htlc_id_arg, }); - return Ok((None, Some(monitor_update))); + #[cfg(any(test, feature = "fuzztarget"))] + self.historical_inbound_htlc_fulfills.insert(htlc_id_arg); + return UpdateFulfillFetch::NewClaim { monitor_update, msg: None }; } + #[cfg(any(test, feature = "fuzztarget"))] + self.historical_inbound_htlc_fulfills.insert(htlc_id_arg); { let htlc = &mut self.pending_inbound_htlcs[pending_idx]; if let InboundHTLCState::Committed = htlc.state { } else { debug_assert!(false, "Have an inbound HTLC we tried to claim before it was fully committed to"); - return Ok((None, Some(monitor_update))); + return UpdateFulfillFetch::NewClaim { monitor_update, msg: None }; } log_trace!(logger, "Upgrading HTLC {} to LocalRemoved with a Fulfill in channel {}!", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id)); htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(payment_preimage_arg.clone())); } - Ok((Some(msgs::UpdateFulfillHTLC { - channel_id: self.channel_id(), - htlc_id: htlc_id_arg, - payment_preimage: payment_preimage_arg, - }), Some(monitor_update))) + UpdateFulfillFetch::NewClaim { + monitor_update, + msg: Some(msgs::UpdateFulfillHTLC { + channel_id: self.channel_id(), + htlc_id: htlc_id_arg, + payment_preimage: payment_preimage_arg, + }), + } } - pub fn get_update_fulfill_htlc_and_commit(&mut self, htlc_id: u64, payment_preimage: PaymentPreimage, logger: &L) -> Result<(Option<(msgs::UpdateFulfillHTLC, msgs::CommitmentSigned)>, Option), ChannelError> where L::Target: Logger { - match self.get_update_fulfill_htlc(htlc_id, payment_preimage, logger)? { - (Some(update_fulfill_htlc), Some(mut monitor_update)) => { - let (commitment, mut additional_update) = self.send_commitment_no_status_check(logger)?; + pub fn get_update_fulfill_htlc_and_commit(&mut self, htlc_id: u64, payment_preimage: PaymentPreimage, logger: &L) -> Result where L::Target: Logger { + match self.get_update_fulfill_htlc(htlc_id, payment_preimage, logger) { + UpdateFulfillFetch::NewClaim { mut monitor_update, msg: Some(update_fulfill_htlc) } => { + let (commitment, mut additional_update) = match self.send_commitment_no_status_check(logger) { + Err(e) => return Err((e, monitor_update)), + Ok(res) => res + }; // send_commitment_no_status_check may bump latest_monitor_id but we want them to be // strictly increasing by one, so decrement it here. self.latest_monitor_update_id = monitor_update.update_id; monitor_update.updates.append(&mut additional_update.updates); - Ok((Some((update_fulfill_htlc, commitment)), Some(monitor_update))) - }, - (Some(update_fulfill_htlc), None) => { - let (commitment, monitor_update) = self.send_commitment_no_status_check(logger)?; - Ok((Some((update_fulfill_htlc, commitment)), Some(monitor_update))) + Ok(UpdateFulfillCommitFetch::NewClaim { monitor_update, msgs: Some((update_fulfill_htlc, commitment)) }) }, - (None, Some(monitor_update)) => Ok((None, Some(monitor_update))), - (None, None) => Ok((None, None)) + UpdateFulfillFetch::NewClaim { monitor_update, msg: None } => Ok(UpdateFulfillCommitFetch::NewClaim { monitor_update, msgs: None }), + UpdateFulfillFetch::DuplicateClaim {} => Ok(UpdateFulfillCommitFetch::DuplicateClaim {}), } } - /// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made. - /// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always - /// return Ok(_) if debug assertions are turned on or preconditions are met. - /// - /// Note that it is still possible to hit these assertions in case we find a preimage on-chain - /// but then have a reorg which settles on an HTLC-failure on chain. + /// We can only have one resolution per HTLC. In some cases around reconnect, we may fulfill + /// an HTLC more than once or fulfill once and then attempt to fail after reconnect. We cannot, + /// however, fail more than once as we wait for an upstream failure to be irrevocably committed + /// before we fail backwards. + /// If we do fail twice, we debug_assert!(false) and return Ok(None). Thus, will always return + /// Ok(_) if debug assertions are turned on or preconditions are met. pub fn get_update_fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket, logger: &L) -> Result, ChannelError> where L::Target: Logger { if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) { panic!("Was asked to fail an HTLC when channel was not in an operational state"); @@ -1372,8 +1433,11 @@ impl Channel { if htlc.htlc_id == htlc_id_arg { match htlc.state { InboundHTLCState::Committed => {}, - InboundHTLCState::LocalRemoved(_) => { - debug_assert!(false, "Tried to fail an HTLC that was already fail/fulfilled"); + InboundHTLCState::LocalRemoved(ref reason) => { + if let &InboundHTLCRemovalReason::Fulfill(_) = reason { + } else { + debug_assert!(false, "Tried to fail an HTLC that was already failed"); + } return Ok(None); }, _ => { @@ -1385,7 +1449,11 @@ impl Channel { } } if pending_idx == core::usize::MAX { - return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned())); + #[cfg(any(test, feature = "fuzztarget"))] + // If we failed to find an HTLC to fail, make sure it was previously fulfilled and this + // is simply a duplicate fail, not previously failed and we failed-back too early. + debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg)); + return Ok(None); } // Now update local state: @@ -1394,8 +1462,9 @@ impl Channel { match pending_update { &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => { if htlc_id_arg == htlc_id { - debug_assert!(false, "Tried to fail an HTLC that was already fulfilled"); - return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned())); + #[cfg(any(test, feature = "fuzztarget"))] + debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg)); + return Ok(None); } }, &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => { @@ -1488,6 +1557,12 @@ impl Channel { if msg.minimum_depth > config.peer_channel_config_limits.max_minimum_depth { return Err(ChannelError::Close(format!("We consider the minimum depth to be unreasonably large. Expected minimum: ({}). Actual: ({})", config.peer_channel_config_limits.max_minimum_depth, msg.minimum_depth))); } + if msg.minimum_depth == 0 { + // Note that if this changes we should update the serialization minimum version to + // indicate to older clients that they don't understand some features of the current + // channel. + return Err(ChannelError::Close("Minimum confirmation depth must be at least 1".to_owned())); + } let counterparty_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() { match &msg.shutdown_scriptpubkey { @@ -1495,11 +1570,11 @@ impl Channel { // Peer is signaling upfront_shutdown and has opt-out with a 0-length script. We don't enforce anything if script.len() == 0 { None - // Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. Fail the channel - } else if is_unsupported_shutdown_script(&their_features, script) { - return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: ({})", script.to_bytes().to_hex()))); } else { - Some(script.clone()) + match ShutdownScript::try_from((script.clone(), &their_features)) { + Ok(shutdown_script) => Some(shutdown_script.into_inner()), + Err(_) => return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: ({})", script.to_bytes().to_hex()))), + } } }, // Peer is signaling upfront shutdown but don't opt-out with correct mechanism (a.k.a 0-length script). Peer looks buggy, we fail the channel @@ -1511,10 +1586,10 @@ impl Channel { self.counterparty_dust_limit_satoshis = msg.dust_limit_satoshis; self.counterparty_max_htlc_value_in_flight_msat = cmp::min(msg.max_htlc_value_in_flight_msat, self.channel_value_satoshis * 1000); - self.counterparty_selected_channel_reserve_satoshis = msg.channel_reserve_satoshis; + self.counterparty_selected_channel_reserve_satoshis = Some(msg.channel_reserve_satoshis); self.counterparty_htlc_minimum_msat = msg.htlc_minimum_msat; self.counterparty_max_accepted_htlcs = msg.max_accepted_htlcs; - self.minimum_depth = msg.minimum_depth; + self.minimum_depth = Some(msg.minimum_depth); let counterparty_pubkeys = ChannelPublicKeys { funding_pubkey: msg.funding_pubkey, @@ -1621,8 +1696,9 @@ impl Channel { let funding_redeemscript = self.get_funding_redeemscript(); let funding_txo_script = funding_redeemscript.to_v0_p2wsh(); let obscure_factor = get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound()); + let shutdown_script = self.shutdown_scriptpubkey.clone().map(|script| script.into_inner()); let channel_monitor = ChannelMonitor::new(self.secp_ctx.clone(), self.holder_signer.clone(), - &self.shutdown_pubkey, self.get_holder_selected_contest_delay(), + shutdown_script, self.get_holder_selected_contest_delay(), &self.destination_script, (funding_txo, funding_txo_script.clone()), &self.channel_transaction_parameters, funding_redeemscript.clone(), self.channel_value_satoshis, @@ -1694,8 +1770,9 @@ impl Channel { let funding_txo = self.get_funding_txo().unwrap(); let funding_txo_script = funding_redeemscript.to_v0_p2wsh(); let obscure_factor = get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound()); + let shutdown_script = self.shutdown_scriptpubkey.clone().map(|script| script.into_inner()); let channel_monitor = ChannelMonitor::new(self.secp_ctx.clone(), self.holder_signer.clone(), - &self.shutdown_pubkey, self.get_holder_selected_contest_delay(), + shutdown_script, self.get_holder_selected_contest_delay(), &self.destination_script, (funding_txo, funding_txo_script), &self.channel_transaction_parameters, funding_redeemscript.clone(), self.channel_value_satoshis, @@ -1785,8 +1862,22 @@ impl Channel { /// corner case properly. pub fn get_inbound_outbound_available_balance_msat(&self) -> (u64, u64) { // Note that we have to handle overflow due to the above case. - (cmp::max(self.channel_value_satoshis as i64 * 1000 - self.value_to_self_msat as i64 - self.get_inbound_pending_htlc_stats().1 as i64, 0) as u64, - cmp::max(self.value_to_self_msat as i64 - self.get_outbound_pending_htlc_stats().1 as i64, 0) as u64) + ( + cmp::max(self.channel_value_satoshis as i64 * 1000 + - self.value_to_self_msat as i64 + - self.get_inbound_pending_htlc_stats().1 as i64 + - Self::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) as i64 * 1000, + 0) as u64, + cmp::max(self.value_to_self_msat as i64 + - self.get_outbound_pending_htlc_stats().1 as i64 + - self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) as i64 * 1000, + 0) as u64 + ) + } + + pub fn get_holder_counterparty_selected_channel_reserve_satoshis(&self) -> (u64, Option) { + (Self::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis), + self.counterparty_selected_channel_reserve_satoshis) } // Get the fee cost of a commitment tx with a given number of HTLC outputs. @@ -2064,7 +2155,7 @@ impl Channel { // Check that they won't violate our local required channel reserve by adding this HTLC. let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered); let local_commit_tx_fee_msat = self.next_local_commit_tx_fee_msat(htlc_candidate, None); - if self.value_to_self_msat < self.counterparty_selected_channel_reserve_satoshis * 1000 + local_commit_tx_fee_msat { + if self.value_to_self_msat < self.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat { return Err(ChannelError::Close("Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_owned())); } } @@ -2236,7 +2327,10 @@ impl Channel { let mut htlcs_and_sigs = Vec::with_capacity(htlcs_cloned.len()); for (idx, (htlc, source)) in htlcs_cloned.drain(..).enumerate() { if let Some(_) = htlc.transaction_output_index { - let htlc_tx = self.build_htlc_transaction(&commitment_txid, &htlc, true, &keys, feerate_per_kw); + let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, feerate_per_kw, + self.get_counterparty_selected_contest_delay().unwrap(), &htlc, + &keys.broadcaster_delayed_payment_key, &keys.revocation_key); + let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys); let htlc_sighash = hash_to_message!(&bip143::SigHashCache::new(&htlc_tx).signature_hash(0, &htlc_redeemscript, htlc.amount_msat / 1000, SigHashType::All)[..]); log_trace!(logger, "Checking HTLC tx signature {} by key {} against tx {} (sighash {}) with redeemscript {} in channel {}.", @@ -2419,24 +2513,28 @@ impl Channel { } }, &HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_preimage, htlc_id, .. } => { - match self.get_update_fulfill_htlc(htlc_id, *payment_preimage, logger) { - Ok((update_fulfill_msg_option, additional_monitor_update_opt)) => { - update_fulfill_htlcs.push(update_fulfill_msg_option.unwrap()); - if let Some(mut additional_monitor_update) = additional_monitor_update_opt { - monitor_update.updates.append(&mut additional_monitor_update.updates); - } - }, - Err(e) => { - if let ChannelError::Ignore(_) = e {} - else { - panic!("Got a non-IgnoreError action trying to fulfill holding cell HTLC"); - } - } - } + // If an HTLC claim was previously added to the holding cell (via + // `get_update_fulfill_htlc`, then generating the claim message itself must + // not fail - any in between attempts to claim the HTLC will have resulted + // in it hitting the holding cell again and we cannot change the state of a + // holding cell HTLC from fulfill to anything else. + let (update_fulfill_msg_option, mut additional_monitor_update) = + if let UpdateFulfillFetch::NewClaim { msg, monitor_update } = self.get_update_fulfill_htlc(htlc_id, *payment_preimage, logger) { + (msg, monitor_update) + } else { unreachable!() }; + update_fulfill_htlcs.push(update_fulfill_msg_option.unwrap()); + monitor_update.updates.append(&mut additional_monitor_update.updates); }, &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, ref err_packet } => { match self.get_update_fail_htlc(htlc_id, err_packet.clone(), logger) { - Ok(update_fail_msg_option) => update_fail_htlcs.push(update_fail_msg_option.unwrap()), + Ok(update_fail_msg_option) => { + // If an HTLC failure was previously added to the holding cell (via + // `get_update_fail_htlc`) then generating the fail message itself + // must not fail - we should never end up in a state where we + // double-fail an HTLC or fail-then-claim an HTLC as it indicates + // we didn't wait for a full revocation before failing. + update_fail_htlcs.push(update_fail_msg_option.unwrap()) + }, Err(e) => { if let ChannelError::Ignore(_) = e {} else { @@ -3005,6 +3103,7 @@ impl Channel { self.channel_state &= !(ChannelState::PeerDisconnected as u32); let shutdown_msg = if self.channel_state & (ChannelState::LocalShutdownSent as u32) != 0 { + assert!(self.shutdown_scriptpubkey.is_some()); Some(msgs::Shutdown { channel_id: self.channel_id, scriptpubkey: self.get_closing_scriptpubkey(), @@ -3116,6 +3215,7 @@ impl Channel { if self.feerate_per_kw > proposed_feerate { proposed_feerate = self.feerate_per_kw; } + assert!(self.shutdown_scriptpubkey.is_some()); let tx_weight = self.get_closing_transaction_weight(Some(&self.get_closing_scriptpubkey()), Some(self.counterparty_shutdown_scriptpubkey.as_ref().unwrap())); let proposed_total_fee_satoshis = proposed_feerate as u64 * tx_weight / 1000; @@ -3134,8 +3234,12 @@ impl Channel { }) } - pub fn shutdown(&mut self, fee_estimator: &F, their_features: &InitFeatures, msg: &msgs::Shutdown) -> Result<(Option, Option, Vec<(HTLCSource, PaymentHash)>), ChannelError> - where F::Target: FeeEstimator + pub fn shutdown( + &mut self, fee_estimator: &F, keys_provider: &K, their_features: &InitFeatures, msg: &msgs::Shutdown + ) -> Result<(Option, Option, Option, Vec<(HTLCSource, PaymentHash)>), ChannelError> + where + F::Target: FeeEstimator, + K::Target: KeysInterface { if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 { return Err(ChannelError::Close("Peer sent shutdown when we needed a channel_reestablish".to_owned())); @@ -3153,18 +3257,36 @@ impl Channel { } assert_eq!(self.channel_state & ChannelState::ShutdownComplete as u32, 0); - if is_unsupported_shutdown_script(&their_features, &msg.scriptpubkey) { - return Err(ChannelError::Close(format!("Got a nonstandard scriptpubkey ({}) from remote peer", msg.scriptpubkey.to_bytes().to_hex()))); - } + let shutdown_scriptpubkey = match ShutdownScript::try_from((msg.scriptpubkey.clone(), their_features)) { + Ok(script) => script.into_inner(), + Err(_) => return Err(ChannelError::Close(format!("Got a nonstandard scriptpubkey ({}) from remote peer", msg.scriptpubkey.to_bytes().to_hex()))), + }; if self.counterparty_shutdown_scriptpubkey.is_some() { - if Some(&msg.scriptpubkey) != self.counterparty_shutdown_scriptpubkey.as_ref() { - return Err(ChannelError::Close(format!("Got shutdown request with a scriptpubkey ({}) which did not match their previous scriptpubkey.", msg.scriptpubkey.to_bytes().to_hex()))); + if Some(&shutdown_scriptpubkey) != self.counterparty_shutdown_scriptpubkey.as_ref() { + return Err(ChannelError::Close(format!("Got shutdown request with a scriptpubkey ({}) which did not match their previous scriptpubkey.", shutdown_scriptpubkey.to_bytes().to_hex()))); } } else { - self.counterparty_shutdown_scriptpubkey = Some(msg.scriptpubkey.clone()); + self.counterparty_shutdown_scriptpubkey = Some(shutdown_scriptpubkey); } + // If we have any LocalAnnounced updates we'll probably just get back a update_fail_htlc + // immediately after the commitment dance, but we can send a Shutdown cause we won't send + // any further commitment updates after we set LocalShutdownSent. + let send_shutdown = (self.channel_state & ChannelState::LocalShutdownSent as u32) != ChannelState::LocalShutdownSent as u32; + + let shutdown_scriptpubkey = match self.shutdown_scriptpubkey { + Some(_) => None, + None => { + assert!(send_shutdown); + let shutdown_scriptpubkey = keys_provider.get_shutdown_scriptpubkey(); + if !shutdown_scriptpubkey.is_compatible(their_features) { + return Err(ChannelError::Close(format!("Provided a scriptpubkey format not accepted by peer. script: ({})", shutdown_scriptpubkey.clone().into_inner().to_bytes().to_hex()))); + } + Some(shutdown_scriptpubkey) + }, + }; + // From here on out, we may not fail! self.channel_state |= ChannelState::RemoteShutdownSent as u32; @@ -3184,23 +3306,31 @@ impl Channel { _ => true } }); - // If we have any LocalAnnounced updates we'll probably just get back a update_fail_htlc - // immediately after the commitment dance, but we can send a Shutdown cause we won't send - // any further commitment updates after we set LocalShutdownSent. - let shutdown = if (self.channel_state & ChannelState::LocalShutdownSent as u32) == ChannelState::LocalShutdownSent as u32 { - None - } else { + let monitor_update = match shutdown_scriptpubkey { + Some(shutdown_scriptpubkey) => { + self.shutdown_scriptpubkey = Some(shutdown_scriptpubkey); + self.latest_monitor_update_id += 1; + Some(ChannelMonitorUpdate { + update_id: self.latest_monitor_update_id, + updates: vec![ChannelMonitorUpdateStep::ShutdownScript { + scriptpubkey: self.get_closing_scriptpubkey(), + }], + }) + }, + None => None, + }; + let shutdown = if send_shutdown { Some(msgs::Shutdown { channel_id: self.channel_id, scriptpubkey: self.get_closing_scriptpubkey(), }) - }; + } else { None }; self.channel_state |= ChannelState::LocalShutdownSent as u32; self.update_time_counter += 1; - Ok((shutdown, self.maybe_propose_first_closing_signed(fee_estimator), dropped_outbound_htlcs)) + Ok((shutdown, self.maybe_propose_first_closing_signed(fee_estimator), monitor_update, dropped_outbound_htlcs)) } fn build_signed_closing_transaction(&self, tx: &mut Transaction, counterparty_sig: &Signature, sig: &Signature) { @@ -3275,6 +3405,7 @@ impl Channel { macro_rules! propose_new_feerate { ($new_feerate: expr) => { + assert!(self.shutdown_scriptpubkey.is_some()); let tx_weight = self.get_closing_transaction_weight(Some(&self.get_closing_scriptpubkey()), Some(self.counterparty_shutdown_scriptpubkey.as_ref().unwrap())); let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate as u64 * tx_weight / 1000, false); let sig = self.holder_signer @@ -3336,6 +3467,10 @@ impl Channel { self.channel_id } + pub fn minimum_depth(&self) -> Option { + self.minimum_depth + } + /// Gets the "user_id" value passed into the construction of this channel. It has no special /// meaning and exists only to allow users to have a persistent identifier of a channel. pub fn get_user_id(&self) -> u64 { @@ -3363,8 +3498,9 @@ impl Channel { &self.channel_transaction_parameters.holder_pubkeys } - fn get_counterparty_selected_contest_delay(&self) -> u16 { - self.channel_transaction_parameters.counterparty_parameters.as_ref().unwrap().selected_contest_delay + pub fn get_counterparty_selected_contest_delay(&self) -> Option { + self.channel_transaction_parameters.counterparty_parameters + .as_ref().map(|params| params.selected_contest_delay) } fn get_counterparty_pubkeys(&self) -> &ChannelPublicKeys { @@ -3404,7 +3540,7 @@ impl Channel { } pub fn get_fee_proportional_millionths(&self) -> u32 { - self.config.fee_proportional_millionths + self.config.forwarding_fee_proportional_millionths } pub fn get_cltv_expiry_delta(&self) -> u16 { @@ -3438,7 +3574,7 @@ impl Channel { ChannelValueStat { value_to_self_msat: self.value_to_self_msat, channel_value_msat: self.channel_value_satoshis * 1000, - channel_reserve_msat: self.counterparty_selected_channel_reserve_satoshis * 1000, + channel_reserve_msat: self.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000, pending_outbound_htlcs_amount_msat: self.pending_outbound_htlcs.iter().map(|ref h| h.amount_msat).sum::(), pending_inbound_htlcs_amount_msat: self.pending_inbound_htlcs.iter().map(|ref h| h.amount_msat).sum::(), holding_cell_outbound_amount_msat: { @@ -3477,24 +3613,8 @@ impl Channel { /// Gets the fee we'd want to charge for adding an HTLC output to this Channel /// Allowed in any state (including after shutdown) - pub fn get_holder_fee_base_msat(&self, fee_estimator: &F) -> u32 - where F::Target: FeeEstimator - { - // For lack of a better metric, we calculate what it would cost to consolidate the new HTLC - // output value back into a transaction with the regular channel output: - - // the fee cost of the HTLC-Success/HTLC-Timeout transaction: - let mut res = self.feerate_per_kw as u64 * cmp::max(HTLC_TIMEOUT_TX_WEIGHT, HTLC_SUCCESS_TX_WEIGHT) / 1000; - - if self.is_outbound() { - // + the marginal fee increase cost to us in the commitment transaction: - res += self.feerate_per_kw as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC / 1000; - } - - // + the marginal cost of an input which spends the HTLC-Success/HTLC-Timeout output: - res += fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal) as u64 * SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT / 1000; - - res as u32 + pub fn get_outbound_forwarding_fee_base_msat(&self) -> u32 { + self.config.forwarding_fee_base_msat } /// Returns true if we've ever received a message from the remote end for this Channel @@ -3555,7 +3675,7 @@ impl Channel { self.funding_tx_confirmation_height = 0; } - if funding_tx_confirmations < self.minimum_depth as i64 { + if funding_tx_confirmations < self.minimum_depth.unwrap_or(0) as i64 { return None; } @@ -3710,10 +3830,10 @@ impl Channel { // the funding transaction's confirmation count has dipped below minimum_depth / 2, // close the channel and hope we can get the latest state on chain (because presumably // the funding transaction is at least still in the mempool of most nodes). - if funding_tx_confirmations < self.minimum_depth as i64 / 2 { + if funding_tx_confirmations < self.minimum_depth.unwrap() as i64 / 2 { return Err(msgs::ErrorMessage { channel_id: self.channel_id(), - data: format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.", self.minimum_depth, funding_tx_confirmations), + data: format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.", self.minimum_depth.unwrap(), funding_tx_confirmations), }); } } @@ -3784,7 +3904,10 @@ impl Channel { htlc_basepoint: keys.htlc_basepoint, first_per_commitment_point, channel_flags: if self.config.announced_channel {1} else {0}, - shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() }) + shutdown_scriptpubkey: OptionalField::Present(match &self.shutdown_scriptpubkey { + Some(script) => script.clone().into_inner(), + None => Builder::new().into_script(), + }), } } @@ -3808,7 +3931,7 @@ impl Channel { max_htlc_value_in_flight_msat: Channel::::get_holder_max_htlc_value_in_flight_msat(self.channel_value_satoshis), channel_reserve_satoshis: Channel::::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis), htlc_minimum_msat: self.holder_htlc_minimum_msat, - minimum_depth: self.minimum_depth, + minimum_depth: self.minimum_depth.unwrap(), to_self_delay: self.get_holder_selected_contest_delay(), max_accepted_htlcs: OUR_MAX_HTLCS, funding_pubkey: keys.funding_pubkey, @@ -3817,7 +3940,10 @@ impl Channel { delayed_payment_basepoint: keys.delayed_payment_basepoint, htlc_basepoint: keys.htlc_basepoint, first_per_commitment_point, - shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() }) + shutdown_scriptpubkey: OptionalField::Present(match &self.shutdown_scriptpubkey { + Some(script) => script.clone().into_inner(), + None => Builder::new().into_script(), + }), } } @@ -4082,7 +4208,7 @@ impl Channel { if !self.is_outbound() { // Check that we won't violate the remote channel reserve by adding this HTLC. let counterparty_balance_msat = self.channel_value_satoshis * 1000 - self.value_to_self_msat; - let holder_selected_chan_reserve_msat = Channel::::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis); + let holder_selected_chan_reserve_msat = Channel::::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) * 1000; let htlc_candidate = HTLCCandidate::new(amount_msat, HTLCInitiator::LocalOffered); let counterparty_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(htlc_candidate, None); if counterparty_balance_msat < holder_selected_chan_reserve_msat + counterparty_commit_tx_fee_msat { @@ -4106,7 +4232,7 @@ impl Channel { // Check self.counterparty_selected_channel_reserve_satoshis (the amount we must keep as // reserve for the remote to have something to claim if we misbehave) - let chan_reserve_msat = self.counterparty_selected_channel_reserve_satoshis * 1000; + let chan_reserve_msat = self.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000; if pending_value_to_self_msat - amount_msat - commit_tx_fee_msat < chan_reserve_msat { return Err(ChannelError::Ignore(format!("Cannot send value that would put our balance under counterparty-announced channel reserve value ({})", chan_reserve_msat))); } @@ -4311,8 +4437,7 @@ impl Channel { } pub fn channel_update(&mut self, msg: &msgs::ChannelUpdate) -> Result<(), ChannelError> { - let usable_channel_value_msat = (self.channel_value_satoshis - self.counterparty_selected_channel_reserve_satoshis) * 1000; - if msg.contents.htlc_minimum_msat >= usable_channel_value_msat { + if msg.contents.htlc_minimum_msat >= self.channel_value_satoshis * 1000 { return Err(ChannelError::Close("Minimum htlc value is greater than channel value".to_string())); } self.counterparty_forwarding_info = Some(CounterpartyForwardingInfo { @@ -4326,7 +4451,8 @@ impl Channel { /// Begins the shutdown process, getting a message for the remote peer and returning all /// holding cell HTLCs for payment failure. - pub fn get_shutdown(&mut self) -> Result<(msgs::Shutdown, Vec<(HTLCSource, PaymentHash)>), APIError> { + pub fn get_shutdown(&mut self, keys_provider: &K, their_features: &InitFeatures) -> Result<(msgs::Shutdown, Option, Vec<(HTLCSource, PaymentHash)>), APIError> + where K::Target: KeysInterface { for htlc in self.pending_outbound_htlcs.iter() { if let OutboundHTLCState::LocalAnnounced(_) = htlc.state { return Err(APIError::APIMisuseError{err: "Cannot begin shutdown with pending HTLCs. Process pending events first".to_owned()}); @@ -4345,7 +4471,30 @@ impl Channel { return Err(APIError::ChannelUnavailable{err: "Cannot begin shutdown while peer is disconnected or we're waiting on a monitor update, maybe force-close instead?".to_owned()}); } - let closing_script = self.get_closing_scriptpubkey(); + let shutdown_scriptpubkey = match self.shutdown_scriptpubkey { + Some(_) => None, + None => { + let shutdown_scriptpubkey = keys_provider.get_shutdown_scriptpubkey(); + if !shutdown_scriptpubkey.is_compatible(their_features) { + return Err(APIError::APIMisuseError { err: format!("Provided a scriptpubkey format not accepted by peer. script: ({})", shutdown_scriptpubkey.clone().into_inner().to_bytes().to_hex()) }); + } + Some(shutdown_scriptpubkey) + }, + }; + + let monitor_update = match shutdown_scriptpubkey { + Some(shutdown_scriptpubkey) => { + self.shutdown_scriptpubkey = Some(shutdown_scriptpubkey); + self.latest_monitor_update_id += 1; + Some(ChannelMonitorUpdate { + update_id: self.latest_monitor_update_id, + updates: vec![ChannelMonitorUpdateStep::ShutdownScript { + scriptpubkey: self.get_closing_scriptpubkey(), + }], + }) + }, + None => None, + }; // From here on out, we may not fail! if self.channel_state < ChannelState::FundingSent as u32 { @@ -4371,8 +4520,8 @@ impl Channel { Ok((msgs::Shutdown { channel_id: self.channel_id, - scriptpubkey: closing_script, - }, dropped_outbound_htlcs)) + scriptpubkey: self.get_closing_scriptpubkey(), + }, monitor_update, dropped_outbound_htlcs)) } /// Gets the latest commitment transaction and any dependent transactions for relay (forcing @@ -4421,25 +4570,7 @@ impl Channel { } } -fn is_unsupported_shutdown_script(their_features: &InitFeatures, script: &Script) -> bool { - // We restrain shutdown scripts to standards forms to avoid transactions not propagating on the p2p tx-relay network - - // BOLT 2 says we must only send a scriptpubkey of certain standard forms, - // which for a a BIP-141-compliant witness program is at max 42 bytes in length. - // So don't let the remote peer feed us some super fee-heavy script. - let is_script_too_long = script.len() > 42; - if is_script_too_long { - return true; - } - - if their_features.supports_shutdown_anysegwit() && script.is_witness_program() && script.as_bytes()[0] != OP_PUSHBYTES_0.into_u8() { - return false; - } - - return !script.is_p2pkh() && !script.is_p2sh() && !script.is_v0_p2wpkh() && !script.is_v0_p2wsh() -} - -const SERIALIZATION_VERSION: u8 = 1; +const SERIALIZATION_VERSION: u8 = 2; const MIN_SERIALIZATION_VERSION: u8 = 1; impl_writeable_tlv_based_enum!(InboundHTLCRemovalReason,; @@ -4481,7 +4612,13 @@ impl Writeable for Channel { write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION); self.user_id.write(writer)?; - self.config.write(writer)?; + + // Write out the old serialization for the config object. This is read by version-1 + // deserializers, but we will read the version in the TLV at the end instead. + self.config.forwarding_fee_proportional_millionths.write(writer)?; + self.config.cltv_expiry_delta.write(writer)?; + self.config.announced_channel.write(writer)?; + self.config.commit_upfront_shutdown_pubkey.write(writer)?; self.channel_id.write(writer)?; (self.channel_state | ChannelState::PeerDisconnected as u32).write(writer)?; @@ -4496,7 +4633,12 @@ impl Writeable for Channel { (key_data.0.len() as u32).write(writer)?; writer.write_all(&key_data.0[..])?; - self.shutdown_pubkey.write(writer)?; + // Write out the old serialization for shutdown_pubkey for backwards compatibility, if + // deserialized from that format. + match self.shutdown_scriptpubkey.as_ref().and_then(|script| script.as_legacy_pubkey()) { + Some(shutdown_pubkey) => shutdown_pubkey.write(writer)?, + None => [0u8; PUBLIC_KEY_SIZE].write(writer)?, + } self.destination_script.write(writer)?; self.cur_holder_commitment_transaction_number.write(writer)?; @@ -4640,11 +4782,16 @@ impl Writeable for Channel { self.counterparty_dust_limit_satoshis.write(writer)?; self.holder_dust_limit_satoshis.write(writer)?; self.counterparty_max_htlc_value_in_flight_msat.write(writer)?; - self.counterparty_selected_channel_reserve_satoshis.write(writer)?; + + // Note that this field is ignored by 0.0.99+ as the TLV Optional variant is used instead. + self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0).write(writer)?; + self.counterparty_htlc_minimum_msat.write(writer)?; self.holder_htlc_minimum_msat.write(writer)?; self.counterparty_max_accepted_htlcs.write(writer)?; - self.minimum_depth.write(writer)?; + + // Note that this field is ignored by 0.0.99+ as the TLV Optional variant is used instead. + self.minimum_depth.unwrap_or(0).write(writer)?; match &self.counterparty_forwarding_info { Some(info) => { @@ -4669,7 +4816,26 @@ impl Writeable for Channel { self.channel_update_status.write(writer)?; - write_tlv_fields!(writer, {(0, self.announcement_sigs, option)}); + #[cfg(any(test, feature = "fuzztarget"))] + (self.historical_inbound_htlc_fulfills.len() as u64).write(writer)?; + #[cfg(any(test, feature = "fuzztarget"))] + for htlc in self.historical_inbound_htlc_fulfills.iter() { + htlc.write(writer)?; + } + + write_tlv_fields!(writer, { + (0, self.announcement_sigs, option), + // minimum_depth and counterparty_selected_channel_reserve_satoshis used to have a + // default value instead of being Option<>al. Thus, to maintain compatibility we write + // them twice, once with their original default values above, and once as an option + // here. On the read side, old versions will simply ignore the odd-type entries here, + // and new versions map the default values to None and allow the TLV entries here to + // override that. + (1, self.minimum_depth, option), + (3, self.counterparty_selected_channel_reserve_satoshis, option), + (5, self.config, required), + (7, self.shutdown_scriptpubkey, option), + }); Ok(()) } @@ -4679,10 +4845,21 @@ const MAX_ALLOC_SIZE: usize = 64*1024; impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel where K::Target: KeysInterface { fn read(reader: &mut R, keys_source: &'a K) -> Result { - let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION); + let ver = read_ver_prefix!(reader, SERIALIZATION_VERSION); let user_id = Readable::read(reader)?; - let config: ChannelConfig = Readable::read(reader)?; + + let mut config = Some(ChannelConfig::default()); + if ver == 1 { + // Read the old serialization of the ChannelConfig from version 0.0.98. + config.as_mut().unwrap().forwarding_fee_proportional_millionths = Readable::read(reader)?; + config.as_mut().unwrap().cltv_expiry_delta = Readable::read(reader)?; + config.as_mut().unwrap().announced_channel = Readable::read(reader)?; + config.as_mut().unwrap().commit_upfront_shutdown_pubkey = Readable::read(reader)?; + } else { + // Read the 8 bytes of backwards-compatibility ChannelConfig data. + let mut _val: u64 = Readable::read(reader)?; + } let channel_id = Readable::read(reader)?; let channel_state = Readable::read(reader)?; @@ -4701,7 +4878,12 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel } let holder_signer = keys_source.read_chan_signer(&keys_data)?; - let shutdown_pubkey = Readable::read(reader)?; + // Read the old serialization for shutdown_pubkey, preferring it for shutdown_scriptpubkey + // over the TLV if valid. + let mut shutdown_scriptpubkey = match ::read(reader) { + Ok(pubkey) => Some(ShutdownScript::new_p2wpkh_from_pubkey(pubkey)), + Err(_) => None, + }; let destination_script = Readable::read(reader)?; let cur_holder_commitment_transaction_number = Readable::read(reader)?; @@ -4812,11 +4994,26 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel let counterparty_dust_limit_satoshis = Readable::read(reader)?; let holder_dust_limit_satoshis = Readable::read(reader)?; let counterparty_max_htlc_value_in_flight_msat = Readable::read(reader)?; - let counterparty_selected_channel_reserve_satoshis = Readable::read(reader)?; + let mut counterparty_selected_channel_reserve_satoshis = None; + if ver == 1 { + // Read the old serialization from version 0.0.98. + counterparty_selected_channel_reserve_satoshis = Some(Readable::read(reader)?); + } else { + // Read the 8 bytes of backwards-compatibility data. + let _dummy: u64 = Readable::read(reader)?; + } let counterparty_htlc_minimum_msat = Readable::read(reader)?; let holder_htlc_minimum_msat = Readable::read(reader)?; let counterparty_max_accepted_htlcs = Readable::read(reader)?; - let minimum_depth = Readable::read(reader)?; + + let mut minimum_depth = None; + if ver == 1 { + // Read the old serialization from version 0.0.98. + minimum_depth = Some(Readable::read(reader)?); + } else { + // Read the 4 bytes of backwards-compatibility data. + let _dummy: u32 = Readable::read(reader)?; + } let counterparty_forwarding_info = match ::read(reader)? { 0 => None, @@ -4841,8 +5038,24 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel let channel_update_status = Readable::read(reader)?; + #[cfg(any(test, feature = "fuzztarget"))] + let mut historical_inbound_htlc_fulfills = HashSet::new(); + #[cfg(any(test, feature = "fuzztarget"))] + { + let htlc_fulfills_len: u64 = Readable::read(reader)?; + for _ in 0..htlc_fulfills_len { + assert!(historical_inbound_htlc_fulfills.insert(Readable::read(reader)?)); + } + } + let mut announcement_sigs = None; - read_tlv_fields!(reader, {(0, announcement_sigs, option)}); + read_tlv_fields!(reader, { + (0, announcement_sigs, option), + (1, minimum_depth, option), + (3, counterparty_selected_channel_reserve_satoshis, option), + (5, config, option), // Note that if none is provided we will *not* overwrite the existing one. + (7, shutdown_scriptpubkey, option), + }); let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&keys_source.get_secure_random_bytes()); @@ -4850,7 +5063,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel Ok(Channel { user_id, - config, + config: config.unwrap(), channel_id, channel_state, secp_ctx, @@ -4859,7 +5072,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel latest_monitor_update_id, holder_signer, - shutdown_pubkey, + shutdown_scriptpubkey, destination_script, cur_holder_commitment_transaction_number, @@ -4886,9 +5099,9 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel feerate_per_kw, #[cfg(debug_assertions)] - holder_max_commitment_tx_output: ::std::sync::Mutex::new((0, 0)), + holder_max_commitment_tx_output: Mutex::new((0, 0)), #[cfg(debug_assertions)] - counterparty_max_commitment_tx_output: ::std::sync::Mutex::new((0, 0)), + counterparty_max_commitment_tx_output: Mutex::new((0, 0)), last_sent_closing_fee, @@ -4928,6 +5141,9 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel next_remote_commitment_tx_fee_info_cached: Mutex::new(None), workaround_lnd_bug_4006: None, + + #[cfg(any(test, feature = "fuzztarget"))] + historical_inbound_htlc_fulfills, }) } } @@ -4944,13 +5160,15 @@ mod tests { use bitcoin::hashes::hex::FromHex; use hex; use ln::{PaymentPreimage, PaymentHash}; - use ln::channelmanager::{BestBlock, HTLCSource}; + use ln::channelmanager::HTLCSource; use ln::channel::{Channel,InboundHTLCOutput,OutboundHTLCOutput,InboundHTLCState,OutboundHTLCState,HTLCOutputInCommitment,HTLCCandidate,HTLCInitiator,TxCreationKeys}; use ln::channel::MAX_FUNDING_SATOSHIS; use ln::features::InitFeatures; use ln::msgs::{ChannelUpdate, DataLossProtect, DecodeError, OptionalField, UnsignedChannelUpdate}; + use ln::script::ShutdownScript; use ln::chan_utils; use ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT}; + use chain::BestBlock; use chain::chaininterface::{FeeEstimator,ConfirmationTarget}; use chain::keysinterface::{InMemorySigner, KeysInterface, BaseSign}; use chain::transaction::OutPoint; @@ -4965,7 +5183,7 @@ mod tests { use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::Hash; use bitcoin::hash_types::{Txid, WPubkeyHash}; - use std::sync::Arc; + use sync::Arc; use prelude::*; struct TestFeeEstimator { @@ -4997,10 +5215,10 @@ mod tests { Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&channel_monitor_claim_key_hash[..]).into_script() } - fn get_shutdown_pubkey(&self) -> PublicKey { + fn get_shutdown_scriptpubkey(&self) -> ShutdownScript { let secp_ctx = Secp256k1::signing_only(); let channel_close_key = SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(); - PublicKey::from_secret_key(&secp_ctx, &channel_close_key) + ShutdownScript::new_p2wpkh_from_pubkey(PublicKey::from_secret_key(&secp_ctx, &channel_close_key)) } fn get_channel_signer(&self, _inbound: bool, _channel_value_satoshis: u64) -> InMemorySigner { @@ -5028,7 +5246,7 @@ mod tests { let node_a_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let node_a_chan = Channel::::new_outbound(&&fee_est, &&keys_provider, node_a_node_id, 10000000, 100000, 42, &config).unwrap(); + let node_a_chan = Channel::::new_outbound(&&fee_est, &&keys_provider, node_a_node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap(); // Now change the fee so we can check that the fee in the open_channel message is the // same as the old fee. @@ -5053,7 +5271,7 @@ mod tests { // Create Node A's channel pointing to Node B's pubkey let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, 10000000, 100000, 42, &config).unwrap(); + let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap(); // Create Node B's channel by receiving Node A's open_channel message // Make sure A's dust limit is as we expect. @@ -5120,7 +5338,7 @@ mod tests { let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let mut chan = Channel::::new_outbound(&&fee_est, &&keys_provider, node_id, 10000000, 100000, 42, &config).unwrap(); + let mut chan = Channel::::new_outbound(&&fee_est, &&keys_provider, node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap(); let commitment_tx_fee_0_htlcs = chan.commit_tx_fee_msat(0); let commitment_tx_fee_1_htlc = chan.commit_tx_fee_msat(1); @@ -5169,7 +5387,7 @@ mod tests { // Create Node A's channel pointing to Node B's pubkey let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, 10000000, 100000, 42, &config).unwrap(); + let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap(); // Create Node B's channel by receiving Node A's open_channel message let open_channel_msg = node_a_chan.get_open_channel(chain_hash); @@ -5231,7 +5449,7 @@ mod tests { // Create a channel. let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, 10000000, 100000, 42, &config).unwrap(); + let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap(); assert!(node_a_chan.counterparty_forwarding_info.is_none()); assert_eq!(node_a_chan.holder_htlc_minimum_msat, 1); // the default assert!(node_a_chan.counterparty_forwarding_info().is_none()); @@ -5295,8 +5513,9 @@ mod tests { let counterparty_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let mut config = UserConfig::default(); config.channel_options.announced_channel = false; - let mut chan = Channel::::new_outbound(&&feeest, &&keys_provider, counterparty_node_id, 10_000_000, 100000, 42, &config).unwrap(); // Nothing uses their network key in this test + let mut chan = Channel::::new_outbound(&&feeest, &&keys_provider, counterparty_node_id, InitFeatures::known(), 10_000_000, 100000, 42, &config).unwrap(); // Nothing uses their network key in this test chan.holder_dust_limit_satoshis = 546; + chan.counterparty_selected_channel_reserve_satoshis = Some(0); // Filled in in accept_channel let funding_info = OutPoint{ txid: Txid::from_hex("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(), index: 0 }; @@ -5384,7 +5603,9 @@ mod tests { let remote_signature = Signature::from_der(&hex::decode($counterparty_htlc_sig_hex).unwrap()[..]).unwrap(); let ref htlc = htlcs[$htlc_idx]; - let htlc_tx = chan.build_htlc_transaction(&unsigned_tx.txid, &htlc, true, &keys, chan.feerate_per_kw); + let htlc_tx = chan_utils::build_htlc_transaction(&unsigned_tx.txid, chan.feerate_per_kw, + chan.get_counterparty_selected_contest_delay().unwrap(), + &htlc, &keys.broadcaster_delayed_payment_key, &keys.revocation_key); let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys); let htlc_sighash = Message::from_slice(&bip143::SigHashCache::new(&htlc_tx).signature_hash(0, &htlc_redeemscript, htlc.amount_msat / 1000, SigHashType::All)[..]).unwrap(); secp_ctx.verify(&htlc_sighash, &remote_signature, &keys.countersignatory_htlc_key).unwrap();