X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannel.rs;h=e39478bd740c063ce1f29a7dc853238ec75fcf48;hb=c0bbd4d91877b3f7eca5b6aba877257acf4eec0b;hp=0ada482f3679563b98de11a861ee9a9d9caf6080;hpb=177810b1522833f85b8b1c422a455e736afe3676;p=rust-lightning diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 0ada482f..e39478bd 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -8,7 +8,7 @@ // licenses. use bitcoin::blockdata::script::{Script,Builder}; -use bitcoin::blockdata::transaction::{TxIn, TxOut, Transaction, SigHashType}; +use bitcoin::blockdata::transaction::{Transaction, SigHashType}; use bitcoin::util::bip143; use bitcoin::consensus::encode; @@ -23,19 +23,18 @@ use bitcoin::secp256k1::{Secp256k1,Signature}; use bitcoin::secp256k1; use ln::{PaymentPreimage, PaymentHash}; -use ln::features::{ChannelFeatures, InitFeatures}; +use ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures}; use ln::msgs; use ln::msgs::{DecodeError, OptionalField, DataLossProtect}; -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::script::{self, ShutdownScript}; +use ln::channelmanager::{CounterpartyForwardingInfo, 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, ClosingTransaction}; 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}; use chain::keysinterface::{Sign, KeysInterface}; -use util::transaction_utils; use util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter}; use util::logger::Logger; use util::errors::APIError; @@ -45,7 +44,6 @@ use util::scid_utils::scid_from_parts; use io; use prelude::*; use core::{cmp,mem,fmt}; -use core::convert::TryFrom; use core::ops::Deref; #[cfg(any(test, feature = "fuzztarget", debug_assertions))] use sync::Mutex; @@ -255,8 +253,6 @@ enum ChannelState { RemoteShutdownSent = 1 << 10, /// Flag which is set on ChannelFunded or FundingSent after sending a shutdown message. At this /// point, we may not add any new HTLCs to the channel. - /// TODO: Investigate some kind of timeout mechanism by which point the remote end must provide - /// us their shutdown. LocalShutdownSent = 1 << 11, /// We've successfully negotiated a closing_signed dance. At this point ChannelManager is about /// to drop us, but we store this anyway. @@ -313,19 +309,6 @@ impl HTLCCandidate { } } -/// Information needed for constructing an invoice route hint for this channel. -#[derive(Clone, Debug, PartialEq)] -pub struct CounterpartyForwardingInfo { - /// Base routing fee in millisatoshis. - pub fee_base_msat: u32, - /// Amount in millionths of a satoshi the channel will charge per transferred satoshi. - pub fee_proportional_millionths: u32, - /// The minimum difference in cltv_expiry between an ingoing HTLC and its outgoing counterpart, - /// such that the outgoing HTLC is forwardable to this counterparty. See `msgs::ChannelUpdate`'s - /// `cltv_expiry_delta` for more details. - pub cltv_expiry_delta: u16, -} - /// A return value enum for get_update_fulfill_htlc. See UpdateFulfillCommitFetch variants for /// description enum UpdateFulfillFetch { @@ -356,6 +339,29 @@ pub enum UpdateFulfillCommitFetch { DuplicateClaim {}, } +/// The return value of `revoke_and_ack` on success, primarily updates to other channels or HTLC +/// state. +pub(super) struct RAAUpdates { + pub commitment_update: Option, + pub accepted_htlcs: Vec<(PendingHTLCInfo, u64)>, + pub failed_htlcs: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, + pub finalized_claimed_htlcs: Vec, + pub monitor_update: ChannelMonitorUpdate, + pub holding_cell_failed_htlcs: Vec<(HTLCSource, PaymentHash)>, +} + +/// The return value of `monitor_updating_restored` +pub(super) struct MonitorRestoreUpdates { + pub raa: Option, + pub commitment_update: Option, + pub order: RAACommitmentOrder, + pub accepted_htlcs: Vec<(PendingHTLCInfo, u64)>, + pub failed_htlcs: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, + pub finalized_claimed_htlcs: Vec, + pub funding_broadcastable: Option, + pub funding_locked: Option, +} + /// If the majority of the channels funds are to the fundee and the initiator holds only just /// enough funds to cover their reserve value, channels are at risk of getting "stuck". Because the /// initiator controls the feerate, if they then go to increase the channel fee, they may have no @@ -423,23 +429,20 @@ pub(super) struct Channel { monitor_pending_commitment_signed: bool, monitor_pending_forwards: Vec<(PendingHTLCInfo, u64)>, monitor_pending_failures: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, + monitor_pending_finalized_fulfills: Vec, - // pending_update_fee is filled when sending and receiving update_fee - // For outbound channel, feerate_per_kw is updated with the value from - // pending_update_fee when revoke_and_ack is received + // pending_update_fee is filled when sending and receiving update_fee. // - // For inbound channel, feerate_per_kw is updated when it receives - // commitment_signed and revoke_and_ack is generated - // The pending value is kept when another pair of update_fee and commitment_signed - // is received during AwaitingRemoteRevoke and relieved when the expected - // revoke_and_ack is received and new commitment_signed is generated to be - // sent to the funder. Otherwise, the pending value is removed when receiving - // commitment_signed. + // Because it follows the same commitment flow as HTLCs, `FeeUpdateState` is either `Outbound` + // or matches a subset of the `InboundHTLCOutput` variants. It is then updated/used when + // generating new commitment transactions with exactly the same criteria as inbound/outbound + // HTLCs with similar state. pending_update_fee: Option<(u32, FeeUpdateState)>, - // update_fee() during ChannelState::AwaitingRemoteRevoke is hold in - // holdina_cell_update_fee then moved to pending_udpate_fee when revoke_and_ack - // is received. holding_cell_update_fee is updated when there are additional - // update_fee() during ChannelState::AwaitingRemoteRevoke. + // If a `send_update_fee()` call is made with ChannelState::AwaitingRemoteRevoke set, we place + // it here instead of `pending_update_fee` in the same way as we place outbound HTLC updates in + // `holding_cell_htlc_updates` instead of `pending_outbound_htlcs`. It is released into + // `pending_update_fee` with the same criteria as outbound HTLC updates but can be updated by + // further `send_update_fee` calls, dropping the previous holding cell update entirely. holding_cell_update_fee: Option, next_holder_htlc_id: u64, next_counterparty_htlc_id: u64, @@ -454,7 +457,6 @@ pub(super) struct Channel { counterparty_max_commitment_tx_output: Mutex<(u64, u64)>, last_sent_closing_fee: Option<(u64, Signature)>, // (fee, holder_sig) - closing_fee_limits: Option<(u64, u64)>, target_closing_feerate_sats_per_kw: Option, /// If our counterparty sent us a closing_signed while we were waiting for a `ChannelMonitor` @@ -462,6 +464,13 @@ pub(super) struct Channel { /// closing_signed message and handling it in `maybe_propose_closing_signed`. pending_counterparty_closing_signed: Option, + /// The minimum and maximum absolute fee, in satoshis, we are willing to place on the closing + /// transaction. These are set once we reach `closing_negotiation_ready`. + #[cfg(test)] + pub(crate) closing_fee_limits: Option<(u64, u64)>, + #[cfg(not(test))] + closing_fee_limits: Option<(u64, u64)>, + /// The hash of the block in which the funding transaction was included. funding_tx_confirmed_in: Option, funding_tx_confirmation_height: u32, @@ -503,6 +512,13 @@ pub(super) struct Channel { commitment_secrets: CounterpartyCommitmentSecrets, channel_update_status: ChannelUpdateStatus, + /// Once we reach `closing_negotiation_ready`, we set this, indicating if closing_signed does + /// not complete within a single timer tick (one minute), we should force-close the channel. + /// This prevents us from keeping unusable channels around forever if our counterparty wishes + /// to DoS us. + /// Note that this field is reset to false on deserialization to give us a chance to connect to + /// our peer and start the closing_signed negotiation fresh. + closing_signed_in_flight: bool, /// Our counterparty's channel_announcement signatures provided in announcement_signatures. /// This can be used to rebroadcast the channel_announcement message later. @@ -534,6 +550,9 @@ pub(super) struct Channel { // 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, + + /// This channel's type, as negotiated during channel open + channel_type: ChannelTypeFeatures, } #[cfg(any(test, feature = "fuzztarget"))] @@ -556,25 +575,30 @@ const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172; #[cfg(test)] pub const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172; -/// Maximmum `funding_satoshis` value, according to the BOLT #2 specification +pub const ANCHOR_OUTPUT_VALUE_SATOSHI: u64 = 330; + +/// Maximum `funding_satoshis` value, according to the BOLT #2 specification /// it's 2^24. pub const MAX_FUNDING_SATOSHIS: u64 = 1 << 24; -/// Maximum counterparty `dust_limit_satoshis` allowed. 2 * standard dust threshold on p2wsh output -/// Scales up on Bitcoin Core's proceeding policy with dust outputs. A typical p2wsh output is 43 -/// bytes to which Core's `GetDustThreshold()` sums up a minimal spend of 67 bytes (even if -/// a p2wsh witnessScript might be *effectively* smaller), `dustRelayFee` is set to 3000sat/kb, thus -/// 110 * 3000 / 1000 = 330. Per-protocol rules, all time-sensitive outputs are p2wsh, a value of -/// 330 sats is the lower bound desired to ensure good propagation of transactions. We give a bit -/// of margin to our counterparty and pick up 660 satoshis as an accepted `dust_limit_satoshis` -/// upper bound to avoid negotiation conflicts with other implementations. -pub const MAX_DUST_LIMIT_SATOSHIS: u64 = 2 * 330; - -/// A typical p2wsh output is 43 bytes to which Core's `GetDustThreshold()` sums up a minimal -/// spend of 67 bytes (even if a p2wsh witnessScript might be *effectively* smaller), `dustRelayFee` -/// is set to 3000sat/kb, thus 110 * 3000 / 1000 = 330. Per-protocol rules, all time-sensitive outputs -/// are p2wsh, a value of 330 sats is the lower bound desired to ensure good propagation of transactions. -pub const MIN_DUST_LIMIT_SATOSHIS: u64 = 330; +/// The maximum network dust limit for standard script formats. This currently represents the +/// minimum output value for a P2SH output before Bitcoin Core 22 considers the entire +/// transaction non-standard and thus refuses to relay it. +/// We also use this as the maximum counterparty `dust_limit_satoshis` allowed, given many +/// implementations use this value for their dust limit today. +pub const MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS: u64 = 546; + +/// The maximum channel dust limit we will accept from our counterparty. +pub const MAX_CHAN_DUST_LIMIT_SATOSHIS: u64 = MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS; + +/// The dust limit is used for both the commitment transaction outputs as well as the closing +/// transactions. For cooperative closing transactions, we require segwit outputs, though accept +/// *any* segwit scripts, which are allowed to be up to 42 bytes in length. +/// In order to avoid having to concern ourselves with standardness during the closing process, we +/// simply require our counterparty to use a dust limit which will leave any segwit output +/// standard. +/// See https://github.com/lightningnetwork/lightning-rfc/issues/905 for more details. +pub const MIN_CHAN_DUST_LIMIT_SATOSHIS: u64 = 354; /// Used to return a simple Error back to ChannelManager. Will get converted to a /// msgs::ErrorAction::SendErrorMessage or msgs::ErrorAction::IgnoreError as appropriate with our @@ -641,7 +665,7 @@ impl Channel { return Err(APIError::APIMisuseError {err: format!("Configured with an unreasonable our_to_self_delay ({}) putting user funds at risks", holder_selected_contest_delay)}); } let holder_selected_channel_reserve_satoshis = Channel::::get_holder_selected_channel_reserve_satoshis(channel_value_satoshis); - if holder_selected_channel_reserve_satoshis < MIN_DUST_LIMIT_SATOSHIS { + if holder_selected_channel_reserve_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS { return Err(APIError::APIMisuseError { err: format!("Holder selected channel reserve below implemention limit dust_limit_satoshis {}", holder_selected_channel_reserve_satoshis) }); } @@ -695,6 +719,7 @@ impl Channel { monitor_pending_commitment_signed: false, monitor_pending_forwards: Vec::new(), monitor_pending_failures: Vec::new(), + monitor_pending_finalized_fulfills: Vec::new(), #[cfg(debug_assertions)] holder_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)), @@ -712,7 +737,7 @@ impl Channel { feerate_per_kw: feerate, counterparty_dust_limit_satoshis: 0, - holder_dust_limit_satoshis: MIN_DUST_LIMIT_SATOSHIS, + holder_dust_limit_satoshis: MIN_CHAN_DUST_LIMIT_SATOSHIS, counterparty_max_htlc_value_in_flight_msat: 0, counterparty_selected_channel_reserve_satoshis: None, // Filled in in accept_channel counterparty_htlc_minimum_msat: 0, @@ -740,6 +765,7 @@ impl Channel { commitment_secrets: CounterpartyCommitmentSecrets::new(), channel_update_status: ChannelUpdateStatus::Enabled, + closing_signed_in_flight: false, announcement_sigs: None, @@ -752,6 +778,11 @@ impl Channel { #[cfg(any(test, feature = "fuzztarget"))] historical_inbound_htlc_fulfills: HashSet::new(), + + // We currently only actually support one channel type, so don't retry with new types + // on error messages. When we support more we'll need fallback support (assuming we + // want to support old types). + channel_type: ChannelTypeFeatures::only_static_remote_key(), }) } @@ -780,6 +811,23 @@ impl Channel { where K::Target: KeysInterface, F::Target: FeeEstimator { + // First check the channel type is known, failing before we do anything else if we don't + // support this channel type. + let channel_type = if let Some(channel_type) = &msg.channel_type { + if channel_type.supports_any_optional_bits() { + return Err(ChannelError::Close("Channel Type field contained optional bits - this is not allowed".to_owned())); + } + if *channel_type != ChannelTypeFeatures::only_static_remote_key() { + return Err(ChannelError::Close("Channel Type was not understood".to_owned())); + } + channel_type.clone() + } else { + ChannelTypeFeatures::from_counterparty_init(&their_features) + }; + if !channel_type.supports_static_remote_key() { + return Err(ChannelError::Close("Channel Type was not understood - we require static remote key".to_owned())); + } + let holder_signer = keys_provider.get_channel_signer(true, msg.funding_satoshis); let pubkeys = holder_signer.pubkeys().clone(); let counterparty_pubkeys = ChannelPublicKeys { @@ -845,11 +893,11 @@ impl Channel { if msg.max_accepted_htlcs < config.peer_channel_config_limits.min_max_accepted_htlcs { return Err(ChannelError::Close(format!("max_accepted_htlcs ({}) is less than the user specified limit ({})", msg.max_accepted_htlcs, config.peer_channel_config_limits.min_max_accepted_htlcs))); } - if msg.dust_limit_satoshis < MIN_DUST_LIMIT_SATOSHIS { - return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is less than the implementation limit ({})", msg.dust_limit_satoshis, MIN_DUST_LIMIT_SATOSHIS))); + if msg.dust_limit_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS { + return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is less than the implementation limit ({})", msg.dust_limit_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS))); } - if msg.dust_limit_satoshis > MAX_DUST_LIMIT_SATOSHIS { - return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is greater than the implementation limit ({})", msg.dust_limit_satoshis, MAX_DUST_LIMIT_SATOSHIS))); + if msg.dust_limit_satoshis > MAX_CHAN_DUST_LIMIT_SATOSHIS { + return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is greater than the implementation limit ({})", msg.dust_limit_satoshis, MAX_CHAN_DUST_LIMIT_SATOSHIS))); } // Convert things into internal flags and prep our state: @@ -866,11 +914,11 @@ impl Channel { let background_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background); let holder_selected_channel_reserve_satoshis = Channel::::get_holder_selected_channel_reserve_satoshis(msg.funding_satoshis); - if holder_selected_channel_reserve_satoshis < MIN_DUST_LIMIT_SATOSHIS { - return Err(ChannelError::Close(format!("Suitable channel reserve not found. remote_channel_reserve was ({}). dust_limit_satoshis is ({}).", holder_selected_channel_reserve_satoshis, MIN_DUST_LIMIT_SATOSHIS))); + if holder_selected_channel_reserve_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS { + return Err(ChannelError::Close(format!("Suitable channel reserve not found. remote_channel_reserve was ({}). dust_limit_satoshis is ({}).", holder_selected_channel_reserve_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS))); } - if msg.channel_reserve_satoshis < MIN_DUST_LIMIT_SATOSHIS { - return Err(ChannelError::Close(format!("channel_reserve_satoshis ({}) is smaller than our dust limit ({})", msg.channel_reserve_satoshis, MIN_DUST_LIMIT_SATOSHIS))); + if msg.channel_reserve_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS { + return Err(ChannelError::Close(format!("channel_reserve_satoshis ({}) is smaller than our dust limit ({})", msg.channel_reserve_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS))); } if holder_selected_channel_reserve_satoshis < msg.dust_limit_satoshis { return Err(ChannelError::Close(format!("Dust limit ({}) too high for the channel reserve we require the remote to keep ({})", msg.dust_limit_satoshis, holder_selected_channel_reserve_satoshis))); @@ -897,10 +945,10 @@ impl Channel { if script.len() == 0 { None } else { - 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 an unacceptable scriptpubkey format: {}", script))), + if !script::is_bolt2_compliant(&script, their_features) { + return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided an unacceptable scriptpubkey format: {}", script))) } + Some(script.clone()) } }, // 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 @@ -957,6 +1005,7 @@ impl Channel { monitor_pending_commitment_signed: false, monitor_pending_forwards: Vec::new(), monitor_pending_failures: Vec::new(), + monitor_pending_finalized_fulfills: Vec::new(), #[cfg(debug_assertions)] holder_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)), @@ -975,7 +1024,7 @@ impl Channel { feerate_per_kw: msg.feerate_per_kw, channel_value_satoshis: msg.funding_satoshis, counterparty_dust_limit_satoshis: msg.dust_limit_satoshis, - holder_dust_limit_satoshis: MIN_DUST_LIMIT_SATOSHIS, + holder_dust_limit_satoshis: MIN_CHAN_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: Some(msg.channel_reserve_satoshis), counterparty_htlc_minimum_msat: msg.htlc_minimum_msat, @@ -1006,6 +1055,7 @@ impl Channel { commitment_secrets: CounterpartyCommitmentSecrets::new(), channel_update_status: ChannelUpdateStatus::Enabled, + closing_signed_in_flight: false, announcement_sigs: None, @@ -1018,6 +1068,8 @@ impl Channel { #[cfg(any(test, feature = "fuzztarget"))] historical_inbound_htlc_fulfills: HashSet::new(), + + channel_type, }; Ok(chan) @@ -1193,6 +1245,11 @@ impl Channel { let mut value_to_a = if local { value_to_self } else { value_to_remote }; let mut value_to_b = if local { value_to_remote } else { value_to_self }; + let (funding_pubkey_a, funding_pubkey_b) = if local { + (self.get_holder_pubkeys().funding_pubkey, self.get_counterparty_pubkeys().funding_pubkey) + } else { + (self.get_counterparty_pubkeys().funding_pubkey, self.get_holder_pubkeys().funding_pubkey) + }; if value_to_a >= (broadcaster_dust_limit_satoshis as i64) { log_trace!(logger, " ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, value_to_a); @@ -1214,6 +1271,9 @@ impl Channel { let tx = CommitmentTransaction::new_with_auxiliary_htlc_data(commitment_number, value_to_a as u64, value_to_b as u64, + false, + funding_pubkey_a, + funding_pubkey_b, keys.clone(), feerate_per_kw, &mut included_non_dust_htlcs, @@ -1263,63 +1323,38 @@ impl Channel { } #[inline] - fn build_closing_transaction(&self, proposed_total_fee_satoshis: u64, skip_remote_output: bool) -> (Transaction, u64) { - let txins = { - let mut ins: Vec = Vec::new(); - ins.push(TxIn { - previous_output: self.funding_outpoint().into_bitcoin_outpoint(), - script_sig: Script::new(), - sequence: 0xffffffff, - witness: Vec::new(), - }); - ins - }; - + fn build_closing_transaction(&self, proposed_total_fee_satoshis: u64, skip_remote_output: bool) -> (ClosingTransaction, u64) { assert!(self.pending_inbound_htlcs.is_empty()); assert!(self.pending_outbound_htlcs.is_empty()); assert!(self.pending_update_fee.is_none()); - let mut txouts: Vec<(TxOut, ())> = Vec::new(); let mut total_fee_satoshis = proposed_total_fee_satoshis; - let value_to_self: i64 = (self.value_to_self_msat as i64) / 1000 - if self.is_outbound() { total_fee_satoshis as i64 } else { 0 }; - let value_to_remote: i64 = ((self.channel_value_satoshis * 1000 - self.value_to_self_msat) as i64 / 1000) - if self.is_outbound() { 0 } else { total_fee_satoshis as i64 }; + let mut value_to_holder: i64 = (self.value_to_self_msat as i64) / 1000 - if self.is_outbound() { total_fee_satoshis as i64 } else { 0 }; + let mut value_to_counterparty: i64 = ((self.channel_value_satoshis * 1000 - self.value_to_self_msat) as i64 / 1000) - if self.is_outbound() { 0 } else { total_fee_satoshis as i64 }; - if value_to_self < 0 { + if value_to_holder < 0 { assert!(self.is_outbound()); - total_fee_satoshis += (-value_to_self) as u64; - } else if value_to_remote < 0 { + total_fee_satoshis += (-value_to_holder) as u64; + } else if value_to_counterparty < 0 { assert!(!self.is_outbound()); - total_fee_satoshis += (-value_to_remote) as u64; + total_fee_satoshis += (-value_to_counterparty) as u64; } - if !skip_remote_output && value_to_remote as u64 > self.holder_dust_limit_satoshis { - txouts.push((TxOut { - script_pubkey: self.counterparty_shutdown_scriptpubkey.clone().unwrap(), - value: value_to_remote as u64 - }, ())); + if skip_remote_output || value_to_counterparty as u64 <= self.holder_dust_limit_satoshis { + value_to_counterparty = 0; } - 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(), - value: value_to_self as u64 - }, ())); + if value_to_holder as u64 <= self.holder_dust_limit_satoshis { + value_to_holder = 0; } - transaction_utils::sort_outputs(&mut txouts, |_, _| { cmp::Ordering::Equal }); // Ordering doesnt matter if they used our pubkey... - - let mut outputs: Vec = Vec::new(); - for out in txouts.drain(..) { - outputs.push(out.0); - } + assert!(self.shutdown_scriptpubkey.is_some()); + let holder_shutdown_script = self.get_closing_scriptpubkey(); + let counterparty_shutdown_script = self.counterparty_shutdown_scriptpubkey.clone().unwrap(); + let funding_outpoint = self.funding_outpoint().into_bitcoin_outpoint(); - (Transaction { - version: 2, - lock_time: 0, - input: txins, - output: outputs, - }, total_fee_satoshis) + let closing_transaction = ClosingTransaction::new(value_to_holder as u64, value_to_counterparty as u64, holder_shutdown_script, counterparty_shutdown_script, funding_outpoint); + (closing_transaction, total_fee_satoshis) } fn funding_outpoint(&self) -> OutPoint { @@ -1635,11 +1670,11 @@ impl Channel { if msg.max_accepted_htlcs < config.peer_channel_config_limits.min_max_accepted_htlcs { return Err(ChannelError::Close(format!("max_accepted_htlcs ({}) is less than the user specified limit ({})", msg.max_accepted_htlcs, config.peer_channel_config_limits.min_max_accepted_htlcs))); } - if msg.dust_limit_satoshis < MIN_DUST_LIMIT_SATOSHIS { - return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is less than the implementation limit ({})", msg.dust_limit_satoshis, MIN_DUST_LIMIT_SATOSHIS))); + if msg.dust_limit_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS { + return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is less than the implementation limit ({})", msg.dust_limit_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS))); } - if msg.dust_limit_satoshis > MAX_DUST_LIMIT_SATOSHIS { - return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is greater than the implementation limit ({})", msg.dust_limit_satoshis, MAX_DUST_LIMIT_SATOSHIS))); + if msg.dust_limit_satoshis > MAX_CHAN_DUST_LIMIT_SATOSHIS { + return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is greater than the implementation limit ({})", msg.dust_limit_satoshis, MAX_CHAN_DUST_LIMIT_SATOSHIS))); } 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))); @@ -1658,10 +1693,10 @@ impl Channel { if script.len() == 0 { None } else { - 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 an unacceptable scriptpubkey format: {}", script))), + if !script::is_bolt2_compliant(&script, their_features) { + return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided an unacceptable scriptpubkey format: {}", script))); } + Some(script.clone()) } }, // 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 @@ -1778,6 +1813,9 @@ impl Channel { self.counterparty_funding_pubkey() ); + self.holder_signer.validate_holder_commitment(&holder_commitment_tx) + .map_err(|_| ChannelError::Close("Failed to validate our commitment".to_owned()))?; + // Now that we're past error-generating stuff, update our local state: let funding_redeemscript = self.get_funding_redeemscript(); @@ -1852,6 +1890,9 @@ impl Channel { self.counterparty_funding_pubkey() ); + self.holder_signer.validate_holder_commitment(&holder_commitment_tx) + .map_err(|_| ChannelError::Close("Failed to validate our commitment".to_owned()))?; + let funding_redeemscript = self.get_funding_redeemscript(); let funding_txo = self.get_funding_txo().unwrap(); @@ -1915,6 +1956,15 @@ impl Channel { Ok(()) } + /// Returns transaction if there is pending funding transaction that is yet to broadcast + pub fn unbroadcasted_funding(&self) -> Option { + if self.channel_state & (ChannelState::FundingCreated as u32) != 0 { + self.funding_transaction.clone() + } else { + None + } + } + /// Returns a HTLCStats about inbound pending htlcs fn get_inbound_pending_htlc_stats(&self) -> HTLCStats { let mut stats = HTLCStats { @@ -2179,7 +2229,7 @@ impl Channel { // We can't accept HTLCs sent after we've sent a shutdown. let local_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::LocalShutdownSent as u32)) != (ChannelState::ChannelFunded as u32); if local_sent_shutdown { - pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|20); + pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x4000|8); } // If the remote has sent a shutdown prior to adding this HTLC, then they are in violation of the spec. let remote_sent_shutdown = (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::RemoteShutdownSent as u32)) != (ChannelState::ChannelFunded as u32); @@ -2489,6 +2539,8 @@ impl Channel { ); let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number - 1, &self.secp_ctx); + self.holder_signer.validate_holder_commitment(&holder_commitment_tx) + .map_err(|_| (None, ChannelError::Close("Failed to validate our commitment".to_owned())))?; let per_commitment_secret = self.holder_signer.release_commitment_secret(self.cur_holder_commitment_transaction_number + 1); // Update state now that we've passed all the can-fail calls... @@ -2712,7 +2764,7 @@ impl Channel { /// waiting on this revoke_and_ack. The generation of this new commitment_signed may also fail, /// generating an appropriate error *after* the channel state has been updated based on the /// revoke_and_ack message. - pub fn revoke_and_ack(&mut self, msg: &msgs::RevokeAndACK, logger: &L) -> Result<(Option, Vec<(PendingHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>), ChannelError> + pub fn revoke_and_ack(&mut self, msg: &msgs::RevokeAndACK, logger: &L) -> Result where L::Target: Logger, { if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) { @@ -2725,8 +2777,10 @@ impl Channel { return Err(ChannelError::Close("Peer sent revoke_and_ack after we'd started exchanging closing_signeds".to_owned())); } + let secret = secp_check!(SecretKey::from_slice(&msg.per_commitment_secret), "Peer provided an invalid per_commitment_secret".to_owned()); + if let Some(counterparty_prev_commitment_point) = self.counterparty_prev_commitment_point { - if PublicKey::from_secret_key(&self.secp_ctx, &secp_check!(SecretKey::from_slice(&msg.per_commitment_secret), "Peer provided an invalid per_commitment_secret".to_owned())) != counterparty_prev_commitment_point { + if PublicKey::from_secret_key(&self.secp_ctx, &secret) != counterparty_prev_commitment_point { return Err(ChannelError::Close("Got a revoke commitment secret which didn't correspond to their current pubkey".to_owned())); } } @@ -2748,6 +2802,11 @@ impl Channel { *self.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None; } + self.holder_signer.validate_counterparty_revocation( + self.cur_counterparty_commitment_transaction_number + 1, + &secret + ).map_err(|_| ChannelError::Close("Failed to validate revocation from peer".to_owned()))?; + self.commitment_secrets.provide_secret(self.cur_counterparty_commitment_transaction_number + 1, msg.per_commitment_secret) .map_err(|_| ChannelError::Close("Previous secrets did not match new one".to_owned()))?; self.latest_monitor_update_id += 1; @@ -2771,6 +2830,7 @@ impl Channel { log_trace!(logger, "Updating HTLCs on receipt of RAA in channel {}...", log_bytes!(self.channel_id())); let mut to_forward_infos = Vec::new(); let mut revoked_htlcs = Vec::new(); + let mut finalized_claimed_htlcs = Vec::new(); let mut update_fail_htlcs = Vec::new(); let mut update_fail_malformed_htlcs = Vec::new(); let mut require_commitment = false; @@ -2797,6 +2857,7 @@ impl Channel { if let Some(reason) = fail_reason.clone() { // We really want take() here, but, again, non-mut ref :( revoked_htlcs.push((htlc.source.clone(), htlc.payment_hash, reason)); } else { + finalized_claimed_htlcs.push(htlc.source.clone()); // They fulfilled, so we sent them money value_to_self_msat_diff -= htlc.amount_msat as i64; } @@ -2893,8 +2954,14 @@ impl Channel { } self.monitor_pending_forwards.append(&mut to_forward_infos); self.monitor_pending_failures.append(&mut revoked_htlcs); + self.monitor_pending_finalized_fulfills.append(&mut finalized_claimed_htlcs); log_debug!(logger, "Received a valid revoke_and_ack for channel {} but awaiting a monitor update resolution to reply.", log_bytes!(self.channel_id())); - return Ok((None, Vec::new(), Vec::new(), monitor_update, Vec::new())) + return Ok(RAAUpdates { + commitment_update: None, finalized_claimed_htlcs: Vec::new(), + accepted_htlcs: Vec::new(), failed_htlcs: Vec::new(), + monitor_update, + holding_cell_failed_htlcs: Vec::new() + }); } match self.free_holding_cell_htlcs(logger)? { @@ -2913,7 +2980,14 @@ impl Channel { self.latest_monitor_update_id = monitor_update.update_id; monitor_update.updates.append(&mut additional_update.updates); - Ok((Some(commitment_update), to_forward_infos, revoked_htlcs, monitor_update, htlcs_to_fail)) + Ok(RAAUpdates { + commitment_update: Some(commitment_update), + finalized_claimed_htlcs, + accepted_htlcs: to_forward_infos, + failed_htlcs: revoked_htlcs, + monitor_update, + holding_cell_failed_htlcs: htlcs_to_fail + }) }, (None, htlcs_to_fail) => { if require_commitment { @@ -2926,17 +3000,27 @@ impl Channel { log_debug!(logger, "Received a valid revoke_and_ack for channel {}. Responding with a commitment update with {} HTLCs failed.", log_bytes!(self.channel_id()), update_fail_htlcs.len() + update_fail_malformed_htlcs.len()); - Ok((Some(msgs::CommitmentUpdate { - update_add_htlcs: Vec::new(), - update_fulfill_htlcs: Vec::new(), - update_fail_htlcs, - update_fail_malformed_htlcs, - update_fee: None, - commitment_signed - }), to_forward_infos, revoked_htlcs, monitor_update, htlcs_to_fail)) + Ok(RAAUpdates { + commitment_update: Some(msgs::CommitmentUpdate { + update_add_htlcs: Vec::new(), + update_fulfill_htlcs: Vec::new(), + update_fail_htlcs, + update_fail_malformed_htlcs, + update_fee: None, + commitment_signed + }), + finalized_claimed_htlcs, + accepted_htlcs: to_forward_infos, failed_htlcs: revoked_htlcs, + monitor_update, holding_cell_failed_htlcs: htlcs_to_fail + }) } else { log_debug!(logger, "Received a valid revoke_and_ack for channel {} with no reply necessary.", log_bytes!(self.channel_id())); - Ok((None, to_forward_infos, revoked_htlcs, monitor_update, htlcs_to_fail)) + Ok(RAAUpdates { + commitment_update: None, + finalized_claimed_htlcs, + accepted_htlcs: to_forward_infos, failed_htlcs: revoked_htlcs, + monitor_update, holding_cell_failed_htlcs: htlcs_to_fail + }) } } } @@ -3051,21 +3135,23 @@ impl Channel { /// which failed. The messages which were generated from that call which generated the /// monitor update failure must *not* have been sent to the remote end, and must instead /// have been dropped. They will be regenerated when monitor_updating_restored is called. - pub fn monitor_update_failed(&mut self, resend_raa: bool, resend_commitment: bool, mut pending_forwards: Vec<(PendingHTLCInfo, u64)>, mut pending_fails: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>) { - assert_eq!(self.channel_state & ChannelState::MonitorUpdateFailed as u32, 0); - self.monitor_pending_revoke_and_ack = resend_raa; - self.monitor_pending_commitment_signed = resend_commitment; - assert!(self.monitor_pending_forwards.is_empty()); - mem::swap(&mut pending_forwards, &mut self.monitor_pending_forwards); - assert!(self.monitor_pending_failures.is_empty()); - mem::swap(&mut pending_fails, &mut self.monitor_pending_failures); + pub fn monitor_update_failed(&mut self, resend_raa: bool, resend_commitment: bool, + mut pending_forwards: Vec<(PendingHTLCInfo, u64)>, + mut pending_fails: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, + mut pending_finalized_claimed_htlcs: Vec + ) { + self.monitor_pending_revoke_and_ack |= resend_raa; + self.monitor_pending_commitment_signed |= resend_commitment; + self.monitor_pending_forwards.append(&mut pending_forwards); + self.monitor_pending_failures.append(&mut pending_fails); + self.monitor_pending_finalized_fulfills.append(&mut pending_finalized_claimed_htlcs); self.channel_state |= ChannelState::MonitorUpdateFailed as u32; } /// Indicates that the latest ChannelMonitor update has been committed by the client /// successfully and we should restore normal operation. Returns messages which should be sent /// to the remote side. - pub fn monitor_updating_restored(&mut self, logger: &L) -> (Option, Option, RAACommitmentOrder, Vec<(PendingHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, Option, Option) where L::Target: Logger { + pub fn monitor_updating_restored(&mut self, logger: &L) -> MonitorRestoreUpdates where L::Target: Logger { assert_eq!(self.channel_state & ChannelState::MonitorUpdateFailed as u32, ChannelState::MonitorUpdateFailed as u32); self.channel_state &= !(ChannelState::MonitorUpdateFailed as u32); @@ -3088,15 +3174,20 @@ impl Channel { }) } else { None }; - let mut forwards = Vec::new(); - mem::swap(&mut forwards, &mut self.monitor_pending_forwards); - let mut failures = Vec::new(); - mem::swap(&mut failures, &mut self.monitor_pending_failures); + let mut accepted_htlcs = Vec::new(); + mem::swap(&mut accepted_htlcs, &mut self.monitor_pending_forwards); + let mut failed_htlcs = Vec::new(); + mem::swap(&mut failed_htlcs, &mut self.monitor_pending_failures); + let mut finalized_claimed_htlcs = Vec::new(); + mem::swap(&mut finalized_claimed_htlcs, &mut self.monitor_pending_finalized_fulfills); if self.channel_state & (ChannelState::PeerDisconnected as u32) != 0 { self.monitor_pending_revoke_and_ack = false; self.monitor_pending_commitment_signed = false; - return (None, None, RAACommitmentOrder::RevokeAndACKFirst, forwards, failures, funding_broadcastable, funding_locked); + return MonitorRestoreUpdates { + raa: None, commitment_update: None, order: RAACommitmentOrder::RevokeAndACKFirst, + accepted_htlcs, failed_htlcs, finalized_claimed_htlcs, funding_broadcastable, funding_locked + }; } let raa = if self.monitor_pending_revoke_and_ack { @@ -3113,7 +3204,9 @@ impl Channel { log_bytes!(self.channel_id()), if funding_broadcastable.is_some() { "a funding broadcastable, " } else { "" }, if commitment_update.is_some() { "a" } else { "no" }, if raa.is_some() { "an" } else { "no" }, match order { RAACommitmentOrder::CommitmentFirst => "commitment", RAACommitmentOrder::RevokeAndACKFirst => "RAA"}); - (raa, commitment_update, order, forwards, failures, funding_broadcastable, funding_locked) + MonitorRestoreUpdates { + raa, commitment_update, order, accepted_htlcs, failed_htlcs, finalized_claimed_htlcs, funding_broadcastable, funding_locked + } } pub fn update_fee(&mut self, fee_estimator: &F, msg: &msgs::UpdateFee) -> Result<(), ChannelError> @@ -3406,23 +3499,45 @@ impl Channel { cmp::max(normal_feerate as u64 * tx_weight / 1000 + self.config.force_close_avoidance_max_fee_satoshis, proposed_max_feerate as u64 * tx_weight / 1000) } else { - u64::max_value() + self.channel_value_satoshis - (self.value_to_self_msat + 999) / 1000 }; self.closing_fee_limits = Some((proposed_total_fee_satoshis, proposed_max_total_fee_satoshis)); self.closing_fee_limits.clone().unwrap() } + /// Returns true if we're ready to commence the closing_signed negotiation phase. This is true + /// after both sides have exchanged a `shutdown` message and all HTLCs have been drained. At + /// this point if we're the funder we should send the initial closing_signed, and in any case + /// shutdown should complete within a reasonable timeframe. + fn closing_negotiation_ready(&self) -> bool { + self.pending_inbound_htlcs.is_empty() && self.pending_outbound_htlcs.is_empty() && + self.channel_state & + (BOTH_SIDES_SHUTDOWN_MASK | ChannelState::AwaitingRemoteRevoke as u32 | + ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32) + == BOTH_SIDES_SHUTDOWN_MASK && + self.pending_update_fee.is_none() + } + + /// Checks if the closing_signed negotiation is making appropriate progress, possibly returning + /// an Err if no progress is being made and the channel should be force-closed instead. + /// Should be called on a one-minute timer. + pub fn timer_check_closing_negotiation_progress(&mut self) -> Result<(), ChannelError> { + if self.closing_negotiation_ready() { + if self.closing_signed_in_flight { + return Err(ChannelError::Close("closing_signed negotiation failed to finish within two timer ticks".to_owned())); + } else { + self.closing_signed_in_flight = true; + } + } + Ok(()) + } + pub fn maybe_propose_closing_signed(&mut self, fee_estimator: &F, logger: &L) -> Result<(Option, Option), ChannelError> where F::Target: FeeEstimator, L::Target: Logger { - if !self.pending_inbound_htlcs.is_empty() || !self.pending_outbound_htlcs.is_empty() || - self.channel_state & - (BOTH_SIDES_SHUTDOWN_MASK | ChannelState::AwaitingRemoteRevoke as u32 | - ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32) - != BOTH_SIDES_SHUTDOWN_MASK || - self.last_sent_closing_fee.is_some() || self.pending_update_fee.is_some() { + if self.last_sent_closing_fee.is_some() || !self.closing_negotiation_ready() { return Ok((None, None)); } @@ -3477,17 +3592,16 @@ impl Channel { } assert_eq!(self.channel_state & ChannelState::ShutdownComplete as u32, 0); - 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 !script::is_bolt2_compliant(&msg.scriptpubkey, their_features) { + 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(&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()))); + 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()))); } } else { - self.counterparty_shutdown_scriptpubkey = Some(shutdown_scriptpubkey); + self.counterparty_shutdown_scriptpubkey = Some(msg.scriptpubkey.clone()); } // If we have any LocalAnnounced updates we'll probably just get back an update_fail_htlc @@ -3550,10 +3664,8 @@ impl Channel { Ok((shutdown, monitor_update, dropped_outbound_htlcs)) } - fn build_signed_closing_transaction(&self, tx: &mut Transaction, counterparty_sig: &Signature, sig: &Signature) { - if tx.input.len() != 1 { panic!("Tried to sign closing transaction that had input count != 1!"); } - if tx.input[0].witness.len() != 0 { panic!("Tried to re-sign closing transaction"); } - if tx.output.len() > 2 { panic!("Tried to sign bogus closing transaction"); } + fn build_signed_closing_transaction(&self, closing_tx: &ClosingTransaction, counterparty_sig: &Signature, sig: &Signature) -> Transaction { + let mut tx = closing_tx.trust().built_transaction().clone(); tx.input[0].witness.push(Vec::new()); // First is the multisig dummy @@ -3570,6 +3682,7 @@ impl Channel { tx.input[0].witness[2].push(SigHashType::All as u8); tx.input[0].witness.push(self.get_funding_redeemscript().into_bytes()); + tx } pub fn closing_signed(&mut self, fee_estimator: &F, msg: &msgs::ClosingSigned) -> Result<(Option, Option), ChannelError> @@ -3600,9 +3713,9 @@ impl Channel { let funding_redeemscript = self.get_funding_redeemscript(); let (mut closing_tx, used_total_fee) = self.build_closing_transaction(msg.fee_satoshis, false); if used_total_fee != msg.fee_satoshis { - return Err(ChannelError::Close(format!("Remote sent us a closing_signed with a fee greater than the value they can claim. Fee in message: {}", msg.fee_satoshis))); + return Err(ChannelError::Close(format!("Remote sent us a closing_signed with a fee other than the value they can claim. Fee in message: {}. Actual closing tx fee: {}", msg.fee_satoshis, used_total_fee))); } - let mut sighash = hash_to_message!(&bip143::SigHashCache::new(&closing_tx).signature_hash(0, &funding_redeemscript, self.channel_value_satoshis, SigHashType::All)[..]); + let sighash = closing_tx.trust().get_sighash_all(&funding_redeemscript, self.channel_value_satoshis); match self.secp_ctx.verify(&sighash, &msg.signature, &self.get_counterparty_pubkeys().funding_pubkey) { Ok(_) => {}, @@ -3610,18 +3723,24 @@ impl Channel { // The remote end may have decided to revoke their output due to inconsistent dust // limits, so check for that case by re-checking the signature here. closing_tx = self.build_closing_transaction(msg.fee_satoshis, true).0; - sighash = hash_to_message!(&bip143::SigHashCache::new(&closing_tx).signature_hash(0, &funding_redeemscript, self.channel_value_satoshis, SigHashType::All)[..]); + let sighash = closing_tx.trust().get_sighash_all(&funding_redeemscript, self.channel_value_satoshis); secp_check!(self.secp_ctx.verify(&sighash, &msg.signature, self.counterparty_funding_pubkey()), "Invalid closing tx signature from peer".to_owned()); }, }; + for outp in closing_tx.trust().built_transaction().output.iter() { + if !outp.script_pubkey.is_witness_program() && outp.value < MAX_STD_OUTPUT_DUST_LIMIT_SATOSHIS { + return Err(ChannelError::Close("Remote sent us a closing_signed with a dust output. Always use segwit closing scripts!".to_owned())); + } + } + assert!(self.shutdown_scriptpubkey.is_some()); if let Some((last_fee, sig)) = self.last_sent_closing_fee { if last_fee == msg.fee_satoshis { - self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &sig); + let tx = self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &sig); self.channel_state = ChannelState::ShutdownComplete as u32; self.update_time_counter += 1; - return Ok((None, Some(closing_tx))); + return Ok((None, Some(tx))); } } @@ -3629,20 +3748,20 @@ impl Channel { macro_rules! propose_fee { ($new_fee: expr) => { - let (mut tx, used_fee) = if $new_fee == msg.fee_satoshis { + let (closing_tx, used_fee) = if $new_fee == msg.fee_satoshis { (closing_tx, $new_fee) } else { self.build_closing_transaction($new_fee, false) }; let sig = self.holder_signer - .sign_closing_transaction(&tx, &self.secp_ctx) + .sign_closing_transaction(&closing_tx, &self.secp_ctx) .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction".to_owned()))?; let signed_tx = if $new_fee == msg.fee_satoshis { self.channel_state = ChannelState::ShutdownComplete as u32; self.update_time_counter += 1; - self.build_signed_closing_transaction(&mut tx, &msg.signature, &sig); + let tx = self.build_signed_closing_transaction(&closing_tx, &msg.signature, &sig); Some(tx) } else { None }; @@ -3672,7 +3791,8 @@ impl Channel { if !self.is_outbound() { // They have to pay, so pick the highest fee in the overlapping range. - debug_assert_eq!(our_max_fee, u64::max_value()); // We should never set an upper bound + // We should never set an upper bound aside from their full balance + debug_assert_eq!(our_max_fee, self.channel_value_satoshis - (self.value_to_self_msat + 999) / 1000); propose_fee!(cmp::min(max_fee_satoshis, our_max_fee)); } else { if msg.fee_satoshis < our_min_fee || msg.fee_satoshis > our_max_fee { @@ -3816,6 +3936,8 @@ impl Channel { // more dust balance if the feerate increases when we have several HTLCs pending // which are near the dust limit. let mut feerate_per_kw = self.feerate_per_kw; + // If there's a pending update fee, use it to ensure we aren't under-estimating + // potential feerate updates coming soon. if let Some((feerate, _)) = self.pending_update_fee { feerate_per_kw = cmp::max(feerate_per_kw, feerate); } @@ -4188,6 +4310,7 @@ impl Channel { Some(script) => script.clone().into_inner(), None => Builder::new().into_script(), }), + channel_type: Some(self.channel_type.clone()), } } @@ -5064,9 +5187,10 @@ impl Writeable for Channel { if self.is_outbound() { self.pending_update_fee.map(|(a, _)| a).write(writer)?; } else if let Some((feerate, FeeUpdateState::AwaitingRemoteRevokeToAnnounce)) = self.pending_update_fee { - // As for inbound HTLCs, if the update was only announced and never committed, drop it. Some(feerate).write(writer)?; } else { + // As for inbound HTLCs, if the update was only announced and never committed in a + // commitment_signed, drop it. None::.write(writer)?; } self.holding_cell_update_fee.write(writer)?; @@ -5143,6 +5267,8 @@ impl Writeable for Channel { (5, self.config, required), (7, self.shutdown_scriptpubkey, option), (9, self.target_closing_feerate_sats_per_kw, option), + (11, self.monitor_pending_finalized_fulfills, vec_type), + (13, self.channel_type, required), }); Ok(()) @@ -5376,6 +5502,10 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel let mut announcement_sigs = None; let mut target_closing_feerate_sats_per_kw = None; + let mut monitor_pending_finalized_fulfills = Some(Vec::new()); + // Prior to supporting channel type negotiation, all of our channels were static_remotekey + // only, so we default to that if none was written. + let mut channel_type = Some(ChannelTypeFeatures::only_static_remote_key()); read_tlv_fields!(reader, { (0, announcement_sigs, option), (1, minimum_depth, option), @@ -5383,8 +5513,17 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel (5, config, option), // Note that if none is provided we will *not* overwrite the existing one. (7, shutdown_scriptpubkey, option), (9, target_closing_feerate_sats_per_kw, option), + (11, monitor_pending_finalized_fulfills, vec_type), + (13, channel_type, option), }); + let chan_features = channel_type.as_ref().unwrap(); + if chan_features.supports_unknown_bits() || chan_features.requires_unknown_bits() { + // If the channel was written by a new version and negotiated with features we don't + // understand yet, refuse to read it. + return Err(DecodeError::UnknownRequiredFeature); + } + let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&keys_source.get_secure_random_bytes()); @@ -5418,6 +5557,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel monitor_pending_commitment_signed, monitor_pending_forwards, monitor_pending_failures, + monitor_pending_finalized_fulfills: monitor_pending_finalized_fulfills.unwrap(), pending_update_fee, holding_cell_update_fee, @@ -5463,6 +5603,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel commitment_secrets, channel_update_status, + closing_signed_in_flight: false, announcement_sigs, @@ -5475,6 +5616,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel #[cfg(any(test, feature = "fuzztarget"))] historical_inbound_htlc_fulfills, + + channel_type: channel_type.unwrap(), }) } } @@ -5491,7 +5634,7 @@ mod tests { use bitcoin::hashes::hex::FromHex; use hex; use ln::{PaymentPreimage, PaymentHash}; - use ln::channelmanager::HTLCSource; + use ln::channelmanager::{HTLCSource, PaymentId}; use ln::channel::{Channel,InboundHTLCOutput,OutboundHTLCOutput,InboundHTLCState,OutboundHTLCState,HTLCOutputInCommitment,HTLCCandidate,HTLCInitiator,TxCreationKeys}; use ln::channel::MAX_FUNDING_SATOSHIS; use ln::features::InitFeatures; @@ -5665,6 +5808,9 @@ mod tests { path: Vec::new(), session_priv: SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(), first_hop_htlc_msat: 548, + payment_id: PaymentId([42; 32]), + payment_secret: None, + payee: None, } });