X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannel.rs;h=e39478bd740c063ce1f29a7dc853238ec75fcf48;hb=c0bbd4d91877b3f7eca5b6aba877257acf4eec0b;hp=2a3edd6845e7ad2dcd514d19de83935a7abb0ec8;hpb=f8caa325e51d8afb0cb65effd9cdb351ffda3fc7;p=rust-lightning diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 2a3edd68..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; @@ -63,6 +61,21 @@ pub struct ChannelValueStat { pub counterparty_dust_limit_msat: u64, } +#[derive(Clone, Copy, PartialEq)] +enum FeeUpdateState { + // Inbound states mirroring InboundHTLCState + RemoteAnnounced, + AwaitingRemoteRevokeToAnnounce, + // Note that we do not have a AwaitingAnnouncedRemoteRevoke variant here as it is universally + // handled the same as `Committed`, with the only exception in `InboundHTLCState` being the + // distinction of when we allow ourselves to forward the HTLC. Because we aren't "forwarding" + // the fee update anywhere, we can simply consider the fee update `Committed` immediately + // instead of setting it to AwaitingAnnouncedRemoteRevoke. + + // Outbound state can only be `LocalAnnounced` or `Committed` + Outbound, +} + enum InboundHTLCRemovalReason { FailRelay(msgs::OnionErrorPacket), FailMalformed(([u8; 32], u16)), @@ -240,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. @@ -298,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 { @@ -341,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 @@ -408,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. - pending_update_fee: Option, - // 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. + // 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)>, + // 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, @@ -438,7 +456,20 @@ pub(super) struct Channel { /// Max to_local and to_remote outputs in a remote-generated commitment transaction counterparty_max_commitment_tx_output: Mutex<(u64, u64)>, - last_sent_closing_fee: Option<(u32, u64, Signature)>, // (feerate, fee, holder_sig) + last_sent_closing_fee: Option<(u64, Signature)>, // (fee, holder_sig) + target_closing_feerate_sats_per_kw: Option, + + /// If our counterparty sent us a closing_signed while we were waiting for a `ChannelMonitor` + /// update, we need to delay processing it until later. We do that here by simply storing the + /// 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, @@ -481,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. @@ -512,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"))] @@ -534,31 +575,37 @@ 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 /// channel_id in ChannelManager. pub(super) enum ChannelError { Ignore(String), + Warn(String), Close(String), CloseDelayBroadcast(String), } @@ -567,6 +614,7 @@ impl fmt::Debug for ChannelError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { &ChannelError::Ignore(ref e) => write!(f, "Ignore : {}", e), + &ChannelError::Warn(ref e) => write!(f, "Warn : {}", e), &ChannelError::Close(ref e) => write!(f, "Close : {}", e), &ChannelError::CloseDelayBroadcast(ref e) => write!(f, "CloseDelayBroadcast : {}", e) } @@ -617,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) }); } @@ -671,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)), @@ -678,6 +727,9 @@ impl Channel { counterparty_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)), last_sent_closing_fee: None, + pending_counterparty_closing_signed: None, + closing_fee_limits: None, + target_closing_feerate_sats_per_kw: None, funding_tx_confirmed_in: None, funding_tx_confirmation_height: 0, @@ -685,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, @@ -713,6 +765,7 @@ impl Channel { commitment_secrets: CounterpartyCommitmentSecrets::new(), channel_update_status: ChannelUpdateStatus::Enabled, + closing_signed_in_flight: false, announcement_sigs: None, @@ -725,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(), }) } @@ -735,7 +793,12 @@ impl Channel { if feerate_per_kw < lower_limit { return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {}", feerate_per_kw, lower_limit))); } - let upper_limit = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64 * 2; + // We only bound the fee updates on the upper side to prevent completely absurd feerates, + // always accepting up to 25 sat/vByte or 10x our fee estimator's "High Priority" fee. + // We generally don't care too much if they set the feerate to something very high, but it + // could result in the channel being useless due to everything being dust. + let upper_limit = cmp::max(250 * 25, + fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64 * 10); if feerate_per_kw as u64 > upper_limit { return Err(ChannelError::Close(format!("Peer's feerate much too high. Actual: {}. Our expected upper limit: {}", feerate_per_kw, upper_limit))); } @@ -748,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 { @@ -813,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: @@ -834,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))); @@ -865,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 @@ -925,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)), @@ -932,6 +1013,9 @@ impl Channel { counterparty_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)), last_sent_closing_fee: None, + pending_counterparty_closing_signed: None, + closing_fee_limits: None, + target_closing_feerate_sats_per_kw: None, funding_tx_confirmed_in: None, funding_tx_confirmation_height: 0, @@ -940,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, @@ -971,6 +1055,7 @@ impl Channel { commitment_secrets: CounterpartyCommitmentSecrets::new(), channel_update_status: ChannelUpdateStatus::Enabled, + closing_signed_in_flight: false, announcement_sigs: None, @@ -983,6 +1068,8 @@ impl Channel { #[cfg(any(test, feature = "fuzztarget"))] historical_inbound_htlc_fulfills: HashSet::new(), + + channel_type, }; Ok(chan) @@ -1003,10 +1090,10 @@ impl Channel { /// which peer generated this transaction and "to whom" this transaction flows. /// Returns (the transaction info, the number of HTLC outputs which were present in the /// transaction, the list of HTLCs which were not ignored when building the transaction). - /// Note that below-dust HTLCs are included in the third return value, but not the second, and - /// sources are provided only for outbound HTLCs in the third return value. + /// Note that below-dust HTLCs are included in the fourth return value, but not the third, and + /// sources are provided only for outbound HTLCs in the fourth return value. #[inline] - fn build_commitment_transaction(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, feerate_per_kw: u32, logger: &L) -> (CommitmentTransaction, usize, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>) where L::Target: Logger { + fn build_commitment_transaction(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, logger: &L) -> (CommitmentTransaction, u32, usize, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>) where L::Target: Logger { let mut included_dust_htlcs: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::new(); let num_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len(); let mut included_non_dust_htlcs: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::with_capacity(num_htlcs); @@ -1016,6 +1103,19 @@ impl Channel { let mut local_htlc_total_msat = 0; let mut value_to_self_msat_offset = 0; + let mut feerate_per_kw = self.feerate_per_kw; + if let Some((feerate, update_state)) = self.pending_update_fee { + if match update_state { + // Note that these match the inclusion criteria when scanning + // pending_inbound_htlcs below. + FeeUpdateState::RemoteAnnounced => { debug_assert!(!self.is_outbound()); !generated_by_local }, + FeeUpdateState::AwaitingRemoteRevokeToAnnounce => { debug_assert!(!self.is_outbound()); !generated_by_local }, + FeeUpdateState::Outbound => { assert!(self.is_outbound()); generated_by_local }, + } { + feerate_per_kw = feerate; + } + } + log_trace!(logger, "Building commitment transaction number {} (really {} xor {}) for channel {} for {}, generated by {} with fee {}...", commitment_number, (INITIAL_COMMITMENT_NUMBER - commitment_number), get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound()), @@ -1145,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); @@ -1166,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, @@ -1176,7 +1284,7 @@ impl Channel { htlcs_included.sort_unstable_by_key(|h| h.0.transaction_output_index.unwrap()); htlcs_included.append(&mut included_dust_htlcs); - (tx, num_nondust_htlcs, htlcs_included) + (tx, feerate_per_kw, num_nondust_htlcs, htlcs_included) } #[inline] @@ -1215,62 +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()); - let mut txouts: Vec<(TxOut, ())> = Vec::new(); + assert!(self.pending_update_fee.is_none()); 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 { @@ -1586,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))); @@ -1609,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 @@ -1654,7 +1738,7 @@ impl Channel { let funding_script = self.get_funding_redeemscript(); let keys = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number)?; - let initial_commitment_tx = self.build_commitment_transaction(self.cur_holder_commitment_transaction_number, &keys, true, false, self.feerate_per_kw, logger).0; + let initial_commitment_tx = self.build_commitment_transaction(self.cur_holder_commitment_transaction_number, &keys, true, false, logger).0; { let trusted_tx = initial_commitment_tx.trust(); let initial_commitment_bitcoin_tx = trusted_tx.built_transaction(); @@ -1668,7 +1752,7 @@ impl Channel { } let counterparty_keys = self.build_remote_transaction_keys()?; - let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, self.feerate_per_kw, logger).0; + let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).0; let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust(); let counterparty_initial_bitcoin_tx = counterparty_trusted_tx.built_transaction(); @@ -1729,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(); @@ -1776,7 +1863,7 @@ impl Channel { let funding_script = self.get_funding_redeemscript(); let counterparty_keys = self.build_remote_transaction_keys()?; - let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, self.feerate_per_kw, logger).0; + let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).0; let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust(); let counterparty_initial_bitcoin_tx = counterparty_trusted_tx.built_transaction(); @@ -1784,7 +1871,7 @@ impl Channel { log_bytes!(self.channel_id()), counterparty_initial_bitcoin_tx.txid, encode::serialize_hex(&counterparty_initial_bitcoin_tx.transaction)); let holder_signer = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number)?; - let initial_commitment_tx = self.build_commitment_transaction(self.cur_holder_commitment_transaction_number, &holder_signer, true, false, self.feerate_per_kw, logger).0; + let initial_commitment_tx = self.build_commitment_transaction(self.cur_holder_commitment_transaction_number, &holder_signer, true, false, logger).0; { let trusted_tx = initial_commitment_tx.trust(); let initial_commitment_bitcoin_tx = trusted_tx.built_transaction(); @@ -1803,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(); @@ -1866,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 { @@ -2130,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); @@ -2337,9 +2436,8 @@ impl Channel { Ok(()) } - pub fn commitment_signed(&mut self, msg: &msgs::CommitmentSigned, fee_estimator: &F, logger: &L) -> Result<(msgs::RevokeAndACK, Option, Option, ChannelMonitorUpdate), (Option, ChannelError)> - where F::Target: FeeEstimator, - L::Target: Logger + pub fn commitment_signed(&mut self, msg: &msgs::CommitmentSigned, logger: &L) -> Result<(msgs::RevokeAndACK, Option, ChannelMonitorUpdate), (Option, ChannelError)> + where L::Target: Logger { if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) { return Err((None, ChannelError::Close("Got commitment signed message when channel was not in an operational state".to_owned()))); @@ -2355,16 +2453,8 @@ impl Channel { let keys = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number).map_err(|e| (None, e))?; - let mut update_fee = false; - let feerate_per_kw = if !self.is_outbound() && self.pending_update_fee.is_some() { - update_fee = true; - self.pending_update_fee.unwrap() - } else { - self.feerate_per_kw - }; - - let (num_htlcs, mut htlcs_cloned, commitment_tx, commitment_txid) = { - let commitment_tx = self.build_commitment_transaction(self.cur_holder_commitment_transaction_number, &keys, true, false, feerate_per_kw, logger); + let (num_htlcs, mut htlcs_cloned, commitment_tx, commitment_txid, feerate_per_kw) = { + let commitment_tx = self.build_commitment_transaction(self.cur_holder_commitment_transaction_number, &keys, true, false, logger); let commitment_txid = { let trusted_tx = commitment_tx.0.trust(); let bitcoin_tx = trusted_tx.built_transaction(); @@ -2379,12 +2469,17 @@ impl Channel { } bitcoin_tx.txid }; - let htlcs_cloned: Vec<_> = commitment_tx.2.iter().map(|htlc| (htlc.0.clone(), htlc.1.map(|h| h.clone()))).collect(); - (commitment_tx.1, htlcs_cloned, commitment_tx.0, commitment_txid) + let htlcs_cloned: Vec<_> = commitment_tx.3.iter().map(|htlc| (htlc.0.clone(), htlc.1.map(|h| h.clone()))).collect(); + (commitment_tx.2, htlcs_cloned, commitment_tx.0, commitment_txid, commitment_tx.1) }; + // If our counterparty updated the channel fee in this commitment transaction, check that + // they can actually afford the new fee now. + let update_fee = if let Some((_, update_state)) = self.pending_update_fee { + update_state == FeeUpdateState::RemoteAnnounced + } else { false }; + if update_fee { debug_assert!(!self.is_outbound()); } let total_fee = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; - //If channel fee was updated by funder confirm funder can afford the new fee rate when applied to the current local commitment transaction if update_fee { let counterparty_reserve_we_require = Channel::::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis); if self.channel_value_satoshis - self.value_to_self_msat / 1000 < total_fee + counterparty_reserve_we_require { @@ -2444,20 +2539,16 @@ 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... let mut need_commitment = false; - if !self.is_outbound() { - if let Some(fee_update) = self.pending_update_fee { - self.feerate_per_kw = fee_update; - // We later use the presence of pending_update_fee to indicate we should generate a - // commitment_signed upon receipt of revoke_and_ack, so we can only set it to None - // if we're not awaiting a revoke (ie will send a commitment_signed now). - if (self.channel_state & ChannelState::AwaitingRemoteRevoke as u32) == 0 { - need_commitment = true; - self.pending_update_fee = None; - } + if let &mut Some((_, ref mut update_state)) = &mut self.pending_update_fee { + if *update_state == FeeUpdateState::RemoteAnnounced { + *update_state = FeeUpdateState::AwaitingRemoteRevokeToAnnounce; + need_commitment = true; } } @@ -2514,12 +2605,10 @@ impl Channel { } log_debug!(logger, "Received valid commitment_signed from peer in channel {}, updated HTLC state but awaiting a monitor update resolution to reply.", log_bytes!(self.channel_id)); - // TODO: Call maybe_propose_first_closing_signed on restoration (or call it here and - // re-send the message on restoration) return Err((Some(monitor_update), ChannelError::Ignore("Previous monitor update failure prevented generation of RAA".to_owned()))); } - let (commitment_signed, closing_signed) = if need_commitment && (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == 0 { + let commitment_signed = if need_commitment && (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == 0 { // If we're AwaitingRemoteRevoke we can't send a new commitment here, but that's ok - // we'll send one right away when we get the revoke_and_ack when we // free_holding_cell_htlcs(). @@ -2528,10 +2617,8 @@ impl Channel { // 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); - (Some(msg), None) - } else if !need_commitment { - (None, self.maybe_propose_first_closing_signed(fee_estimator)) - } else { (None, None) }; + Some(msg) + } else { None }; log_debug!(logger, "Received valid commitment_signed from peer in channel {}, updating HTLC state and responding with{} a revoke_and_ack.", log_bytes!(self.channel_id()), if commitment_signed.is_some() { " our own commitment_signed and" } else { "" }); @@ -2540,7 +2627,7 @@ impl Channel { channel_id: self.channel_id, per_commitment_secret, next_per_commitment_point, - }, commitment_signed, closing_signed, monitor_update)) + }, commitment_signed, monitor_update)) } /// Public version of the below, checking relevant preconditions first. @@ -2638,8 +2725,9 @@ impl Channel { if update_add_htlcs.is_empty() && update_fulfill_htlcs.is_empty() && update_fail_htlcs.is_empty() && self.holding_cell_update_fee.is_none() { return Ok((None, htlcs_to_fail)); } - let update_fee = if let Some(feerate) = self.holding_cell_update_fee { - self.pending_update_fee = self.holding_cell_update_fee.take(); + let update_fee = if let Some(feerate) = self.holding_cell_update_fee.take() { + assert!(self.is_outbound()); + self.pending_update_fee = Some((feerate, FeeUpdateState::Outbound)); Some(msgs::UpdateFee { channel_id: self.channel_id, feerate_per_kw: feerate as u32, @@ -2676,9 +2764,8 @@ 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, fee_estimator: &F, logger: &L) -> Result<(Option, Vec<(PendingHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, Option, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>), ChannelError> - where F::Target: FeeEstimator, - L::Target: Logger, + 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) { return Err(ChannelError::Close("Got revoke/ACK message when channel was not in an operational state".to_owned())); @@ -2690,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())); } } @@ -2713,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; @@ -2736,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; @@ -2762,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; } @@ -2823,21 +2919,22 @@ impl Channel { } self.value_to_self_msat = (self.value_to_self_msat as i64 + value_to_self_msat_diff) as u64; - if self.is_outbound() { - if let Some(feerate) = self.pending_update_fee.take() { - self.feerate_per_kw = feerate; - } - } else { - if let Some(feerate) = self.pending_update_fee { - // Because a node cannot send two commitment_signeds in a row without getting a - // revoke_and_ack from us (as it would otherwise not know the per_commitment_point - // it should use to create keys with) and because a node can't send a - // commitment_signed without changes, checking if the feerate is equal to the - // pending feerate update is sufficient to detect require_commitment. - if feerate == self.feerate_per_kw { + if let Some((feerate, update_state)) = self.pending_update_fee { + match update_state { + FeeUpdateState::Outbound => { + debug_assert!(self.is_outbound()); + log_trace!(logger, " ...promoting outbound fee update {} to Committed", feerate); + self.feerate_per_kw = feerate; + self.pending_update_fee = None; + }, + FeeUpdateState::RemoteAnnounced => { debug_assert!(!self.is_outbound()); }, + FeeUpdateState::AwaitingRemoteRevokeToAnnounce => { + debug_assert!(!self.is_outbound()); + log_trace!(logger, " ...promoting inbound AwaitingRemoteRevokeToAnnounce fee update {} to Committed", feerate); require_commitment = true; + self.feerate_per_kw = feerate; self.pending_update_fee = None; - } + }, } } @@ -2857,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(), None, 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)? { @@ -2877,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, None, 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 { @@ -2890,21 +3000,30 @@ 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, None, 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, self.maybe_propose_first_closing_signed(fee_estimator), 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 + }) } } } - } /// Adds a pending update to this channel. See the doc for send_htlc for @@ -2927,7 +3046,7 @@ impl Channel { } debug_assert!(self.pending_update_fee.is_none()); - self.pending_update_fee = Some(feerate_per_kw); + self.pending_update_fee = Some((feerate_per_kw, FeeUpdateState::Outbound)); Some(msgs::UpdateFee { channel_id: self.channel_id, @@ -2959,6 +3078,8 @@ impl Channel { // Upon reconnect we have to start the closing_signed dance over, but shutdown messages // will be retransmitted. self.last_sent_closing_fee = None; + self.pending_counterparty_closing_signed = None; + self.closing_fee_limits = None; let mut inbound_drop_count = 0; self.pending_inbound_htlcs.retain(|htlc| { @@ -2988,6 +3109,13 @@ impl Channel { }); self.next_counterparty_htlc_id -= inbound_drop_count; + if let Some((_, update_state)) = self.pending_update_fee { + if update_state == FeeUpdateState::RemoteAnnounced { + debug_assert!(!self.is_outbound()); + self.pending_update_fee = None; + } + } + for htlc in self.pending_outbound_htlcs.iter_mut() { if let OutboundHTLCState::RemoteRemoved(_) = htlc.state { // They sent us an update to remove this but haven't yet sent the corresponding @@ -3007,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); @@ -3044,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 { @@ -3069,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> @@ -3082,8 +3219,27 @@ impl Channel { return Err(ChannelError::Close("Peer sent update_fee when we needed a channel_reestablish".to_owned())); } Channel::::check_remote_fee(fee_estimator, msg.feerate_per_kw)?; - self.pending_update_fee = Some(msg.feerate_per_kw); + let feerate_over_dust_buffer = msg.feerate_per_kw > self.get_dust_buffer_feerate(); + + self.pending_update_fee = Some((msg.feerate_per_kw, FeeUpdateState::RemoteAnnounced)); self.update_time_counter += 1; + // If the feerate has increased over the previous dust buffer (note that + // `get_dust_buffer_feerate` considers the `pending_update_fee` status), check that we + // won't be pushed over our dust exposure limit by the feerate increase. + if feerate_over_dust_buffer { + let inbound_stats = self.get_inbound_pending_htlc_stats(); + let outbound_stats = self.get_outbound_pending_htlc_stats(); + let holder_tx_dust_exposure = inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat; + let counterparty_tx_dust_exposure = inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat; + if holder_tx_dust_exposure > self.get_max_dust_htlc_exposure_msat() { + return Err(ChannelError::Close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our own transactions (totaling {} msat)", + msg.feerate_per_kw, holder_tx_dust_exposure))); + } + if counterparty_tx_dust_exposure > self.get_max_dust_htlc_exposure_msat() { + return Err(ChannelError::Close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our counterparty's transactions (totaling {} msat)", + msg.feerate_per_kw, counterparty_tx_dust_exposure))); + } + } Ok(()) } @@ -3145,11 +3301,18 @@ impl Channel { } } - log_trace!(logger, "Regenerated latest commitment update in channel {} with {} update_adds, {} update_fulfills, {} update_fails, and {} update_fail_malformeds", - log_bytes!(self.channel_id()), update_add_htlcs.len(), update_fulfill_htlcs.len(), update_fail_htlcs.len(), update_fail_malformed_htlcs.len()); + let update_fee = if self.is_outbound() && self.pending_update_fee.is_some() { + Some(msgs::UpdateFee { + channel_id: self.channel_id(), + feerate_per_kw: self.pending_update_fee.unwrap().0, + }) + } else { None }; + + log_trace!(logger, "Regenerated latest commitment update in channel {} with{} {} update_adds, {} update_fulfills, {} update_fails, and {} update_fail_malformeds", + log_bytes!(self.channel_id()), if update_fee.is_some() { " update_fee," } else { "" }, + update_add_htlcs.len(), update_fulfill_htlcs.len(), update_fail_htlcs.len(), update_fail_malformed_htlcs.len()); msgs::CommitmentUpdate { - update_add_htlcs, update_fulfill_htlcs, update_fail_htlcs, update_fail_malformed_htlcs, - update_fee: None, + update_add_htlcs, update_fulfill_htlcs, update_fail_htlcs, update_fail_malformed_htlcs, update_fee, commitment_signed: self.send_commitment_no_state_update(logger).expect("It looks like we failed to re-generate a commitment_signed we had previously sent?").0, } } @@ -3263,7 +3426,8 @@ impl Channel { // now! match self.free_holding_cell_htlcs(logger) { Err(ChannelError::Close(msg)) => return Err(ChannelError::Close(msg)), - Err(ChannelError::Ignore(_)) | Err(ChannelError::CloseDelayBroadcast(_)) => panic!("Got non-channel-failing result from free_holding_cell_htlcs"), + Err(ChannelError::Warn(_)) | Err(ChannelError::Ignore(_)) | Err(ChannelError::CloseDelayBroadcast(_)) => + panic!("Got non-channel-failing result from free_holding_cell_htlcs"), Ok((Some((commitment_update, monitor_update)), htlcs_to_fail)) => { return Ok((resend_funding_locked, required_revoke, Some(commitment_update), Some(monitor_update), self.resend_order.clone(), htlcs_to_fail, shutdown_msg)); }, @@ -3292,44 +3456,125 @@ impl Channel { } } - fn maybe_propose_first_closing_signed(&mut self, fee_estimator: &F) -> Option + /// Calculates and returns our minimum and maximum closing transaction fee amounts, in whole + /// satoshis. The amounts remain consistent unless a peer disconnects/reconnects or we restart, + /// at which point they will be recalculated. + fn calculate_closing_fee_limits(&mut self, fee_estimator: &F) -> (u64, u64) where F::Target: FeeEstimator { - if !self.is_outbound() || !self.pending_inbound_htlcs.is_empty() || !self.pending_outbound_htlcs.is_empty() || - self.channel_state & (BOTH_SIDES_SHUTDOWN_MASK | ChannelState::AwaitingRemoteRevoke as u32) != BOTH_SIDES_SHUTDOWN_MASK || - self.last_sent_closing_fee.is_some() || self.pending_update_fee.is_some() { - return None; - } + if let Some((min, max)) = self.closing_fee_limits { return (min, max); } + // Propose a range from our current Background feerate to our Normal feerate plus our + // force_close_avoidance_max_fee_satoshis. + // If we fail to come to consensus, we'll have to force-close. let mut proposed_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background); - if self.feerate_per_kw > proposed_feerate { - proposed_feerate = self.feerate_per_kw; - } - assert!(self.shutdown_scriptpubkey.is_some()); + let normal_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal); + let mut proposed_max_feerate = if self.is_outbound() { normal_feerate } else { u32::max_value() }; + + // The spec requires that (when the channel does not have anchors) we only send absolute + // channel fees no greater than the absolute channel fee on the current commitment + // transaction. It's unclear *which* commitment transaction this refers to, and there isn't + // very good reason to apply such a limit in any case. We don't bother doing so, risking + // some force-closure by old nodes, but we wanted to close the channel anyway. + + if let Some(target_feerate) = self.target_closing_feerate_sats_per_kw { + let min_feerate = if self.is_outbound() { target_feerate } else { cmp::min(self.feerate_per_kw, target_feerate) }; + proposed_feerate = cmp::max(proposed_feerate, min_feerate); + proposed_max_feerate = cmp::max(proposed_max_feerate, min_feerate); + } + + // Note that technically we could end up with a lower minimum fee if one sides' balance is + // below our dust limit, causing the output to disappear. We don't bother handling this + // case, however, as this should only happen if a channel is closed before any (material) + // payments have been made on it. This may cause slight fee overpayment and/or failure to + // come to consensus with our counterparty on appropriate fees, however it should be a + // relatively rare case. We can revisit this later, though note that in order to determine + // if the funders' output is dust we have to know the absolute fee we're going to use. 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; + let proposed_max_total_fee_satoshis = if self.is_outbound() { + // We always add force_close_avoidance_max_fee_satoshis to our normal + // feerate-calculated fee, but allow the max to be overridden if we're using a + // target feerate-calculated fee. + 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 { + 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.last_sent_closing_fee.is_some() || !self.closing_negotiation_ready() { + return Ok((None, None)); + } + + if !self.is_outbound() { + if let Some(msg) = &self.pending_counterparty_closing_signed.take() { + return self.closing_signed(fee_estimator, &msg); + } + return Ok((None, None)); + } + + let (our_min_fee, our_max_fee) = self.calculate_closing_fee_limits(fee_estimator); + + assert!(self.shutdown_scriptpubkey.is_some()); + let (closing_tx, total_fee_satoshis) = self.build_closing_transaction(our_min_fee, false); + log_trace!(logger, "Proposing initial closing_signed for our counterparty with a fee range of {}-{} sat (with initial proposal {} sats)", + our_min_fee, our_max_fee, total_fee_satoshis); - let (closing_tx, total_fee_satoshis) = self.build_closing_transaction(proposed_total_fee_satoshis, false); let sig = self.holder_signer .sign_closing_transaction(&closing_tx, &self.secp_ctx) - .ok(); - assert!(closing_tx.get_weight() as u64 <= tx_weight); - if sig.is_none() { return None; } + .map_err(|()| ChannelError::Close("Failed to get signature for closing transaction.".to_owned()))?; - self.last_sent_closing_fee = Some((proposed_feerate, total_fee_satoshis, sig.clone().unwrap())); - Some(msgs::ClosingSigned { + self.last_sent_closing_fee = Some((total_fee_satoshis, sig.clone())); + Ok((Some(msgs::ClosingSigned { channel_id: self.channel_id, fee_satoshis: total_fee_satoshis, - signature: sig.unwrap(), - }) + signature: sig, + fee_range: Some(msgs::ClosingSignedFeeRange { + min_fee_satoshis: our_min_fee, + max_fee_satoshis: our_max_fee, + }), + }), None)) } - 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 + pub fn shutdown( + &mut self, keys_provider: &K, their_features: &InitFeatures, msg: &msgs::Shutdown + ) -> Result<(Option, Option, Vec<(HTLCSource, PaymentHash)>), ChannelError> + where 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())); @@ -3347,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 @@ -3417,13 +3661,11 @@ impl Channel { self.channel_state |= ChannelState::LocalShutdownSent as u32; self.update_time_counter += 1; - Ok((shutdown, self.maybe_propose_first_closing_signed(fee_estimator), monitor_update, dropped_outbound_htlcs)) + 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 @@ -3440,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> @@ -3458,12 +3701,21 @@ impl Channel { return Err(ChannelError::Close("Remote tried to send us a closing tx with > 21 million BTC fee".to_owned())); } + if self.is_outbound() && self.last_sent_closing_fee.is_none() { + return Err(ChannelError::Close("Remote tried to send a closing_signed when we were supposed to propose the first one".to_owned())); + } + + if self.channel_state & ChannelState::MonitorUpdateFailed as u32 != 0 { + self.pending_counterparty_closing_signed = Some(msg.clone()); + return Ok((None, None)); + } + 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(_) => {}, @@ -3471,81 +3723,116 @@ 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()); }, }; - let closing_tx_max_weight = self.get_closing_transaction_weight( - if let Some(oup) = closing_tx.output.get(0) { Some(&oup.script_pubkey) } else { None }, - if let Some(oup) = closing_tx.output.get(1) { Some(&oup.script_pubkey) } else { None }); - if let Some((_, last_fee, sig)) = self.last_sent_closing_fee { + 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); - assert!(closing_tx.get_weight() as u64 <= closing_tx_max_weight); - debug_assert!(closing_tx.get_weight() as u64 >= closing_tx_max_weight - 2); + 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))); } } - 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 (our_min_fee, our_max_fee) = self.calculate_closing_fee_limits(fee_estimator); + + macro_rules! propose_fee { + ($new_fee: expr) => { + 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(&closing_tx, &self.secp_ctx) .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction".to_owned()))?; - assert!(closing_tx.get_weight() as u64 <= tx_weight); - self.last_sent_closing_fee = Some(($new_feerate, used_total_fee, sig.clone())); + + let signed_tx = if $new_fee == msg.fee_satoshis { + self.channel_state = ChannelState::ShutdownComplete as u32; + self.update_time_counter += 1; + let tx = self.build_signed_closing_transaction(&closing_tx, &msg.signature, &sig); + Some(tx) + } else { None }; + + self.last_sent_closing_fee = Some((used_fee, sig.clone())); return Ok((Some(msgs::ClosingSigned { channel_id: self.channel_id, - fee_satoshis: used_total_fee, + fee_satoshis: used_fee, signature: sig, - }), None)) + fee_range: Some(msgs::ClosingSignedFeeRange { + min_fee_satoshis: our_min_fee, + max_fee_satoshis: our_max_fee, + }), + }), signed_tx)) } } - let mut min_feerate = 253; - if self.is_outbound() { - let max_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal); - if (msg.fee_satoshis as u64) > max_feerate as u64 * closing_tx_max_weight / 1000 { - if let Some((last_feerate, _, _)) = self.last_sent_closing_fee { - if max_feerate <= last_feerate { - return Err(ChannelError::Close(format!("Unable to come to consensus about closing feerate, remote wanted something higher ({}) than our Normal feerate ({})", last_feerate, max_feerate))); - } + if let Some(msgs::ClosingSignedFeeRange { min_fee_satoshis, max_fee_satoshis }) = msg.fee_range { + if msg.fee_satoshis < min_fee_satoshis || msg.fee_satoshis > max_fee_satoshis { + return Err(ChannelError::Close(format!("Peer sent a bogus closing_signed - suggested fee of {} sat was not in their desired range of {} sat - {} sat", msg.fee_satoshis, min_fee_satoshis, max_fee_satoshis))); + } + if max_fee_satoshis < our_min_fee { + return Err(ChannelError::Warn(format!("Unable to come to consensus about closing feerate, remote's max fee ({} sat) was smaller than our min fee ({} sat)", max_fee_satoshis, our_min_fee))); + } + if min_fee_satoshis > our_max_fee { + return Err(ChannelError::Warn(format!("Unable to come to consensus about closing feerate, remote's min fee ({} sat) was greater than our max fee ({} sat)", min_fee_satoshis, our_max_fee))); + } + + if !self.is_outbound() { + // They have to pay, so pick the highest fee in the overlapping range. + // 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 { + return Err(ChannelError::Close(format!("Peer sent a bogus closing_signed - suggested fee of {} sat was not in our desired range of {} sat - {} sat after we informed them of our range.", + msg.fee_satoshis, our_min_fee, our_max_fee))); } - propose_new_feerate!(max_feerate); + // The proposed fee is in our acceptable range, accept it and broadcast! + propose_fee!(msg.fee_satoshis); } } else { - min_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background); - } - if (msg.fee_satoshis as u64) < min_feerate as u64 * closing_tx_max_weight / 1000 { - if let Some((last_feerate, _, _)) = self.last_sent_closing_fee { - if min_feerate >= last_feerate { - return Err(ChannelError::Close(format!("Unable to come to consensus about closing feerate, remote wanted something lower ({}) than our Background feerate ({}).", last_feerate, min_feerate))); + // Old fee style negotiation. We don't bother to enforce whether they are complying + // with the "making progress" requirements, we just comply and hope for the best. + if let Some((last_fee, _)) = self.last_sent_closing_fee { + if msg.fee_satoshis > last_fee { + if msg.fee_satoshis < our_max_fee { + propose_fee!(msg.fee_satoshis); + } else if last_fee < our_max_fee { + propose_fee!(our_max_fee); + } else { + return Err(ChannelError::Close(format!("Unable to come to consensus about closing feerate, remote wants something ({} sat) higher than our max fee ({} sat)", msg.fee_satoshis, our_max_fee))); + } + } else { + if msg.fee_satoshis > our_min_fee { + propose_fee!(msg.fee_satoshis); + } else if last_fee > our_min_fee { + propose_fee!(our_min_fee); + } else { + return Err(ChannelError::Close(format!("Unable to come to consensus about closing feerate, remote wants something ({} sat) lower than our min fee ({} sat)", msg.fee_satoshis, our_min_fee))); + } + } + } else { + if msg.fee_satoshis < our_min_fee { + propose_fee!(our_min_fee); + } else if msg.fee_satoshis > our_max_fee { + propose_fee!(our_max_fee); + } else { + propose_fee!(msg.fee_satoshis); } } - propose_new_feerate!(min_feerate); } - - let sig = self.holder_signer - .sign_closing_transaction(&closing_tx, &self.secp_ctx) - .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction".to_owned()))?; - self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &sig); - assert!(closing_tx.get_weight() as u64 <= closing_tx_max_weight); - debug_assert!(closing_tx.get_weight() as u64 >= closing_tx_max_weight - 2); - - self.channel_state = ChannelState::ShutdownComplete as u32; - self.update_time_counter += 1; - - Ok((Some(msgs::ClosingSigned { - channel_id: self.channel_id, - fee_satoshis: msg.fee_satoshis, - signature: sig, - }), Some(closing_tx))) } // Public utilities: @@ -3648,7 +3935,13 @@ impl Channel { // whichever is higher. This ensures that we aren't suddenly exposed to significantly // more dust balance if the feerate increases when we have several HTLCs pending // which are near the dust limit. - cmp::max(2530, self.feerate_per_kw * 1250 / 1000) + 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); + } + cmp::max(2530, feerate_per_kw * 1250 / 1000) } pub fn get_cur_holder_commitment_transaction_number(&self) -> u64 { @@ -3746,6 +4039,16 @@ impl Channel { self.channel_state >= ChannelState::FundingSent as u32 } + /// Returns true if our peer has either initiated or agreed to shut down the channel. + pub fn received_shutdown(&self) -> bool { + (self.channel_state & ChannelState::RemoteShutdownSent as u32) != 0 + } + + /// Returns true if we either initiated or agreed to shut down the channel. + pub fn sent_shutdown(&self) -> bool { + (self.channel_state & ChannelState::LocalShutdownSent as u32) != 0 + } + /// Returns true if this channel is fully shut down. True here implies that no further actions /// may/will be taken on this channel, and thus this object should be freed. Any future changes /// will be handled appropriately by the chain monitor. @@ -4007,6 +4310,7 @@ impl Channel { Some(script) => script.clone().into_inner(), None => Builder::new().into_script(), }), + channel_type: Some(self.channel_type.clone()), } } @@ -4049,7 +4353,7 @@ impl Channel { /// If an Err is returned, it is a ChannelError::Close (for get_outbound_funding_created) fn get_outbound_funding_created_signature(&mut self, logger: &L) -> Result where L::Target: Logger { let counterparty_keys = self.build_remote_transaction_keys()?; - let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, self.feerate_per_kw, logger).0; + let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).0; Ok(self.holder_signer.sign_counterparty_commitment(&counterparty_initial_commitment_tx, &self.secp_ctx) .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0) } @@ -4406,7 +4710,7 @@ impl Channel { if (self.channel_state & (ChannelState::MonitorUpdateFailed as u32)) == (ChannelState::MonitorUpdateFailed as u32) { panic!("Cannot create commitment tx while awaiting monitor update unfreeze, as send_htlc will have returned an Err so a send_commitment precondition has been violated"); } - let mut have_updates = self.pending_update_fee.is_some(); + let mut have_updates = self.is_outbound() && self.pending_update_fee.is_some(); for htlc in self.pending_outbound_htlcs.iter() { if let OutboundHTLCState::LocalAnnounced(_) = htlc.state { have_updates = true; @@ -4426,6 +4730,7 @@ impl Channel { } /// Only fails in case of bad keys fn send_commitment_no_status_check(&mut self, logger: &L) -> Result<(msgs::CommitmentSigned, ChannelMonitorUpdate), ChannelError> where L::Target: Logger { + log_trace!(logger, "Updating HTLC state for a newly-sent commitment_signed..."); // We can upgrade the status of some HTLCs that are waiting on a commitment, even if we // fail to generate this, we still are at least at a position where upgrading their status // is acceptable. @@ -4434,6 +4739,7 @@ impl Channel { Some(InboundHTLCState::AwaitingAnnouncedRemoteRevoke(forward_info.clone())) } else { None }; if let Some(state) = new_state { + log_trace!(logger, " ...promoting inbound AwaitingRemoteRevokeToAnnounce {} to AwaitingAnnouncedRemoteRevoke", log_bytes!(htlc.payment_hash.0)); htlc.state = state; } } @@ -4441,9 +4747,18 @@ impl Channel { if let Some(fail_reason) = if let &mut OutboundHTLCState::AwaitingRemoteRevokeToRemove(ref mut fail_reason) = &mut htlc.state { Some(fail_reason.take()) } else { None } { + log_trace!(logger, " ...promoting outbound AwaitingRemoteRevokeToRemove {} to AwaitingRemovedRemoteRevoke", log_bytes!(htlc.payment_hash.0)); htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke(fail_reason); } } + if let Some((feerate, update_state)) = self.pending_update_fee { + if update_state == FeeUpdateState::AwaitingRemoteRevokeToAnnounce { + debug_assert!(!self.is_outbound()); + log_trace!(logger, " ...promoting inbound AwaitingRemoteRevokeToAnnounce fee update {} to Committed", feerate); + self.feerate_per_kw = feerate; + self.pending_update_fee = None; + } + } self.resend_order = RAACommitmentOrder::RevokeAndACKFirst; let (res, counterparty_commitment_txid, htlcs) = match self.send_commitment_no_state_update(logger) { @@ -4473,15 +4788,9 @@ impl Channel { /// Only fails in case of bad keys. Used for channel_reestablish commitment_signed generation /// when we shouldn't change HTLC/channel state. fn send_commitment_no_state_update(&self, logger: &L) -> Result<(msgs::CommitmentSigned, (Txid, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>)), ChannelError> where L::Target: Logger { - let mut feerate_per_kw = self.feerate_per_kw; - if let Some(feerate) = self.pending_update_fee { - if self.is_outbound() { - feerate_per_kw = feerate; - } - } - let counterparty_keys = self.build_remote_transaction_keys()?; - let counterparty_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, true, feerate_per_kw, logger); + let counterparty_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, true, logger); + let feerate_per_kw = counterparty_commitment_tx.1; let counterparty_commitment_txid = counterparty_commitment_tx.0.trust().txid(); let (signature, htlc_signatures); @@ -4496,7 +4805,7 @@ impl Channel { && info.next_holder_htlc_id == self.next_holder_htlc_id && info.next_counterparty_htlc_id == self.next_counterparty_htlc_id && info.feerate == self.feerate_per_kw { - let actual_fee = self.commit_tx_fee_msat(counterparty_commitment_tx.1); + let actual_fee = self.commit_tx_fee_msat(counterparty_commitment_tx.2); assert_eq!(actual_fee, info.fee); } } @@ -4504,8 +4813,8 @@ impl Channel { } { - let mut htlcs = Vec::with_capacity(counterparty_commitment_tx.2.len()); - for &(ref htlc, _) in counterparty_commitment_tx.2.iter() { + let mut htlcs = Vec::with_capacity(counterparty_commitment_tx.3.len()); + for &(ref htlc, _) in counterparty_commitment_tx.3.iter() { htlcs.push(htlc); } @@ -4532,7 +4841,7 @@ impl Channel { channel_id: self.channel_id, signature, htlc_signatures, - }, (counterparty_commitment_txid, counterparty_commitment_tx.2))) + }, (counterparty_commitment_txid, counterparty_commitment_tx.3))) } /// Adds a pending outbound HTLC to this channel, and creates a signed commitment transaction @@ -4569,7 +4878,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, keys_provider: &K, their_features: &InitFeatures) -> Result<(msgs::Shutdown, Option, Vec<(HTLCSource, PaymentHash)>), APIError> + pub fn get_shutdown(&mut self, keys_provider: &K, their_features: &InitFeatures, target_feerate_sats_per_kw: Option) + -> 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 { @@ -4602,6 +4912,7 @@ impl Channel { }; // From here on out, we may not fail! + self.target_closing_feerate_sats_per_kw = target_feerate_sats_per_kw; if self.channel_state < ChannelState::FundingSent as u32 { self.channel_state = ChannelState::ShutdownComplete as u32; } else { @@ -4873,7 +5184,15 @@ impl Writeable for Channel { fail_reason.write(writer)?; } - self.pending_update_fee.write(writer)?; + if self.is_outbound() { + self.pending_update_fee.map(|(a, _)| a).write(writer)?; + } else if let Some((feerate, FeeUpdateState::AwaitingRemoteRevokeToAnnounce)) = self.pending_update_fee { + 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)?; self.next_holder_htlc_id.write(writer)?; @@ -4881,15 +5200,11 @@ impl Writeable for Channel { self.update_time_counter.write(writer)?; self.feerate_per_kw.write(writer)?; - match self.last_sent_closing_fee { - Some((feerate, fee, sig)) => { - 1u8.write(writer)?; - feerate.write(writer)?; - fee.write(writer)?; - sig.write(writer)?; - }, - None => 0u8.write(writer)?, - } + // Versions prior to 0.0.100 expected to read the fields of `last_sent_closing_fee` here, + // however we are supposed to restart shutdown fee negotiation on reconnect (and wipe + // `last_send_closing_fee` in `remove_uncommitted_htlcs_and_mark_paused`) so we should never + // consider the stale state on reload. + 0u8.write(writer)?; self.funding_tx_confirmed_in.write(writer)?; self.funding_tx_confirmation_height.write(writer)?; @@ -4951,6 +5266,9 @@ impl Writeable for Channel { (3, self.counterparty_selected_channel_reserve_satoshis, option), (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(()) @@ -5088,7 +5406,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel monitor_pending_failures.push((Readable::read(reader)?, Readable::read(reader)?, Readable::read(reader)?)); } - let pending_update_fee = Readable::read(reader)?; + let pending_update_fee_value: Option = Readable::read(reader)?; + let holding_cell_update_fee = Readable::read(reader)?; let next_holder_htlc_id = Readable::read(reader)?; @@ -5096,11 +5415,19 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel let update_time_counter = Readable::read(reader)?; let feerate_per_kw = Readable::read(reader)?; - let last_sent_closing_fee = match ::read(reader)? { - 0 => None, - 1 => Some((Readable::read(reader)?, Readable::read(reader)?, Readable::read(reader)?)), + // Versions prior to 0.0.100 expected to read the fields of `last_sent_closing_fee` here, + // however we are supposed to restart shutdown fee negotiation on reconnect (and wipe + // `last_send_closing_fee` in `remove_uncommitted_htlcs_and_mark_paused`) so we should never + // consider the stale state on reload. + match ::read(reader)? { + 0 => {}, + 1 => { + let _: u32 = Readable::read(reader)?; + let _: u64 = Readable::read(reader)?; + let _: Signature = Readable::read(reader)?; + }, _ => return Err(DecodeError::InvalidValue), - }; + } let funding_tx_confirmed_in = Readable::read(reader)?; let funding_tx_confirmation_height = Readable::read(reader)?; @@ -5140,7 +5467,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel _ => return Err(DecodeError::InvalidValue), }; - let channel_parameters = Readable::read(reader)?; + let channel_parameters: ChannelTransactionParameters = Readable::read(reader)?; let funding_transaction = Readable::read(reader)?; let counterparty_cur_commitment_point = Readable::read(reader)?; @@ -5163,15 +5490,40 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel } } + let pending_update_fee = if let Some(feerate) = pending_update_fee_value { + Some((feerate, if channel_parameters.is_outbound_from_holder { + FeeUpdateState::Outbound + } else { + FeeUpdateState::AwaitingRemoteRevokeToAnnounce + })) + } else { + None + }; + 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), (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), + (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()); @@ -5205,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, @@ -5218,7 +5571,10 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel #[cfg(debug_assertions)] counterparty_max_commitment_tx_output: Mutex::new((0, 0)), - last_sent_closing_fee, + last_sent_closing_fee: None, + pending_counterparty_closing_signed: None, + closing_fee_limits: None, + target_closing_feerate_sats_per_kw, funding_tx_confirmed_in, funding_tx_confirmation_height, @@ -5247,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, @@ -5259,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(), }) } } @@ -5275,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; @@ -5449,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, } }); @@ -5701,9 +6063,9 @@ mod tests { $( { $htlc_idx: expr, $counterparty_htlc_sig_hex: expr, $htlc_sig_hex: expr, $htlc_tx_hex: expr } ), * } ) => { { let (commitment_tx, htlcs): (_, Vec) = { - let mut res = chan.build_commitment_transaction(0xffffffffffff - 42, &keys, true, false, chan.feerate_per_kw, &logger); + let mut res = chan.build_commitment_transaction(0xffffffffffff - 42, &keys, true, false, &logger); - let htlcs = res.2.drain(..) + let htlcs = res.3.drain(..) .filter_map(|(htlc, _)| if htlc.transaction_output_index.is_some() { Some(htlc) } else { None }) .collect(); (res.0, htlcs)