X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannel.rs;h=4a6e4eb656b6f6fa2f9a5076814853d8ac77f9ea;hb=843d25d750c3408d3f8f917764b8a58019a9dd81;hp=92a9ab6ab9ad4cd19836ddd8e66ecfc66c5104de;hpb=c2b0db0ac12264a7c145aa4e6358abce6335afde;p=rust-lightning diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 92a9ab6a..4a6e4eb6 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -8,16 +8,16 @@ // licenses. use bitcoin::blockdata::script::{Script,Builder}; -use bitcoin::blockdata::transaction::{TxIn, TxOut, Transaction, SigHashType}; -use bitcoin::blockdata::opcodes; +use bitcoin::blockdata::transaction::{Transaction, SigHashType}; use bitcoin::util::bip143; use bitcoin::consensus::encode; use bitcoin::hashes::Hash; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::sha256d::Hash as Sha256d; -use bitcoin::hash_types::{Txid, BlockHash, WPubkeyHash}; +use bitcoin::hash_types::{Txid, BlockHash}; +use bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE; use bitcoin::secp256k1::key::{PublicKey,SecretKey}; use bitcoin::secp256k1::{Secp256k1,Signature}; use bitcoin::secp256k1; @@ -26,27 +26,28 @@ use ln::{PaymentPreimage, PaymentHash}; use ln::features::{ChannelFeatures, InitFeatures}; use ln::msgs; use ln::msgs::{DecodeError, OptionalField, DataLossProtect}; -use ln::channelmanager::{BestBlock, PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT}; -use ln::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; use util::config::{UserConfig,ChannelConfig}; use util::scid_utils::scid_from_parts; +use io; use prelude::*; use core::{cmp,mem,fmt}; use core::ops::Deref; -#[cfg(any(test, feature = "fuzztarget"))] -use std::sync::Mutex; +#[cfg(any(test, feature = "fuzztarget", debug_assertions))] +use sync::Mutex; use bitcoin::hashes::hex::ToHex; -use bitcoin::blockdata::opcodes::all::OP_PUSHBYTES_0; #[cfg(test)] pub struct ChannelValueStat { @@ -60,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)), @@ -237,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. @@ -272,6 +286,14 @@ enum HTLCInitiator { RemoteOffered, } +/// An enum gathering stats on pending HTLCs, either inbound or outbound side. +struct HTLCStats { + pending_htlcs: u32, + pending_htlcs_value_msat: u64, + on_counterparty_tx_dust_exposure_msat: u64, + on_holder_tx_dust_exposure_msat: u64, +} + /// Used when calculating whether we or the remote can afford an additional HTLC. struct HTLCCandidate { amount_msat: u64, @@ -287,19 +309,52 @@ 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 { + NewClaim { + monitor_update: ChannelMonitorUpdate, + htlc_value_msat: u64, + msg: Option, + }, + DuplicateClaim {}, } +/// The return type of get_update_fulfill_htlc_and_commit. +pub enum UpdateFulfillCommitFetch { + /// Indicates the HTLC fulfill is new, and either generated an update_fulfill message, placed + /// it in the holding cell, or re-generated the update_fulfill message after the same claim was + /// previously placed in the holding cell (and has since been removed). + NewClaim { + /// The ChannelMonitorUpdate which places the new payment preimage in the channel monitor + monitor_update: ChannelMonitorUpdate, + /// The value of the HTLC which was claimed, in msat. + htlc_value_msat: u64, + /// The update_fulfill message and commitment_signed message (if the claim was not placed + /// in the holding cell). + msgs: Option<(msgs::UpdateFulfillHTLC, msgs::CommitmentSigned)>, + }, + /// Indicates the HTLC fulfill is duplicative and already existed either in the holding cell + /// or has been forgotten (presumably previously claimed). + DuplicateClaim {}, +} + +/// 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 +/// balance but the fundee is unable to send a payment as the increase in fee more than drains +/// their reserve value. Thus, neither side can send a new HTLC and the channel becomes useless. +/// Thus, before sending an HTLC when we are the initiator, we check that the feerate can increase +/// by this multiple without hitting this case, before sending. +/// This multiple is effectively the maximum feerate "jump" we expect until more HTLCs flow over +/// the channel. Sadly, there isn't really a good number for this - if we expect to have no new +/// HTLCs for days we may need this to suffice for feerate increases across days, but that may +/// leave the channel less usable as we hold a bigger reserve. +#[cfg(fuzzing)] +pub const FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE: u64 = 2; +#[cfg(not(fuzzing))] +const FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE: u64 = 2; + // TODO: We should refactor this to be an Inbound/OutboundChannel until initial setup handshaking // has been completed, and then turn into a Channel to get compiler-time enforcement of things like // calling channel_id() before we're set up or things like get_outbound_funding_signed on an @@ -308,6 +363,9 @@ pub struct CounterpartyForwardingInfo { // Holder designates channel data owned for the benefice of the user client. // Counterparty designates channel data owned by the another channel participant entity. pub(super) struct Channel { + #[cfg(any(test, feature = "_test_utils"))] + pub(crate) config: ChannelConfig, + #[cfg(not(any(test, feature = "_test_utils")))] config: ChannelConfig, user_id: u64, @@ -320,7 +378,7 @@ pub(super) struct Channel { latest_monitor_update_id: u64, holder_signer: Signer, - shutdown_pubkey: PublicKey, + shutdown_scriptpubkey: Option, destination_script: Script, // Our commitment numbers start at 2^48-1 and count down, whereas the ones used in transaction @@ -349,22 +407,18 @@ pub(super) struct Channel { monitor_pending_forwards: Vec<(PendingHTLCInfo, u64)>, monitor_pending_failures: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, - // 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, @@ -373,12 +427,25 @@ pub(super) struct Channel { #[cfg(debug_assertions)] /// Max to_local and to_remote outputs in a locally-generated commitment transaction - holder_max_commitment_tx_output: ::std::sync::Mutex<(u64, u64)>, + holder_max_commitment_tx_output: Mutex<(u64, u64)>, #[cfg(debug_assertions)] /// Max to_local and to_remote outputs in a remote-generated commitment transaction - counterparty_max_commitment_tx_output: ::std::sync::Mutex<(u64, u64)>, + counterparty_max_commitment_tx_output: Mutex<(u64, u64)>, - last_sent_closing_fee: Option<(u32, u64, Signature)>, // (feerate, fee, holder_sig) + 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, @@ -421,6 +488,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. @@ -443,6 +517,15 @@ pub(super) struct Channel { /// /// See-also pub workaround_lnd_bug_4006: Option, + + #[cfg(any(test, feature = "fuzztarget"))] + // When we receive an HTLC fulfill on an outbound path, we may immediately fulfill the + // corresponding HTLC on the inbound path. If, then, the outbound path channel is + // disconnected and reconnected (before we've exchange commitment_signed and revoke_and_ack + // messages), they may re-broadcast their update_fulfill_htlc, causing a duplicate claim. This + // is fine, but as a sanity check in our failure to generate the second claim, we check here + // that the original was a claim, and that we aren't now trying to fulfill a failed HTLC. + historical_inbound_htlc_fulfills: HashSet, } #[cfg(any(test, feature = "fuzztarget"))] @@ -455,7 +538,6 @@ struct CommitmentTxInfoCached { } pub const OUR_MAX_HTLCS: u16 = 50; //TODO -const SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT: u64 = 79; // prevout: 36, nSequence: 4, script len: 1, witness lengths: (3+1)/4, sig: 73/4, if-selector: 1, redeemScript: (6 ops + 2*33 pubkeys + 1*2 delay)/4 #[cfg(not(test))] const COMMITMENT_TX_BASE_WEIGHT: u64 = 724; @@ -466,31 +548,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), } @@ -499,6 +587,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) } @@ -530,7 +619,7 @@ impl Channel { } // Constructors: - pub fn new_outbound(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig) -> Result, APIError> + pub fn new_outbound(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures, channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig) -> Result, APIError> where K::Target: KeysInterface, F::Target: FeeEstimator, { @@ -549,7 +638,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) }); } @@ -558,6 +647,16 @@ impl Channel { let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes()); + let shutdown_scriptpubkey = if config.channel_options.commit_upfront_shutdown_pubkey { + Some(keys_provider.get_shutdown_scriptpubkey()) + } else { None }; + + if let Some(shutdown_scriptpubkey) = &shutdown_scriptpubkey { + if !shutdown_scriptpubkey.is_compatible(&their_features) { + return Err(APIError::IncompatibleShutdownScript { script: shutdown_scriptpubkey.clone() }); + } + } + Ok(Channel { user_id, config: config.channel_options.clone(), @@ -570,7 +669,7 @@ impl Channel { latest_monitor_update_id: 0, holder_signer, - shutdown_pubkey: keys_provider.get_shutdown_pubkey(), + shutdown_scriptpubkey, destination_script: keys_provider.get_destination_script(), cur_holder_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER, @@ -595,11 +694,14 @@ impl Channel { monitor_pending_failures: Vec::new(), #[cfg(debug_assertions)] - holder_max_commitment_tx_output: ::std::sync::Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)), + holder_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)), #[cfg(debug_assertions)] - counterparty_max_commitment_tx_output: ::std::sync::Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)), + counterparty_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)), last_sent_closing_fee: None, + 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, @@ -607,7 +709,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, @@ -635,6 +737,7 @@ impl Channel { commitment_secrets: CounterpartyCommitmentSecrets::new(), channel_update_status: ChannelUpdateStatus::Enabled, + closing_signed_in_flight: false, announcement_sigs: None, @@ -644,6 +747,9 @@ impl Channel { next_remote_commitment_tx_fee_info_cached: Mutex::new(None), workaround_lnd_bug_4006: None, + + #[cfg(any(test, feature = "fuzztarget"))] + historical_inbound_htlc_fulfills: HashSet::new(), }) } @@ -654,7 +760,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))); } @@ -663,7 +774,7 @@ impl Channel { /// Creates a new channel from a remote sides' request for one. /// Assumes chain_hash has already been checked and corresponds with what we expect! - pub fn new_from_req(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: InitFeatures, msg: &msgs::OpenChannel, user_id: u64, config: &UserConfig) -> Result, ChannelError> + pub fn new_from_req(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures, msg: &msgs::OpenChannel, user_id: u64, config: &UserConfig) -> Result, ChannelError> where K::Target: KeysInterface, F::Target: FeeEstimator { @@ -732,11 +843,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: @@ -753,11 +864,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))); @@ -783,10 +894,10 @@ impl Channel { // Peer is signaling upfront_shutdown and has opt-out with a 0-length script. We don't enforce anything if script.len() == 0 { None - // Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. Fail the channel - } else if is_unsupported_shutdown_script(&their_features, script) { - return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: ({})", script.to_bytes().to_hex()))); } else { + 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()) } }, @@ -797,6 +908,16 @@ impl Channel { } } else { None }; + let shutdown_scriptpubkey = if config.channel_options.commit_upfront_shutdown_pubkey { + Some(keys_provider.get_shutdown_scriptpubkey()) + } else { None }; + + if let Some(shutdown_scriptpubkey) = &shutdown_scriptpubkey { + if !shutdown_scriptpubkey.is_compatible(&their_features) { + return Err(ChannelError::Close(format!("Provided a scriptpubkey format not accepted by peer: {}", shutdown_scriptpubkey))); + } + } + let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes()); @@ -811,7 +932,7 @@ impl Channel { latest_monitor_update_id: 0, holder_signer, - shutdown_pubkey: keys_provider.get_shutdown_pubkey(), + shutdown_scriptpubkey, destination_script: keys_provider.get_destination_script(), cur_holder_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER, @@ -836,11 +957,14 @@ impl Channel { monitor_pending_failures: Vec::new(), #[cfg(debug_assertions)] - holder_max_commitment_tx_output: ::std::sync::Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)), + holder_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)), #[cfg(debug_assertions)] - counterparty_max_commitment_tx_output: ::std::sync::Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)), + counterparty_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)), last_sent_closing_fee: None, + 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, @@ -849,7 +973,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, @@ -880,6 +1004,7 @@ impl Channel { commitment_secrets: CounterpartyCommitmentSecrets::new(), channel_update_status: ChannelUpdateStatus::Enabled, + closing_signed_in_flight: false, announcement_sigs: None, @@ -889,6 +1014,9 @@ impl Channel { next_remote_commitment_tx_fee_info_cached: Mutex::new(None), workaround_lnd_bug_4006: None, + + #[cfg(any(test, feature = "fuzztarget"))] + historical_inbound_htlc_fulfills: HashSet::new(), }; Ok(chan) @@ -909,10 +1037,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); @@ -922,6 +1050,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()), @@ -1051,6 +1192,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); @@ -1072,6 +1218,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, @@ -1082,13 +1231,15 @@ 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] fn get_closing_scriptpubkey(&self) -> Script { - let channel_close_key_hash = WPubkeyHash::hash(&self.shutdown_pubkey.serialize()); - Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&channel_close_key_hash[..]).into_script() + // The shutdown scriptpubkey is set on channel opening when option_upfront_shutdown_script + // is signaled. Otherwise, it is set when sending a shutdown message. Calling this method + // outside of those situations will fail. + self.shutdown_scriptpubkey.clone().unwrap().into_inner() } #[inline] @@ -1119,61 +1270,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; } - 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 { @@ -1216,13 +1344,7 @@ impl Channel { make_funding_redeemscript(&self.get_holder_pubkeys().funding_pubkey, self.counterparty_funding_pubkey()) } - /// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made. - /// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always - /// return Ok(_) if debug assertions are turned on or preconditions are met. - /// - /// Note that it is still possible to hit these assertions in case we find a preimage on-chain - /// but then have a reorg which settles on an HTLC-failure on chain. - fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage, logger: &L) -> Result<(Option, Option), ChannelError> where L::Target: Logger { + fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage, logger: &L) -> UpdateFulfillFetch where L::Target: Logger { // Either ChannelFunded got set (which means it won't be unset) or there is no way any // caller thought we could have something claimed (cause we wouldn't have accepted in an // incoming HTLC anyway). If we got to ShutdownComplete, callers aren't allowed to call us, @@ -1239,6 +1361,7 @@ impl Channel { // these, but for now we just have to treat them as normal. let mut pending_idx = core::usize::MAX; + let mut htlc_value_msat = 0; for (idx, htlc) in self.pending_inbound_htlcs.iter().enumerate() { if htlc.htlc_id == htlc_id_arg { assert_eq!(htlc.payment_hash, payment_hash_calc); @@ -1248,9 +1371,9 @@ impl Channel { if let &InboundHTLCRemovalReason::Fulfill(_) = reason { } else { log_warn!(logger, "Have preimage and want to fulfill HTLC with payment hash {} we already failed against channel {}", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id())); + debug_assert!(false, "Tried to fulfill an HTLC that was already failed"); } - debug_assert!(false, "Tried to fulfill an HTLC that was already fail/fulfilled"); - return Ok((None, None)); + return UpdateFulfillFetch::DuplicateClaim {}; }, _ => { debug_assert!(false, "Have an inbound HTLC we tried to claim before it was fully committed to"); @@ -1258,11 +1381,16 @@ impl Channel { } } pending_idx = idx; + htlc_value_msat = htlc.amount_msat; break; } } if pending_idx == core::usize::MAX { - return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned())); + #[cfg(any(test, feature = "fuzztarget"))] + // If we failed to find an HTLC to fulfill, make sure it was previously fulfilled and + // this is simply a duplicate claim, not previously failed and we lost funds. + debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg)); + return UpdateFulfillFetch::DuplicateClaim {}; } // Now update local state: @@ -1284,8 +1412,9 @@ impl Channel { if htlc_id_arg == htlc_id { // Make sure we don't leave latest_monitor_update_id incremented here: self.latest_monitor_update_id -= 1; - debug_assert!(false, "Tried to fulfill an HTLC that was already fulfilled"); - return Ok((None, None)); + #[cfg(any(test, feature = "fuzztarget"))] + debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg)); + return UpdateFulfillFetch::DuplicateClaim {}; } }, &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => { @@ -1294,7 +1423,7 @@ impl Channel { // TODO: We may actually be able to switch to a fulfill here, though its // rare enough it may not be worth the complexity burden. debug_assert!(false, "Tried to fulfill an HTLC that was already failed"); - return Ok((None, Some(monitor_update))); + return UpdateFulfillFetch::NewClaim { monitor_update, htlc_value_msat, msg: None }; } }, _ => {} @@ -1304,52 +1433,60 @@ impl Channel { self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::ClaimHTLC { payment_preimage: payment_preimage_arg, htlc_id: htlc_id_arg, }); - return Ok((None, Some(monitor_update))); + #[cfg(any(test, feature = "fuzztarget"))] + self.historical_inbound_htlc_fulfills.insert(htlc_id_arg); + return UpdateFulfillFetch::NewClaim { monitor_update, htlc_value_msat, msg: None }; } + #[cfg(any(test, feature = "fuzztarget"))] + self.historical_inbound_htlc_fulfills.insert(htlc_id_arg); { let htlc = &mut self.pending_inbound_htlcs[pending_idx]; if let InboundHTLCState::Committed = htlc.state { } else { debug_assert!(false, "Have an inbound HTLC we tried to claim before it was fully committed to"); - return Ok((None, Some(monitor_update))); + return UpdateFulfillFetch::NewClaim { monitor_update, htlc_value_msat, msg: None }; } log_trace!(logger, "Upgrading HTLC {} to LocalRemoved with a Fulfill in channel {}!", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id)); htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(payment_preimage_arg.clone())); } - Ok((Some(msgs::UpdateFulfillHTLC { - channel_id: self.channel_id(), - htlc_id: htlc_id_arg, - payment_preimage: payment_preimage_arg, - }), Some(monitor_update))) + UpdateFulfillFetch::NewClaim { + monitor_update, + htlc_value_msat, + msg: Some(msgs::UpdateFulfillHTLC { + channel_id: self.channel_id(), + htlc_id: htlc_id_arg, + payment_preimage: payment_preimage_arg, + }), + } } - pub fn get_update_fulfill_htlc_and_commit(&mut self, htlc_id: u64, payment_preimage: PaymentPreimage, logger: &L) -> Result<(Option<(msgs::UpdateFulfillHTLC, msgs::CommitmentSigned)>, Option), ChannelError> where L::Target: Logger { - match self.get_update_fulfill_htlc(htlc_id, payment_preimage, logger)? { - (Some(update_fulfill_htlc), Some(mut monitor_update)) => { - let (commitment, mut additional_update) = self.send_commitment_no_status_check(logger)?; + pub fn get_update_fulfill_htlc_and_commit(&mut self, htlc_id: u64, payment_preimage: PaymentPreimage, logger: &L) -> Result where L::Target: Logger { + match self.get_update_fulfill_htlc(htlc_id, payment_preimage, logger) { + UpdateFulfillFetch::NewClaim { mut monitor_update, htlc_value_msat, msg: Some(update_fulfill_htlc) } => { + let (commitment, mut additional_update) = match self.send_commitment_no_status_check(logger) { + Err(e) => return Err((e, monitor_update)), + Ok(res) => res + }; // send_commitment_no_status_check may bump latest_monitor_id but we want them to be // strictly increasing by one, so decrement it here. self.latest_monitor_update_id = monitor_update.update_id; monitor_update.updates.append(&mut additional_update.updates); - Ok((Some((update_fulfill_htlc, commitment)), Some(monitor_update))) - }, - (Some(update_fulfill_htlc), None) => { - let (commitment, monitor_update) = self.send_commitment_no_status_check(logger)?; - Ok((Some((update_fulfill_htlc, commitment)), Some(monitor_update))) + Ok(UpdateFulfillCommitFetch::NewClaim { monitor_update, htlc_value_msat, msgs: Some((update_fulfill_htlc, commitment)) }) }, - (None, Some(monitor_update)) => Ok((None, Some(monitor_update))), - (None, None) => Ok((None, None)) + UpdateFulfillFetch::NewClaim { monitor_update, htlc_value_msat, msg: None } => + Ok(UpdateFulfillCommitFetch::NewClaim { monitor_update, htlc_value_msat, msgs: None }), + UpdateFulfillFetch::DuplicateClaim {} => Ok(UpdateFulfillCommitFetch::DuplicateClaim {}), } } - /// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made. - /// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always - /// return Ok(_) if debug assertions are turned on or preconditions are met. - /// - /// Note that it is still possible to hit these assertions in case we find a preimage on-chain - /// but then have a reorg which settles on an HTLC-failure on chain. + /// We can only have one resolution per HTLC. In some cases around reconnect, we may fulfill + /// an HTLC more than once or fulfill once and then attempt to fail after reconnect. We cannot, + /// however, fail more than once as we wait for an upstream failure to be irrevocably committed + /// before we fail backwards. + /// If we do fail twice, we debug_assert!(false) and return Ok(None). Thus, will always return + /// Ok(_) if debug assertions are turned on or preconditions are met. pub fn get_update_fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket, logger: &L) -> Result, ChannelError> where L::Target: Logger { if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) { panic!("Was asked to fail an HTLC when channel was not in an operational state"); @@ -1365,8 +1502,11 @@ impl Channel { if htlc.htlc_id == htlc_id_arg { match htlc.state { InboundHTLCState::Committed => {}, - InboundHTLCState::LocalRemoved(_) => { - debug_assert!(false, "Tried to fail an HTLC that was already fail/fulfilled"); + InboundHTLCState::LocalRemoved(ref reason) => { + if let &InboundHTLCRemovalReason::Fulfill(_) = reason { + } else { + debug_assert!(false, "Tried to fail an HTLC that was already failed"); + } return Ok(None); }, _ => { @@ -1378,7 +1518,11 @@ impl Channel { } } if pending_idx == core::usize::MAX { - return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned())); + #[cfg(any(test, feature = "fuzztarget"))] + // If we failed to find an HTLC to fail, make sure it was previously fulfilled and this + // is simply a duplicate fail, not previously failed and we failed-back too early. + debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg)); + return Ok(None); } // Now update local state: @@ -1387,8 +1531,9 @@ impl Channel { match pending_update { &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => { if htlc_id_arg == htlc_id { - debug_assert!(false, "Tried to fail an HTLC that was already fulfilled"); - return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned())); + #[cfg(any(test, feature = "fuzztarget"))] + debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg)); + return Ok(None); } }, &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => { @@ -1423,7 +1568,7 @@ impl Channel { // Message handlers: - pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig, their_features: InitFeatures) -> Result<(), ChannelError> { + pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig, their_features: &InitFeatures) -> Result<(), ChannelError> { // Check sanity of message fields: if !self.is_outbound() { return Err(ChannelError::Close("Got an accept_channel message from an inbound peer".to_owned())); @@ -1472,11 +1617,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))); @@ -1494,10 +1639,10 @@ impl Channel { // Peer is signaling upfront_shutdown and has opt-out with a 0-length script. We don't enforce anything if script.len() == 0 { None - // Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. Fail the channel - } else if is_unsupported_shutdown_script(&their_features, script) { - return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: ({})", script.to_bytes().to_hex()))); } else { + 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()) } }, @@ -1540,7 +1685,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(); @@ -1554,7 +1699,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(); @@ -1615,13 +1760,17 @@ 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(); let funding_txo_script = funding_redeemscript.to_v0_p2wsh(); let obscure_factor = get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound()); + let shutdown_script = self.shutdown_scriptpubkey.clone().map(|script| script.into_inner()); let channel_monitor = ChannelMonitor::new(self.secp_ctx.clone(), self.holder_signer.clone(), - &self.shutdown_pubkey, self.get_holder_selected_contest_delay(), + shutdown_script, self.get_holder_selected_contest_delay(), &self.destination_script, (funding_txo, funding_txo_script.clone()), &self.channel_transaction_parameters, funding_redeemscript.clone(), self.channel_value_satoshis, @@ -1661,7 +1810,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(); @@ -1669,7 +1818,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(); @@ -1688,13 +1837,17 @@ 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(); let funding_txo_script = funding_redeemscript.to_v0_p2wsh(); let obscure_factor = get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound()); + let shutdown_script = self.shutdown_scriptpubkey.clone().map(|script| script.into_inner()); let channel_monitor = ChannelMonitor::new(self.secp_ctx.clone(), self.holder_signer.clone(), - &self.shutdown_pubkey, self.get_holder_selected_contest_delay(), + shutdown_script, self.get_holder_selected_contest_delay(), &self.destination_script, (funding_txo, funding_txo_script), &self.channel_transaction_parameters, funding_redeemscript.clone(), self.channel_value_satoshis, @@ -1750,32 +1903,63 @@ impl Channel { Ok(()) } - /// Returns (inbound_htlc_count, htlc_inbound_value_msat) - fn get_inbound_pending_htlc_stats(&self) -> (u32, u64) { - let mut htlc_inbound_value_msat = 0; + /// Returns a HTLCStats about inbound pending htlcs + fn get_inbound_pending_htlc_stats(&self) -> HTLCStats { + let mut stats = HTLCStats { + pending_htlcs: self.pending_inbound_htlcs.len() as u32, + pending_htlcs_value_msat: 0, + on_counterparty_tx_dust_exposure_msat: 0, + on_holder_tx_dust_exposure_msat: 0, + }; + + let counterparty_dust_limit_timeout_sat = (self.get_dust_buffer_feerate() as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000) + self.counterparty_dust_limit_satoshis; + let holder_dust_limit_success_sat = (self.get_dust_buffer_feerate() as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000) + self.holder_dust_limit_satoshis; for ref htlc in self.pending_inbound_htlcs.iter() { - htlc_inbound_value_msat += htlc.amount_msat; + stats.pending_htlcs_value_msat += htlc.amount_msat; + if htlc.amount_msat / 1000 < counterparty_dust_limit_timeout_sat { + stats.on_counterparty_tx_dust_exposure_msat += htlc.amount_msat; + } + if htlc.amount_msat / 1000 < holder_dust_limit_success_sat { + stats.on_holder_tx_dust_exposure_msat += htlc.amount_msat; + } } - (self.pending_inbound_htlcs.len() as u32, htlc_inbound_value_msat) + stats } - /// Returns (outbound_htlc_count, htlc_outbound_value_msat) *including* pending adds in our - /// holding cell. - fn get_outbound_pending_htlc_stats(&self) -> (u32, u64) { - let mut htlc_outbound_value_msat = 0; + /// Returns a HTLCStats about pending outbound htlcs, *including* pending adds in our holding cell. + fn get_outbound_pending_htlc_stats(&self) -> HTLCStats { + let mut stats = HTLCStats { + pending_htlcs: self.pending_outbound_htlcs.len() as u32, + pending_htlcs_value_msat: 0, + on_counterparty_tx_dust_exposure_msat: 0, + on_holder_tx_dust_exposure_msat: 0, + }; + + let counterparty_dust_limit_success_sat = (self.get_dust_buffer_feerate() as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000) + self.counterparty_dust_limit_satoshis; + let holder_dust_limit_timeout_sat = (self.get_dust_buffer_feerate() as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000) + self.holder_dust_limit_satoshis; for ref htlc in self.pending_outbound_htlcs.iter() { - htlc_outbound_value_msat += htlc.amount_msat; + stats.pending_htlcs_value_msat += htlc.amount_msat; + if htlc.amount_msat / 1000 < counterparty_dust_limit_success_sat { + stats.on_counterparty_tx_dust_exposure_msat += htlc.amount_msat; + } + if htlc.amount_msat / 1000 < holder_dust_limit_timeout_sat { + stats.on_holder_tx_dust_exposure_msat += htlc.amount_msat; + } } - let mut htlc_outbound_count = self.pending_outbound_htlcs.len(); for update in self.holding_cell_htlc_updates.iter() { if let &HTLCUpdateAwaitingACK::AddHTLC { ref amount_msat, .. } = update { - htlc_outbound_count += 1; - htlc_outbound_value_msat += amount_msat; + stats.pending_htlcs += 1; + stats.pending_htlcs_value_msat += amount_msat; + if *amount_msat / 1000 < counterparty_dust_limit_success_sat { + stats.on_counterparty_tx_dust_exposure_msat += amount_msat; + } + if *amount_msat / 1000 < holder_dust_limit_timeout_sat { + stats.on_holder_tx_dust_exposure_msat += amount_msat; + } } } - - (htlc_outbound_count as u32, htlc_outbound_value_msat) + stats } /// Get the available (ie not including pending HTLCs) inbound and outbound balance in msat. @@ -1784,8 +1968,22 @@ impl Channel { /// corner case properly. pub fn get_inbound_outbound_available_balance_msat(&self) -> (u64, u64) { // Note that we have to handle overflow due to the above case. - (cmp::max(self.channel_value_satoshis as i64 * 1000 - self.value_to_self_msat as i64 - self.get_inbound_pending_htlc_stats().1 as i64, 0) as u64, - cmp::max(self.value_to_self_msat as i64 - self.get_outbound_pending_htlc_stats().1 as i64, 0) as u64) + ( + cmp::max(self.channel_value_satoshis as i64 * 1000 + - self.value_to_self_msat as i64 + - self.get_inbound_pending_htlc_stats().pending_htlcs_value_msat as i64 + - Self::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) as i64 * 1000, + 0) as u64, + cmp::max(self.value_to_self_msat as i64 + - self.get_outbound_pending_htlc_stats().pending_htlcs_value_msat as i64 + - self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) as i64 * 1000, + 0) as u64 + ) + } + + pub fn get_holder_counterparty_selected_channel_reserve_satoshis(&self) -> (u64, Option) { + (Self::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis), + self.counterparty_selected_channel_reserve_satoshis) } // Get the fee cost of a commitment tx with a given number of HTLC outputs. @@ -1969,7 +2167,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); @@ -1989,12 +2187,13 @@ impl Channel { return Err(ChannelError::Close(format!("Remote side tried to send less than our minimum HTLC value. Lower limit: ({}). Actual: ({})", self.holder_htlc_minimum_msat, msg.amount_msat))); } - let (inbound_htlc_count, htlc_inbound_value_msat) = self.get_inbound_pending_htlc_stats(); - if inbound_htlc_count + 1 > OUR_MAX_HTLCS as u32 { + let inbound_stats = self.get_inbound_pending_htlc_stats(); + let outbound_stats = self.get_outbound_pending_htlc_stats(); + if inbound_stats.pending_htlcs + 1 > OUR_MAX_HTLCS as u32 { return Err(ChannelError::Close(format!("Remote tried to push more than our max accepted HTLCs ({})", OUR_MAX_HTLCS))); } let holder_max_htlc_value_in_flight_msat = Channel::::get_holder_max_htlc_value_in_flight_msat(self.channel_value_satoshis); - if htlc_inbound_value_msat + msg.amount_msat > holder_max_htlc_value_in_flight_msat { + if inbound_stats.pending_htlcs_value_msat + msg.amount_msat > holder_max_htlc_value_in_flight_msat { return Err(ChannelError::Close(format!("Remote HTLC add would put them over our max HTLC value ({})", holder_max_htlc_value_in_flight_msat))); } // Check holder_selected_channel_reserve_satoshis (we're getting paid, so they have to at least meet @@ -2018,8 +2217,28 @@ impl Channel { } } + let exposure_dust_limit_timeout_sats = (self.get_dust_buffer_feerate() as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000) + self.counterparty_dust_limit_satoshis; + if msg.amount_msat / 1000 < exposure_dust_limit_timeout_sats { + let on_counterparty_tx_dust_htlc_exposure_msat = inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat + msg.amount_msat; + if on_counterparty_tx_dust_htlc_exposure_msat > self.get_max_dust_htlc_exposure_msat() { + log_info!(logger, "Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", + on_counterparty_tx_dust_htlc_exposure_msat, self.get_max_dust_htlc_exposure_msat()); + pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7); + } + } + + let exposure_dust_limit_success_sats = (self.get_dust_buffer_feerate() as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000) + self.holder_dust_limit_satoshis; + if msg.amount_msat / 1000 < exposure_dust_limit_success_sats { + let on_holder_tx_dust_htlc_exposure_msat = inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat + msg.amount_msat; + if on_holder_tx_dust_htlc_exposure_msat > self.get_max_dust_htlc_exposure_msat() { + log_info!(logger, "Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", + on_holder_tx_dust_htlc_exposure_msat, self.get_max_dust_htlc_exposure_msat()); + pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7); + } + } + let pending_value_to_self_msat = - self.value_to_self_msat + htlc_inbound_value_msat - removed_outbound_total_msat; + self.value_to_self_msat + inbound_stats.pending_htlcs_value_msat - removed_outbound_total_msat; let pending_remote_value_msat = self.channel_value_satoshis * 1000 - pending_value_to_self_msat; if pending_remote_value_msat < msg.amount_msat { @@ -2094,7 +2313,7 @@ impl Channel { /// Marks an outbound HTLC which we have received update_fail/fulfill/malformed #[inline] - fn mark_outbound_htlc_removed(&mut self, htlc_id: u64, check_preimage: Option, fail_reason: Option) -> Result<&HTLCSource, ChannelError> { + fn mark_outbound_htlc_removed(&mut self, htlc_id: u64, check_preimage: Option, fail_reason: Option) -> Result<&OutboundHTLCOutput, ChannelError> { for htlc in self.pending_outbound_htlcs.iter_mut() { if htlc.htlc_id == htlc_id { match check_preimage { @@ -2113,13 +2332,13 @@ impl Channel { OutboundHTLCState::AwaitingRemoteRevokeToRemove(_) | OutboundHTLCState::AwaitingRemovedRemoteRevoke(_) | OutboundHTLCState::RemoteRemoved(_) => return Err(ChannelError::Close(format!("Remote tried to fulfill/fail HTLC ({}) that they'd already fulfilled/failed", htlc_id))), } - return Ok(&htlc.source); + return Ok(htlc); } } Err(ChannelError::Close("Remote tried to fulfill/fail an HTLC we couldn't find".to_owned())) } - pub fn update_fulfill_htlc(&mut self, msg: &msgs::UpdateFulfillHTLC) -> Result { + pub fn update_fulfill_htlc(&mut self, msg: &msgs::UpdateFulfillHTLC) -> Result<(HTLCSource, u64), ChannelError> { if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) { return Err(ChannelError::Close("Got fulfill HTLC message when channel was not in an operational state".to_owned())); } @@ -2128,7 +2347,7 @@ impl Channel { } let payment_hash = PaymentHash(Sha256::hash(&msg.payment_preimage.0[..]).into_inner()); - self.mark_outbound_htlc_removed(msg.htlc_id, Some(payment_hash), None).map(|source| source.clone()) + self.mark_outbound_htlc_removed(msg.htlc_id, Some(payment_hash), None).map(|htlc| (htlc.source.clone(), htlc.amount_msat)) } pub fn update_fail_htlc(&mut self, msg: &msgs::UpdateFailHTLC, fail_reason: HTLCFailReason) -> Result<(), ChannelError> { @@ -2155,9 +2374,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()))); @@ -2173,16 +2391,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(); @@ -2197,12 +2407,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 { @@ -2262,20 +2477,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; } } @@ -2332,12 +2543,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(). @@ -2346,10 +2555,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 { "" }); @@ -2358,7 +2565,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. @@ -2421,24 +2628,28 @@ impl Channel { } }, &HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_preimage, htlc_id, .. } => { - match self.get_update_fulfill_htlc(htlc_id, *payment_preimage, logger) { - Ok((update_fulfill_msg_option, additional_monitor_update_opt)) => { - update_fulfill_htlcs.push(update_fulfill_msg_option.unwrap()); - if let Some(mut additional_monitor_update) = additional_monitor_update_opt { - monitor_update.updates.append(&mut additional_monitor_update.updates); - } - }, - Err(e) => { - if let ChannelError::Ignore(_) = e {} - else { - panic!("Got a non-IgnoreError action trying to fulfill holding cell HTLC"); - } - } - } + // If an HTLC claim was previously added to the holding cell (via + // `get_update_fulfill_htlc`, then generating the claim message itself must + // not fail - any in between attempts to claim the HTLC will have resulted + // in it hitting the holding cell again and we cannot change the state of a + // holding cell HTLC from fulfill to anything else. + let (update_fulfill_msg_option, mut additional_monitor_update) = + if let UpdateFulfillFetch::NewClaim { msg, monitor_update, .. } = self.get_update_fulfill_htlc(htlc_id, *payment_preimage, logger) { + (msg, monitor_update) + } else { unreachable!() }; + update_fulfill_htlcs.push(update_fulfill_msg_option.unwrap()); + monitor_update.updates.append(&mut additional_monitor_update.updates); }, &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, ref err_packet } => { match self.get_update_fail_htlc(htlc_id, err_packet.clone(), logger) { - Ok(update_fail_msg_option) => update_fail_htlcs.push(update_fail_msg_option.unwrap()), + Ok(update_fail_msg_option) => { + // If an HTLC failure was previously added to the holding cell (via + // `get_update_fail_htlc`) then generating the fail message itself + // must not fail - we should never end up in a state where we + // double-fail an HTLC or fail-then-claim an HTLC as it indicates + // we didn't wait for a full revocation before failing. + update_fail_htlcs.push(update_fail_msg_option.unwrap()) + }, Err(e) => { if let ChannelError::Ignore(_) = e {} else { @@ -2452,8 +2663,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, @@ -2490,9 +2702,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<(Option, Vec<(PendingHTLCInfo, u64)>, Vec<(HTLCSource, PaymentHash, HTLCFailReason)>, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>), ChannelError> + 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())); @@ -2504,8 +2715,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())); } } @@ -2527,6 +2740,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; @@ -2637,21 +2855,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; - } + }, } } @@ -2672,7 +2891,7 @@ impl Channel { self.monitor_pending_forwards.append(&mut to_forward_infos); self.monitor_pending_failures.append(&mut revoked_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((None, Vec::new(), Vec::new(), monitor_update, Vec::new())) } match self.free_holding_cell_htlcs(logger)? { @@ -2691,7 +2910,7 @@ 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((Some(commitment_update), to_forward_infos, revoked_htlcs, monitor_update, htlcs_to_fail)) }, (None, htlcs_to_fail) => { if require_commitment { @@ -2711,14 +2930,13 @@ impl Channel { update_fail_malformed_htlcs, update_fee: None, commitment_signed - }), to_forward_infos, revoked_htlcs, None, monitor_update, htlcs_to_fail)) + }), to_forward_infos, revoked_htlcs, monitor_update, 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((None, to_forward_infos, revoked_htlcs, monitor_update, htlcs_to_fail)) } } } - } /// Adds a pending update to this channel. See the doc for send_htlc for @@ -2735,13 +2953,13 @@ impl Channel { panic!("Cannot update fee while peer is disconnected/we're awaiting a monitor update (ChannelManager should have caught this)"); } - if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == (ChannelState::AwaitingRemoteRevoke as u32) { + if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32 | ChannelState::MonitorUpdateFailed as u32)) != 0 { self.holding_cell_update_fee = Some(feerate_per_kw); return None; } 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, @@ -2773,6 +2991,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| { @@ -2802,6 +3022,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 @@ -2822,13 +3049,10 @@ impl Channel { /// 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); + 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.channel_state |= ChannelState::MonitorUpdateFailed as u32; } @@ -2896,8 +3120,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(()) } @@ -2959,11 +3202,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, } } @@ -3007,6 +3257,7 @@ impl Channel { self.channel_state &= !(ChannelState::PeerDisconnected as u32); let shutdown_msg = if self.channel_state & (ChannelState::LocalShutdownSent as u32) != 0 { + assert!(self.shutdown_scriptpubkey.is_some()); Some(msgs::Shutdown { channel_id: self.channel_id, scriptpubkey: self.get_closing_scriptpubkey(), @@ -3076,7 +3327,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)); }, @@ -3105,39 +3357,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; - } + 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, their_features: &InitFeatures, msg: &msgs::Shutdown) -> Result<(Option, Option, Vec<(HTLCSource, PaymentHash)>), ChannelError> - where F::Target: FeeEstimator + 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())); @@ -3155,7 +3493,7 @@ impl Channel { } assert_eq!(self.channel_state & ChannelState::ShutdownComplete as u32, 0); - if is_unsupported_shutdown_script(&their_features, &msg.scriptpubkey) { + 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()))); } @@ -3167,11 +3505,45 @@ impl Channel { self.counterparty_shutdown_scriptpubkey = Some(msg.scriptpubkey.clone()); } + // If we have any LocalAnnounced updates we'll probably just get back an update_fail_htlc + // immediately after the commitment dance, but we can send a Shutdown because we won't send + // any further commitment updates after we set LocalShutdownSent. + let send_shutdown = (self.channel_state & ChannelState::LocalShutdownSent as u32) != ChannelState::LocalShutdownSent as u32; + + let update_shutdown_script = match self.shutdown_scriptpubkey { + Some(_) => false, + None => { + assert!(send_shutdown); + let shutdown_scriptpubkey = keys_provider.get_shutdown_scriptpubkey(); + if !shutdown_scriptpubkey.is_compatible(their_features) { + return Err(ChannelError::Close(format!("Provided a scriptpubkey format not accepted by peer: {}", shutdown_scriptpubkey))); + } + self.shutdown_scriptpubkey = Some(shutdown_scriptpubkey); + true + }, + }; + // From here on out, we may not fail! self.channel_state |= ChannelState::RemoteShutdownSent as u32; self.update_time_counter += 1; + let monitor_update = if update_shutdown_script { + self.latest_monitor_update_id += 1; + Some(ChannelMonitorUpdate { + update_id: self.latest_monitor_update_id, + updates: vec![ChannelMonitorUpdateStep::ShutdownScript { + scriptpubkey: self.get_closing_scriptpubkey(), + }], + }) + } else { None }; + let shutdown = if send_shutdown { + Some(msgs::Shutdown { + channel_id: self.channel_id, + scriptpubkey: self.get_closing_scriptpubkey(), + }) + } else { None }; + // We can't send our shutdown until we've committed all of our pending HTLCs, but the // remote side is unlikely to accept any new HTLCs, so we go ahead and "free" any holding // cell HTLCs and return them to fail the payment. @@ -3186,29 +3558,15 @@ impl Channel { _ => true } }); - // If we have any LocalAnnounced updates we'll probably just get back a update_fail_htlc - // immediately after the commitment dance, but we can send a Shutdown cause we won't send - // any further commitment updates after we set LocalShutdownSent. - - let shutdown = if (self.channel_state & ChannelState::LocalShutdownSent as u32) == ChannelState::LocalShutdownSent as u32 { - None - } else { - Some(msgs::Shutdown { - channel_id: self.channel_id, - scriptpubkey: self.get_closing_scriptpubkey(), - }) - }; self.channel_state |= ChannelState::LocalShutdownSent as u32; self.update_time_counter += 1; - Ok((shutdown, self.maybe_propose_first_closing_signed(fee_estimator), dropped_outbound_htlcs)) + Ok((shutdown, 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 @@ -3225,6 +3583,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> @@ -3243,12 +3602,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(_) => {}, @@ -3256,80 +3624,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) => { - 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: @@ -3338,6 +3742,10 @@ impl Channel { self.channel_id } + pub fn minimum_depth(&self) -> Option { + self.minimum_depth + } + /// Gets the "user_id" value passed into the construction of this channel. It has no special /// meaning and exists only to allow users to have a persistent identifier of a channel. pub fn get_user_id(&self) -> u64 { @@ -3365,7 +3773,7 @@ impl Channel { &self.channel_transaction_parameters.holder_pubkeys } - fn get_counterparty_selected_contest_delay(&self) -> Option { + pub fn get_counterparty_selected_contest_delay(&self) -> Option { self.channel_transaction_parameters.counterparty_parameters .as_ref().map(|params| params.selected_contest_delay) } @@ -3407,18 +3815,36 @@ impl Channel { } pub fn get_fee_proportional_millionths(&self) -> u32 { - self.config.fee_proportional_millionths + self.config.forwarding_fee_proportional_millionths } pub fn get_cltv_expiry_delta(&self) -> u16 { cmp::max(self.config.cltv_expiry_delta, MIN_CLTV_EXPIRY_DELTA) } - #[cfg(test)] + pub fn get_max_dust_htlc_exposure_msat(&self) -> u64 { + self.config.max_dust_htlc_exposure_msat + } + pub fn get_feerate(&self) -> u32 { self.feerate_per_kw } + pub fn get_dust_buffer_feerate(&self) -> u32 { + // When calculating our exposure to dust HTLCs, we assume that the channel feerate + // may, at any point, increase by at least 10 sat/vB (i.e 2530 sat/kWU) or 25%, + // 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. + 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 { self.cur_holder_commitment_transaction_number + 1 } @@ -3480,24 +3906,8 @@ impl Channel { /// Gets the fee we'd want to charge for adding an HTLC output to this Channel /// Allowed in any state (including after shutdown) - pub fn get_holder_fee_base_msat(&self, fee_estimator: &F) -> u32 - where F::Target: FeeEstimator - { - // For lack of a better metric, we calculate what it would cost to consolidate the new HTLC - // output value back into a transaction with the regular channel output: - - // the fee cost of the HTLC-Success/HTLC-Timeout transaction: - let mut res = self.feerate_per_kw as u64 * cmp::max(HTLC_TIMEOUT_TX_WEIGHT, HTLC_SUCCESS_TX_WEIGHT) / 1000; - - if self.is_outbound() { - // + the marginal fee increase cost to us in the commitment transaction: - res += self.feerate_per_kw as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC / 1000; - } - - // + the marginal cost of an input which spends the HTLC-Success/HTLC-Timeout output: - res += fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal) as u64 * SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT / 1000; - - res as u32 + pub fn get_outbound_forwarding_fee_base_msat(&self) -> u32 { + self.config.forwarding_fee_base_msat } /// Returns true if we've ever received a message from the remote end for this Channel @@ -3530,6 +3940,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. @@ -3787,7 +4207,10 @@ impl Channel { htlc_basepoint: keys.htlc_basepoint, first_per_commitment_point, channel_flags: if self.config.announced_channel {1} else {0}, - shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() }) + shutdown_scriptpubkey: OptionalField::Present(match &self.shutdown_scriptpubkey { + Some(script) => script.clone().into_inner(), + None => Builder::new().into_script(), + }), } } @@ -3820,14 +4243,17 @@ impl Channel { delayed_payment_basepoint: keys.delayed_payment_basepoint, htlc_basepoint: keys.htlc_basepoint, first_per_commitment_point, - shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() }) + shutdown_scriptpubkey: OptionalField::Present(match &self.shutdown_scriptpubkey { + Some(script) => script.clone().into_inner(), + None => Builder::new().into_script(), + }), } } /// 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) } @@ -4073,19 +4499,20 @@ impl Channel { return Err(ChannelError::Ignore("Cannot send an HTLC while disconnected from channel counterparty".to_owned())); } - let (outbound_htlc_count, htlc_outbound_value_msat) = self.get_outbound_pending_htlc_stats(); - if outbound_htlc_count + 1 > self.counterparty_max_accepted_htlcs as u32 { + let inbound_stats = self.get_inbound_pending_htlc_stats(); + let outbound_stats = self.get_outbound_pending_htlc_stats(); + if outbound_stats.pending_htlcs + 1 > self.counterparty_max_accepted_htlcs as u32 { return Err(ChannelError::Ignore(format!("Cannot push more than their max accepted HTLCs ({})", self.counterparty_max_accepted_htlcs))); } // Check their_max_htlc_value_in_flight_msat - if htlc_outbound_value_msat + amount_msat > self.counterparty_max_htlc_value_in_flight_msat { + if outbound_stats.pending_htlcs_value_msat + amount_msat > self.counterparty_max_htlc_value_in_flight_msat { return Err(ChannelError::Ignore(format!("Cannot send value that would put us over the max HTLC value in flight our peer will accept ({})", self.counterparty_max_htlc_value_in_flight_msat))); } if !self.is_outbound() { // Check that we won't violate the remote channel reserve by adding this HTLC. let counterparty_balance_msat = self.channel_value_satoshis * 1000 - self.value_to_self_msat; - let holder_selected_chan_reserve_msat = Channel::::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis); + let holder_selected_chan_reserve_msat = Channel::::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) * 1000; let htlc_candidate = HTLCCandidate::new(amount_msat, HTLCInitiator::LocalOffered); let counterparty_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(htlc_candidate, None); if counterparty_balance_msat < holder_selected_chan_reserve_msat + counterparty_commit_tx_fee_msat { @@ -4093,7 +4520,25 @@ impl Channel { } } - let pending_value_to_self_msat = self.value_to_self_msat - htlc_outbound_value_msat; + let exposure_dust_limit_success_sats = (self.get_dust_buffer_feerate() as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000) + self.counterparty_dust_limit_satoshis; + if amount_msat / 1000 < exposure_dust_limit_success_sats { + let on_counterparty_dust_htlc_exposure_msat = inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat + amount_msat; + if on_counterparty_dust_htlc_exposure_msat > self.get_max_dust_htlc_exposure_msat() { + return Err(ChannelError::Ignore(format!("Cannot send value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", + on_counterparty_dust_htlc_exposure_msat, self.get_max_dust_htlc_exposure_msat()))); + } + } + + let exposure_dust_limit_timeout_sats = (self.get_dust_buffer_feerate() as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000) + self.holder_dust_limit_satoshis; + if amount_msat / 1000 < exposure_dust_limit_timeout_sats { + let on_holder_dust_htlc_exposure_msat = inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat + amount_msat; + if on_holder_dust_htlc_exposure_msat > self.get_max_dust_htlc_exposure_msat() { + return Err(ChannelError::Ignore(format!("Cannot send value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", + on_holder_dust_htlc_exposure_msat, self.get_max_dust_htlc_exposure_msat()))); + } + } + + let pending_value_to_self_msat = self.value_to_self_msat - outbound_stats.pending_htlcs_value_msat; if pending_value_to_self_msat < amount_msat { return Err(ChannelError::Ignore(format!("Cannot send value that would overdraw remaining funds. Amount: {}, pending value to self {}", amount_msat, pending_value_to_self_msat))); } @@ -4101,7 +4546,7 @@ impl Channel { // `2 *` and extra HTLC are for the fee spike buffer. let commit_tx_fee_msat = if self.is_outbound() { let htlc_candidate = HTLCCandidate::new(amount_msat, HTLCInitiator::LocalOffered); - 2 * self.next_local_commit_tx_fee_msat(htlc_candidate, Some(())) + FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * self.next_local_commit_tx_fee_msat(htlc_candidate, Some(())) } else { 0 }; if pending_value_to_self_msat - amount_msat < commit_tx_fee_msat { return Err(ChannelError::Ignore(format!("Cannot send value that would not leave enough to pay for fees. Pending value to self: {}. local_commit_tx_fee {}", pending_value_to_self_msat, commit_tx_fee_msat))); @@ -4165,7 +4610,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; @@ -4185,6 +4630,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. @@ -4193,6 +4639,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; } } @@ -4200,9 +4647,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) { @@ -4232,15 +4688,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); @@ -4255,7 +4705,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); } } @@ -4263,8 +4713,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); } @@ -4291,7 +4741,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 @@ -4328,7 +4778,9 @@ impl Channel { /// Begins the shutdown process, getting a message for the remote peer and returning all /// holding cell HTLCs for payment failure. - pub fn get_shutdown(&mut self) -> Result<(msgs::Shutdown, Vec<(HTLCSource, PaymentHash)>), APIError> { + pub fn get_shutdown(&mut self, keys_provider: &K, their_features: &InitFeatures, 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 { return Err(APIError::APIMisuseError{err: "Cannot begin shutdown with pending HTLCs. Process pending events first".to_owned()}); @@ -4347,9 +4799,20 @@ impl Channel { return Err(APIError::ChannelUnavailable{err: "Cannot begin shutdown while peer is disconnected or we're waiting on a monitor update, maybe force-close instead?".to_owned()}); } - let closing_script = self.get_closing_scriptpubkey(); + let update_shutdown_script = match self.shutdown_scriptpubkey { + Some(_) => false, + None => { + let shutdown_scriptpubkey = keys_provider.get_shutdown_scriptpubkey(); + if !shutdown_scriptpubkey.is_compatible(their_features) { + return Err(APIError::IncompatibleShutdownScript { script: shutdown_scriptpubkey.clone() }); + } + self.shutdown_scriptpubkey = Some(shutdown_scriptpubkey); + true + }, + }; // 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 { @@ -4357,6 +4820,20 @@ impl Channel { } self.update_time_counter += 1; + let monitor_update = if update_shutdown_script { + self.latest_monitor_update_id += 1; + Some(ChannelMonitorUpdate { + update_id: self.latest_monitor_update_id, + updates: vec![ChannelMonitorUpdateStep::ShutdownScript { + scriptpubkey: self.get_closing_scriptpubkey(), + }], + }) + } else { None }; + let shutdown = msgs::Shutdown { + channel_id: self.channel_id, + scriptpubkey: self.get_closing_scriptpubkey(), + }; + // Go ahead and drop holding cell updates as we'd rather fail payments than wait to send // our shutdown until we've committed all of the pending changes. self.holding_cell_update_fee = None; @@ -4371,10 +4848,7 @@ impl Channel { } }); - Ok((msgs::Shutdown { - channel_id: self.channel_id, - scriptpubkey: closing_script, - }, dropped_outbound_htlcs)) + Ok((shutdown, monitor_update, dropped_outbound_htlcs)) } /// Gets the latest commitment transaction and any dependent transactions for relay (forcing @@ -4423,25 +4897,7 @@ impl Channel { } } -fn is_unsupported_shutdown_script(their_features: &InitFeatures, script: &Script) -> bool { - // We restrain shutdown scripts to standards forms to avoid transactions not propagating on the p2p tx-relay network - - // BOLT 2 says we must only send a scriptpubkey of certain standard forms, - // which for a a BIP-141-compliant witness program is at max 42 bytes in length. - // So don't let the remote peer feed us some super fee-heavy script. - let is_script_too_long = script.len() > 42; - if is_script_too_long { - return true; - } - - if their_features.supports_shutdown_anysegwit() && script.is_witness_program() && script.as_bytes()[0] != OP_PUSHBYTES_0.into_u8() { - return false; - } - - return !script.is_p2pkh() && !script.is_p2sh() && !script.is_v0_p2wpkh() && !script.is_v0_p2wsh() -} - -const SERIALIZATION_VERSION: u8 = 1; +const SERIALIZATION_VERSION: u8 = 2; const MIN_SERIALIZATION_VERSION: u8 = 1; impl_writeable_tlv_based_enum!(InboundHTLCRemovalReason,; @@ -4451,7 +4907,7 @@ impl_writeable_tlv_based_enum!(InboundHTLCRemovalReason,; ); impl Writeable for ChannelUpdateStatus { - fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { + fn write(&self, writer: &mut W) -> Result<(), io::Error> { // We only care about writing out the current state as it was announced, ie only either // Enabled or Disabled. In the case of DisabledStaged, we most recently announced the // channel as enabled, so we write 0. For EnabledStaged, we similarly write a 1. @@ -4466,7 +4922,7 @@ impl Writeable for ChannelUpdateStatus { } impl Readable for ChannelUpdateStatus { - fn read(reader: &mut R) -> Result { + fn read(reader: &mut R) -> Result { Ok(match ::read(reader)? { 0 => ChannelUpdateStatus::Enabled, 1 => ChannelUpdateStatus::Disabled, @@ -4476,14 +4932,20 @@ impl Readable for ChannelUpdateStatus { } impl Writeable for Channel { - fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { + fn write(&self, writer: &mut W) -> Result<(), io::Error> { // Note that we write out as if remove_uncommitted_htlcs_and_mark_paused had just been // called. write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION); self.user_id.write(writer)?; - self.config.write(writer)?; + + // Write out the old serialization for the config object. This is read by version-1 + // deserializers, but we will read the version in the TLV at the end instead. + self.config.forwarding_fee_proportional_millionths.write(writer)?; + self.config.cltv_expiry_delta.write(writer)?; + self.config.announced_channel.write(writer)?; + self.config.commit_upfront_shutdown_pubkey.write(writer)?; self.channel_id.write(writer)?; (self.channel_state | ChannelState::PeerDisconnected as u32).write(writer)?; @@ -4498,7 +4960,12 @@ impl Writeable for Channel { (key_data.0.len() as u32).write(writer)?; writer.write_all(&key_data.0[..])?; - self.shutdown_pubkey.write(writer)?; + // Write out the old serialization for shutdown_pubkey for backwards compatibility, if + // deserialized from that format. + match self.shutdown_scriptpubkey.as_ref().and_then(|script| script.as_legacy_pubkey()) { + Some(shutdown_pubkey) => shutdown_pubkey.write(writer)?, + None => [0u8; PUBLIC_KEY_SIZE].write(writer)?, + } self.destination_script.write(writer)?; self.cur_holder_commitment_transaction_number.write(writer)?; @@ -4617,7 +5084,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)?; @@ -4625,15 +5100,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)?; @@ -4642,10 +5113,15 @@ impl Writeable for Channel { self.counterparty_dust_limit_satoshis.write(writer)?; self.holder_dust_limit_satoshis.write(writer)?; self.counterparty_max_htlc_value_in_flight_msat.write(writer)?; + + // Note that this field is ignored by 0.0.99+ as the TLV Optional variant is used instead. self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0).write(writer)?; + self.counterparty_htlc_minimum_msat.write(writer)?; self.holder_htlc_minimum_msat.write(writer)?; self.counterparty_max_accepted_htlcs.write(writer)?; + + // Note that this field is ignored by 0.0.99+ as the TLV Optional variant is used instead. self.minimum_depth.unwrap_or(0).write(writer)?; match &self.counterparty_forwarding_info { @@ -4671,6 +5147,13 @@ impl Writeable for Channel { self.channel_update_status.write(writer)?; + #[cfg(any(test, feature = "fuzztarget"))] + (self.historical_inbound_htlc_fulfills.len() as u64).write(writer)?; + #[cfg(any(test, feature = "fuzztarget"))] + for htlc in self.historical_inbound_htlc_fulfills.iter() { + htlc.write(writer)?; + } + write_tlv_fields!(writer, { (0, self.announcement_sigs, option), // minimum_depth and counterparty_selected_channel_reserve_satoshis used to have a @@ -4681,6 +5164,9 @@ impl Writeable for Channel { // override that. (1, self.minimum_depth, option), (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), }); Ok(()) @@ -4690,11 +5176,22 @@ impl Writeable for Channel { const MAX_ALLOC_SIZE: usize = 64*1024; impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel where K::Target: KeysInterface { - fn read(reader: &mut R, keys_source: &'a K) -> Result { - let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION); + fn read(reader: &mut R, keys_source: &'a K) -> Result { + let ver = read_ver_prefix!(reader, SERIALIZATION_VERSION); let user_id = Readable::read(reader)?; - let config: ChannelConfig = Readable::read(reader)?; + + let mut config = Some(ChannelConfig::default()); + if ver == 1 { + // Read the old serialization of the ChannelConfig from version 0.0.98. + config.as_mut().unwrap().forwarding_fee_proportional_millionths = Readable::read(reader)?; + config.as_mut().unwrap().cltv_expiry_delta = Readable::read(reader)?; + config.as_mut().unwrap().announced_channel = Readable::read(reader)?; + config.as_mut().unwrap().commit_upfront_shutdown_pubkey = Readable::read(reader)?; + } else { + // Read the 8 bytes of backwards-compatibility ChannelConfig data. + let mut _val: u64 = Readable::read(reader)?; + } let channel_id = Readable::read(reader)?; let channel_state = Readable::read(reader)?; @@ -4713,7 +5210,11 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel } let holder_signer = keys_source.read_chan_signer(&keys_data)?; - let shutdown_pubkey = Readable::read(reader)?; + // Read the old serialization for shutdown_pubkey, preferring the TLV field later if set. + let mut shutdown_scriptpubkey = match ::read(reader) { + Ok(pubkey) => Some(ShutdownScript::new_p2wpkh_from_pubkey(pubkey)), + Err(_) => None, + }; let destination_script = Readable::read(reader)?; let cur_holder_commitment_transaction_number = Readable::read(reader)?; @@ -4803,7 +5304,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)?; @@ -4811,11 +5313,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)?; @@ -4824,20 +5334,25 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel let counterparty_dust_limit_satoshis = Readable::read(reader)?; let holder_dust_limit_satoshis = Readable::read(reader)?; let counterparty_max_htlc_value_in_flight_msat = Readable::read(reader)?; - let mut counterparty_selected_channel_reserve_satoshis = Some(Readable::read(reader)?); - if counterparty_selected_channel_reserve_satoshis == Some(0) { - // Versions up to 0.0.98 had counterparty_selected_channel_reserve_satoshis as a - // non-option, writing 0 for what we now consider None. - counterparty_selected_channel_reserve_satoshis = None; + let mut counterparty_selected_channel_reserve_satoshis = None; + if ver == 1 { + // Read the old serialization from version 0.0.98. + counterparty_selected_channel_reserve_satoshis = Some(Readable::read(reader)?); + } else { + // Read the 8 bytes of backwards-compatibility data. + let _dummy: u64 = Readable::read(reader)?; } let counterparty_htlc_minimum_msat = Readable::read(reader)?; let holder_htlc_minimum_msat = Readable::read(reader)?; let counterparty_max_accepted_htlcs = Readable::read(reader)?; - let mut minimum_depth = Some(Readable::read(reader)?); - if minimum_depth == Some(0) { - // Versions up to 0.0.98 had minimum_depth as a non-option, writing 0 for what we now - // consider None. - minimum_depth = None; + + let mut minimum_depth = None; + if ver == 1 { + // Read the old serialization from version 0.0.98. + minimum_depth = Some(Readable::read(reader)?); + } else { + // Read the 4 bytes of backwards-compatibility data. + let _dummy: u32 = Readable::read(reader)?; } let counterparty_forwarding_info = match ::read(reader)? { @@ -4850,7 +5365,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)?; @@ -4863,11 +5378,35 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel let channel_update_status = Readable::read(reader)?; + #[cfg(any(test, feature = "fuzztarget"))] + let mut historical_inbound_htlc_fulfills = HashSet::new(); + #[cfg(any(test, feature = "fuzztarget"))] + { + let htlc_fulfills_len: u64 = Readable::read(reader)?; + for _ in 0..htlc_fulfills_len { + assert!(historical_inbound_htlc_fulfills.insert(Readable::read(reader)?)); + } + } + + let 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; 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), }); let mut secp_ctx = Secp256k1::new(); @@ -4876,7 +5415,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel Ok(Channel { user_id, - config, + config: config.unwrap(), channel_id, channel_state, secp_ctx, @@ -4885,7 +5424,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel latest_monitor_update_id, holder_signer, - shutdown_pubkey, + shutdown_scriptpubkey, destination_script, cur_holder_commitment_transaction_number, @@ -4912,11 +5451,14 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel feerate_per_kw, #[cfg(debug_assertions)] - holder_max_commitment_tx_output: ::std::sync::Mutex::new((0, 0)), + holder_max_commitment_tx_output: Mutex::new((0, 0)), #[cfg(debug_assertions)] - counterparty_max_commitment_tx_output: ::std::sync::Mutex::new((0, 0)), + counterparty_max_commitment_tx_output: Mutex::new((0, 0)), - last_sent_closing_fee, + 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, @@ -4945,6 +5487,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel commitment_secrets, channel_update_status, + closing_signed_in_flight: false, announcement_sigs, @@ -4954,6 +5497,9 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel next_remote_commitment_tx_fee_info_cached: Mutex::new(None), workaround_lnd_bug_4006: None, + + #[cfg(any(test, feature = "fuzztarget"))] + historical_inbound_htlc_fulfills, }) } } @@ -4970,19 +5516,23 @@ mod tests { use bitcoin::hashes::hex::FromHex; use hex; use ln::{PaymentPreimage, PaymentHash}; - use ln::channelmanager::{BestBlock, 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; use ln::msgs::{ChannelUpdate, DataLossProtect, DecodeError, OptionalField, UnsignedChannelUpdate}; + use ln::script::ShutdownScript; use ln::chan_utils; use ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT}; + use chain::BestBlock; use chain::chaininterface::{FeeEstimator,ConfirmationTarget}; use chain::keysinterface::{InMemorySigner, KeysInterface, BaseSign}; use chain::transaction::OutPoint; use util::config::UserConfig; use util::enforcing_trait_impls::EnforcingSigner; + use util::errors::APIError; use util::test_utils; + use util::test_utils::OnGetShutdownScriptpubkey; use util::logger::Logger; use bitcoin::secp256k1::{Secp256k1, Message, Signature, All}; use bitcoin::secp256k1::ffi::Signature as FFISignature; @@ -4991,7 +5541,8 @@ mod tests { use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::Hash; use bitcoin::hash_types::{Txid, WPubkeyHash}; - use std::sync::Arc; + use core::num::NonZeroU8; + use sync::Arc; use prelude::*; struct TestFeeEstimator { @@ -5023,10 +5574,10 @@ mod tests { Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&channel_monitor_claim_key_hash[..]).into_script() } - fn get_shutdown_pubkey(&self) -> PublicKey { + fn get_shutdown_scriptpubkey(&self) -> ShutdownScript { let secp_ctx = Secp256k1::signing_only(); let channel_close_key = SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(); - PublicKey::from_secret_key(&secp_ctx, &channel_close_key) + ShutdownScript::new_p2wpkh_from_pubkey(PublicKey::from_secret_key(&secp_ctx, &channel_close_key)) } fn get_channel_signer(&self, _inbound: bool, _channel_value_satoshis: u64) -> InMemorySigner { @@ -5041,6 +5592,32 @@ mod tests { PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&hex::decode(hex).unwrap()[..]).unwrap()) } + #[test] + fn upfront_shutdown_script_incompatibility() { + let features = InitFeatures::known().clear_shutdown_anysegwit(); + let non_v0_segwit_shutdown_script = + ShutdownScript::new_witness_program(NonZeroU8::new(16).unwrap(), &[0, 40]).unwrap(); + + let seed = [42; 32]; + let network = Network::Testnet; + let keys_provider = test_utils::TestKeysInterface::new(&seed, network); + keys_provider.expect(OnGetShutdownScriptpubkey { + returns: non_v0_segwit_shutdown_script.clone(), + }); + + let fee_estimator = TestFeeEstimator { fee_est: 253 }; + let secp_ctx = Secp256k1::new(); + let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); + let config = UserConfig::default(); + match Channel::::new_outbound(&&fee_estimator, &&keys_provider, node_id, &features, 10000000, 100000, 42, &config) { + Err(APIError::IncompatibleShutdownScript { script }) => { + assert_eq!(script.into_inner(), non_v0_segwit_shutdown_script.into_inner()); + }, + Err(e) => panic!("Unexpected error: {:?}", e), + Ok(_) => panic!("Expected error"), + } + } + // Check that, during channel creation, we use the same feerate in the open channel message // as we do in the Channel object creation itself. #[test] @@ -5054,7 +5631,7 @@ mod tests { let node_a_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let node_a_chan = Channel::::new_outbound(&&fee_est, &&keys_provider, node_a_node_id, 10000000, 100000, 42, &config).unwrap(); + let node_a_chan = Channel::::new_outbound(&&fee_est, &&keys_provider, node_a_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config).unwrap(); // Now change the fee so we can check that the fee in the open_channel message is the // same as the old fee. @@ -5079,18 +5656,18 @@ mod tests { // Create Node A's channel pointing to Node B's pubkey let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, 10000000, 100000, 42, &config).unwrap(); + let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config).unwrap(); // Create Node B's channel by receiving Node A's open_channel message // Make sure A's dust limit is as we expect. let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.block_hash()); let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap()); - let node_b_chan = Channel::::new_from_req(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), &open_channel_msg, 7, &config).unwrap(); + let node_b_chan = Channel::::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config).unwrap(); // Node B --> Node A: accept channel, explicitly setting B's dust limit. let mut accept_channel_msg = node_b_chan.get_accept_channel(); accept_channel_msg.dust_limit_satoshis = 546; - node_a_chan.accept_channel(&accept_channel_msg, &config, InitFeatures::known()).unwrap(); + node_a_chan.accept_channel(&accept_channel_msg, &config, &InitFeatures::known()).unwrap(); node_a_chan.holder_dust_limit_satoshis = 1560; // Put some inbound and outbound HTLCs in A's channel. @@ -5113,6 +5690,7 @@ 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]), } }); @@ -5146,7 +5724,7 @@ mod tests { let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let mut chan = Channel::::new_outbound(&&fee_est, &&keys_provider, node_id, 10000000, 100000, 42, &config).unwrap(); + let mut chan = Channel::::new_outbound(&&fee_est, &&keys_provider, node_id, &InitFeatures::known(), 10000000, 100000, 42, &config).unwrap(); let commitment_tx_fee_0_htlcs = chan.commit_tx_fee_msat(0); let commitment_tx_fee_1_htlc = chan.commit_tx_fee_msat(1); @@ -5195,16 +5773,16 @@ mod tests { // Create Node A's channel pointing to Node B's pubkey let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, 10000000, 100000, 42, &config).unwrap(); + let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config).unwrap(); // Create Node B's channel by receiving Node A's open_channel message let open_channel_msg = node_a_chan.get_open_channel(chain_hash); let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap()); - let mut node_b_chan = Channel::::new_from_req(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), &open_channel_msg, 7, &config).unwrap(); + let mut node_b_chan = Channel::::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config).unwrap(); // Node B --> Node A: accept channel let accept_channel_msg = node_b_chan.get_accept_channel(); - node_a_chan.accept_channel(&accept_channel_msg, &config, InitFeatures::known()).unwrap(); + node_a_chan.accept_channel(&accept_channel_msg, &config, &InitFeatures::known()).unwrap(); // Node A --> Node B: funding created let output_script = node_a_chan.get_funding_redeemscript(); @@ -5257,7 +5835,7 @@ mod tests { // Create a channel. let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, 10000000, 100000, 42, &config).unwrap(); + let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config).unwrap(); assert!(node_a_chan.counterparty_forwarding_info.is_none()); assert_eq!(node_a_chan.holder_htlc_minimum_msat, 1); // the default assert!(node_a_chan.counterparty_forwarding_info().is_none()); @@ -5321,7 +5899,7 @@ mod tests { let counterparty_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let mut config = UserConfig::default(); config.channel_options.announced_channel = false; - let mut chan = Channel::::new_outbound(&&feeest, &&keys_provider, counterparty_node_id, 10_000_000, 100000, 42, &config).unwrap(); // Nothing uses their network key in this test + let mut chan = Channel::::new_outbound(&&feeest, &&keys_provider, counterparty_node_id, &InitFeatures::known(), 10_000_000, 100000, 42, &config).unwrap(); // Nothing uses their network key in this test chan.holder_dust_limit_satoshis = 546; chan.counterparty_selected_channel_reserve_satoshis = Some(0); // Filled in in accept_channel @@ -5365,9 +5943,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)