X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannel.rs;h=97c9fa6789b3c4a2045539281c27b1a28cf1d0b4;hb=cd13364d442faa1bb4edc433112e4000cbaf0d94;hp=8d55804f41d62a8c83825f1dc7b02df8f52a10c9;hpb=2087032e7a823f6c63a64dac951e0f4843bc4667;p=rust-lightning diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 8d55804f..97c9fa67 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -44,6 +44,7 @@ pub struct ChannelValueStat { pub pending_inbound_htlcs_amount_msat: u64, pub holding_cell_outbound_amount_msat: u64, pub their_max_htlc_value_in_flight_msat: u64, // outgoing + pub their_dust_limit_msat: u64, } enum InboundHTLCRemovalReason { @@ -53,19 +54,43 @@ enum InboundHTLCRemovalReason { } enum InboundHTLCState { - /// Added by remote, to be included in next local commitment tx. + /// Offered by remote, to be included in next local commitment tx. I.e., the remote sent an + /// update_add_htlc message for this HTLC. RemoteAnnounced(PendingHTLCStatus), - /// Included in a received commitment_signed message (implying we've revoke_and_ack'ed it), but - /// the remote side hasn't yet revoked their previous state, which we need them to do before we - /// accept this HTLC. Implies AwaitingRemoteRevoke. - /// We also have not yet included this HTLC in a commitment_signed message, and are waiting on - /// a remote revoke_and_ack on a previous state before we can do so. + /// Included in a received commitment_signed message (implying we've + /// revoke_and_ack'd it), but the remote hasn't yet revoked their previous + /// state (see the example below). We have not yet included this HTLC in a + /// commitment_signed message because we are waiting on the remote's + /// aforementioned state revocation. One reason this missing remote RAA + /// (revoke_and_ack) blocks us from constructing a commitment_signed message + /// is because every time we create a new "state", i.e. every time we sign a + /// new commitment tx (see [BOLT #2]), we need a new per_commitment_point, + /// which are provided one-at-a-time in each RAA. E.g., the last RAA they + /// sent provided the per_commitment_point for our current commitment tx. + /// The other reason we should not send a commitment_signed without their RAA + /// is because their RAA serves to ACK our previous commitment_signed. + /// + /// Here's an example of how an HTLC could come to be in this state: + /// remote --> update_add_htlc(prev_htlc) --> local + /// remote --> commitment_signed(prev_htlc) --> local + /// remote <-- revoke_and_ack <-- local + /// remote <-- commitment_signed(prev_htlc) <-- local + /// [note that here, the remote does not respond with a RAA] + /// remote --> update_add_htlc(this_htlc) --> local + /// remote --> commitment_signed(prev_htlc, this_htlc) --> local + /// Now `this_htlc` will be assigned this state. It's unable to be officially + /// accepted, i.e. included in a commitment_signed, because we're missing the + /// RAA that provides our next per_commitment_point. The per_commitment_point + /// is used to derive commitment keys, which are used to construct the + /// signatures in a commitment_signed message. + /// Implies AwaitingRemoteRevoke. + /// [BOLT #2]: https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md AwaitingRemoteRevokeToAnnounce(PendingHTLCStatus), - /// Included in a received commitment_signed message (implying we've revoke_and_ack'ed it), but - /// the remote side hasn't yet revoked their previous state, which we need them to do before we - /// accept this HTLC. Implies AwaitingRemoteRevoke. - /// We have included this HTLC in our latest commitment_signed and are now just waiting on a - /// revoke_and_ack. + /// Included in a received commitment_signed message (implying we've revoke_and_ack'd it). + /// We have also included this HTLC in our latest commitment_signed and are now just waiting + /// on the remote's revoke_and_ack to make this HTLC an irrevocable part of the state of the + /// channel (before it can then get forwarded and/or removed). + /// Implies AwaitingRemoteRevoke. AwaitingAnnouncedRemoteRevoke(PendingHTLCStatus), Committed, /// Removed by us and a new commitment_signed was sent (if we were AwaitingRemoteRevoke when we @@ -159,9 +184,9 @@ enum HTLCUpdateAwaitingACK { /// move on to ShutdownComplete, at which point most calls into this channel are disallowed. enum ChannelState { /// Implies we have (or are prepared to) send our open_channel/accept_channel message - OurInitSent = (1 << 0), + OurInitSent = 1 << 0, /// Implies we have received their open_channel/accept_channel message - TheirInitSent = (1 << 1), + TheirInitSent = 1 << 1, /// We have sent funding_created and are awaiting a funding_signed to advance to FundingSent. /// Note that this is nonsense for an inbound channel as we immediately generate funding_signed /// upon receipt of funding_created, so simply skip this state. @@ -172,35 +197,35 @@ enum ChannelState { FundingSent = 8, /// Flag which can be set on FundingSent to indicate they sent us a funding_locked message. /// Once both TheirFundingLocked and OurFundingLocked are set, state moves on to ChannelFunded. - TheirFundingLocked = (1 << 4), + TheirFundingLocked = 1 << 4, /// Flag which can be set on FundingSent to indicate we sent them a funding_locked message. /// Once both TheirFundingLocked and OurFundingLocked are set, state moves on to ChannelFunded. - OurFundingLocked = (1 << 5), + OurFundingLocked = 1 << 5, ChannelFunded = 64, /// Flag which is set on ChannelFunded and FundingSent indicating remote side is considered /// "disconnected" and no updates are allowed until after we've done a channel_reestablish /// dance. - PeerDisconnected = (1 << 7), + PeerDisconnected = 1 << 7, /// Flag which is set on ChannelFunded, FundingCreated, and FundingSent indicating the user has /// told us they failed to update our ChannelMonitor somewhere and we should pause sending any /// outbound messages until they've managed to do so. - MonitorUpdateFailed = (1 << 8), + MonitorUpdateFailed = 1 << 8, /// Flag which implies that we have sent a commitment_signed but are awaiting the responding /// revoke_and_ack message. During this time period, we can't generate new commitment_signed /// messages as then we will be unable to determine which HTLCs they included in their /// revoke_and_ack implicit ACK, so instead we have to hold them away temporarily to be sent /// later. /// Flag is set on ChannelFunded. - AwaitingRemoteRevoke = (1 << 9), + AwaitingRemoteRevoke = 1 << 9, /// Flag which is set on ChannelFunded or FundingSent after receiving a shutdown message from /// the remote end. If set, they may not add any new HTLCs to the channel, and we are expected /// to respond with our own shutdown message when possible. - RemoteShutdownSent = (1 << 10), + 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), + 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. ShutdownComplete = 4096, @@ -285,16 +310,16 @@ pub(super) struct Channel { // 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, + 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. - holding_cell_update_fee: Option, + holding_cell_update_fee: Option, next_local_htlc_id: u64, next_remote_htlc_id: u64, update_time_counter: u32, - feerate_per_kw: u64, + feerate_per_kw: u32, #[cfg(debug_assertions)] /// Max to_local and to_remote outputs in a locally-generated commitment transaction @@ -303,7 +328,7 @@ pub(super) struct Channel { /// Max to_local and to_remote outputs in a remote-generated commitment transaction max_commitment_tx_output_remote: ::std::sync::Mutex<(u64, u64)>, - last_sent_closing_fee: Option<(u64, u64, Signature)>, // (feerate, fee, our_sig) + last_sent_closing_fee: Option<(u32, u64, Signature)>, // (feerate, fee, our_sig) funding_txo: Option, @@ -423,8 +448,8 @@ impl Channel { cmp::min(channel_value_satoshis, cmp::max(q, 1000)) //TODO } - fn derive_our_dust_limit_satoshis(at_open_background_feerate: u64) -> u64 { - cmp::max(at_open_background_feerate * B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT / 1000, 546) //TODO + fn derive_our_dust_limit_satoshis(at_open_background_feerate: u32) -> u64 { + cmp::max(at_open_background_feerate as u64 * B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT / 1000, 546) //TODO } // Constructors: @@ -533,10 +558,10 @@ impl Channel { fn check_remote_fee(fee_estimator: &F, feerate_per_kw: u32) -> Result<(), ChannelError> where F::Target: FeeEstimator { - if (feerate_per_kw as u64) < fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) { + if feerate_per_kw < fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) { return Err(ChannelError::Close("Peer's feerate much too low")); } - if (feerate_per_kw as u64) > fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) * 2 { + if feerate_per_kw as u64 > fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64 * 2 { return Err(ChannelError::Close("Peer's feerate much too high")); } Ok(()) @@ -645,12 +670,12 @@ impl Channel { // check if the funder's amount for the initial commitment tx is sufficient // for full fee payment let funders_amount_msat = msg.funding_satoshis * 1000 - msg.push_msat; - if funders_amount_msat < background_feerate * COMMITMENT_TX_BASE_WEIGHT { + if funders_amount_msat < background_feerate as u64 * COMMITMENT_TX_BASE_WEIGHT { return Err(ChannelError::Close("Insufficient funding amount for initial commitment")); } let to_local_msat = msg.push_msat; - let to_remote_msat = funders_amount_msat - background_feerate * COMMITMENT_TX_BASE_WEIGHT; + let to_remote_msat = funders_amount_msat - background_feerate as u64 * COMMITMENT_TX_BASE_WEIGHT; if to_local_msat <= msg.channel_reserve_satoshis * 1000 && to_remote_msat <= remote_channel_reserve_satoshis * 1000 { return Err(ChannelError::Close("Insufficient funding amount for initial commitment")); } @@ -725,7 +750,7 @@ impl Channel { last_block_connected: Default::default(), funding_tx_confirmations: 0, - feerate_per_kw: msg.feerate_per_kw as u64, + feerate_per_kw: msg.feerate_per_kw, channel_value_satoshis: msg.funding_satoshis, their_dust_limit_satoshis: msg.dust_limit_satoshis, our_dust_limit_satoshis: our_dust_limit_satoshis, @@ -766,15 +791,14 @@ impl Channel { fn get_commitment_transaction_number_obscure_factor(&self) -> u64 { let mut sha = Sha256::engine(); - let our_payment_point = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()); let their_payment_point = &self.their_pubkeys.as_ref().unwrap().payment_point.serialize(); if self.channel_outbound { - sha.input(&our_payment_point.serialize()); + sha.input(&self.local_keys.pubkeys().payment_point.serialize()); sha.input(their_payment_point); } else { sha.input(their_payment_point); - sha.input(&our_payment_point.serialize()); + sha.input(&self.local_keys.pubkeys().payment_point.serialize()); } let res = Sha256::from_engine(sha).into_inner(); @@ -804,7 +828,7 @@ impl Channel { /// 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. #[inline] - fn build_commitment_transaction(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, feerate_per_kw: u64, logger: &L) -> (Transaction, 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, feerate_per_kw: u32, logger: &L) -> (Transaction, usize, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>) where L::Target: Logger { let obscured_commitment_transaction_number = self.get_commitment_transaction_number_obscure_factor() ^ (INITIAL_COMMITMENT_NUMBER - commitment_number); let txins = { @@ -844,7 +868,7 @@ impl Channel { ($htlc: expr, $outbound: expr, $source: expr, $state_name: expr) => { if $outbound == local { // "offered HTLC output" let htlc_in_tx = get_htlc_in_commitment!($htlc, true); - if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw * HTLC_TIMEOUT_TX_WEIGHT / 1000) { + if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000) { log_trace!(logger, " ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat); txouts.push((TxOut { script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(), @@ -856,7 +880,7 @@ impl Channel { } } else { let htlc_in_tx = get_htlc_in_commitment!($htlc, false); - if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw * HTLC_SUCCESS_TX_WEIGHT / 1000) { + if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000) { log_trace!(logger, " ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat); txouts.push((TxOut { // "received HTLC output" script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(), @@ -949,7 +973,7 @@ impl Channel { max_commitment_tx_output.1 = cmp::max(max_commitment_tx_output.1, value_to_remote_msat as u64); } - let total_fee: u64 = feerate_per_kw * (COMMITMENT_TX_BASE_WEIGHT + (txouts.len() as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; + let total_fee = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (txouts.len() as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; let (value_to_self, value_to_remote) = if self.channel_outbound { (value_to_self_msat / 1000 - total_fee as i64, value_to_remote_msat / 1000) } else { @@ -1095,11 +1119,11 @@ impl Channel { /// TODO Some magic rust shit to compile-time check this? fn build_local_transaction_keys(&self, commitment_number: u64) -> Result { let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number)); - let delayed_payment_base = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()); - let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()); + let delayed_payment_base = &self.local_keys.pubkeys().delayed_payment_basepoint; + let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint; let their_pubkeys = self.their_pubkeys.as_ref().unwrap(); - Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys")) + Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys")) } #[inline] @@ -1109,25 +1133,24 @@ impl Channel { fn build_remote_transaction_keys(&self) -> Result { //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we //may see payments to it! - let revocation_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()); - let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()); + let revocation_basepoint = &self.local_keys.pubkeys().revocation_basepoint; + let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint; let their_pubkeys = self.their_pubkeys.as_ref().unwrap(); - Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, &revocation_basepoint, &htlc_basepoint), "Remote tx keys generation got bogus keys")) + Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, revocation_basepoint, htlc_basepoint), "Remote tx keys generation got bogus keys")) } /// Gets the redeemscript for the funding transaction output (ie the funding transaction output /// pays to get_funding_redeemscript().to_v0_p2wsh()). /// Panics if called before accept_channel/new_from_req pub fn get_funding_redeemscript(&self) -> Script { - let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()); - make_funding_redeemscript(&our_funding_key, self.their_funding_pubkey()) + make_funding_redeemscript(&self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey()) } /// Builds the htlc-success or htlc-timeout transaction which spends a given HTLC output /// @local is used only to convert relevant internal structures which refer to remote vs local /// to decide value of outputs and direction of HTLCs. - fn build_htlc_transaction(&self, prev_hash: &Txid, htlc: &HTLCOutputInCommitment, local: bool, keys: &TxCreationKeys, feerate_per_kw: u64) -> Transaction { + fn build_htlc_transaction(&self, prev_hash: &Txid, htlc: &HTLCOutputInCommitment, local: bool, keys: &TxCreationKeys, feerate_per_kw: u32) -> Transaction { chan_utils::build_htlc_transaction(prev_hash, feerate_per_kw, if local { self.their_to_self_delay } else { self.our_to_self_delay }, htlc, &keys.a_delayed_payment_key, &keys.revocation_key) } @@ -1455,7 +1478,7 @@ impl Channel { log_trace!(logger, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey().serialize()), encode::serialize_hex(&local_initial_commitment_tx), log_bytes!(local_sighash[..]), encode::serialize_hex(&funding_script)); secp_check!(self.secp_ctx.verify(&local_sighash, &sig, self.their_funding_pubkey()), "Invalid funding_created signature from peer"); - let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new()); + let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new()); let remote_keys = self.build_remote_transaction_keys()?; let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw, logger).0; @@ -1568,7 +1591,7 @@ impl Channel { let funding_txo_script = funding_redeemscript.to_v0_p2wsh(); macro_rules! create_monitor { () => { { - let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new()); + let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new()); let mut channel_monitor = ChannelMonitor::new(self.local_keys.clone(), &self.shutdown_pubkey, self.our_to_self_delay, &self.destination_script, (funding_txo.clone(), funding_txo_script.clone()), @@ -1665,8 +1688,83 @@ impl Channel { cmp::min(self.value_to_self_msat as i64 - self.get_outbound_pending_htlc_stats().1 as i64, 0) as u64) } - pub fn update_add_htlc(&mut self, msg: &msgs::UpdateAddHTLC, pending_forward_state: PendingHTLCStatus) -> Result<(), ChannelError> { - if (self.channel_state & (ChannelState::ChannelFunded as u32 | ChannelState::RemoteShutdownSent as u32)) != (ChannelState::ChannelFunded as u32) { + // Get the fee cost of a commitment tx with a given number of HTLC outputs. + // Note that num_htlcs should not include dust HTLCs. + fn commit_tx_fee_msat(&self, num_htlcs: usize) -> u64 { + // Note that we need to divide before multiplying to round properly, + // since the lowest denomination of bitcoin on-chain is the satoshi. + (COMMITMENT_TX_BASE_WEIGHT + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * self.feerate_per_kw as u64 / 1000 * 1000 + } + + // Get the commitment tx fee for the local (i.e our) next commitment transaction + // based on the number of pending HTLCs that are on track to be in our next + // commitment tx. `addl_htcs` is an optional parameter allowing the caller + // to add a number of additional HTLCs to the calculation. Note that dust + // HTLCs are excluded. + fn next_local_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 { + assert!(self.channel_outbound); + + let mut their_acked_htlcs = self.pending_inbound_htlcs.len(); + for ref htlc in self.pending_outbound_htlcs.iter() { + if htlc.amount_msat / 1000 <= self.our_dust_limit_satoshis { + continue + } + match htlc.state { + OutboundHTLCState::Committed => their_acked_htlcs += 1, + OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1, + OutboundHTLCState::LocalAnnounced {..} => their_acked_htlcs += 1, + _ => {}, + } + } + + for htlc in self.holding_cell_htlc_updates.iter() { + match htlc { + &HTLCUpdateAwaitingACK::AddHTLC { .. } => their_acked_htlcs += 1, + _ => {}, + } + } + + self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs) + } + + // Get the commitment tx fee for the remote's next commitment transaction + // based on the number of pending HTLCs that are on track to be in their + // next commitment tx. `addl_htcs` is an optional parameter allowing the caller + // to add a number of additional HTLCs to the calculation. Note that dust HTLCs + // are excluded. + fn next_remote_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 { + assert!(!self.channel_outbound); + + // When calculating the set of HTLCs which will be included in their next + // commitment_signed, all inbound HTLCs are included (as all states imply it will be + // included) and only committed outbound HTLCs, see below. + let mut their_acked_htlcs = self.pending_inbound_htlcs.len(); + for ref htlc in self.pending_outbound_htlcs.iter() { + if htlc.amount_msat / 1000 <= self.their_dust_limit_satoshis { + continue + } + // We only include outbound HTLCs if it will not be included in their next + // commitment_signed, i.e. if they've responded to us with an RAA after announcement. + match htlc.state { + OutboundHTLCState::Committed => their_acked_htlcs += 1, + OutboundHTLCState::RemoteRemoved {..} => their_acked_htlcs += 1, + _ => {}, + } + } + + self.commit_tx_fee_msat(their_acked_htlcs + addl_htlcs) + } + + pub fn update_add_htlc(&mut self, msg: &msgs::UpdateAddHTLC, mut pending_forward_status: PendingHTLCStatus, create_pending_htlc_status: F, logger: &L) -> Result<(), ChannelError> + where F: for<'a> Fn(&'a Self, PendingHTLCStatus, u16) -> PendingHTLCStatus, L::Target: Logger { + // 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); + } + // 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); + if remote_sent_shutdown { return Err(ChannelError::Close("Got add HTLC message when channel was not in an operational state")); } if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 { @@ -1710,9 +1808,56 @@ impl Channel { removed_outbound_total_msat += htlc.amount_msat; } } - if htlc_inbound_value_msat + msg.amount_msat + self.value_to_self_msat > (self.channel_value_satoshis - Channel::::get_remote_channel_reserve_satoshis(self.channel_value_satoshis)) * 1000 + removed_outbound_total_msat { - return Err(ChannelError::Close("Remote HTLC add would put them under their reserve value")); + + let pending_value_to_self_msat = + self.value_to_self_msat + htlc_inbound_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 { + return Err(ChannelError::Close("Remote HTLC add would overdraw remaining funds")); + } + + // Check that the remote can afford to pay for this HTLC on-chain at the current + // feerate_per_kw, while maintaining their channel reserve (as required by the spec). + let remote_commit_tx_fee_msat = if self.channel_outbound { 0 } else { + // +1 for this HTLC. + self.next_remote_commit_tx_fee_msat(1) + }; + if pending_remote_value_msat - msg.amount_msat < remote_commit_tx_fee_msat { + return Err(ChannelError::Close("Remote HTLC add would not leave enough to pay for fees")); + }; + + let chan_reserve_msat = + Channel::::get_remote_channel_reserve_satoshis(self.channel_value_satoshis) * 1000; + if pending_remote_value_msat - msg.amount_msat - remote_commit_tx_fee_msat < chan_reserve_msat { + return Err(ChannelError::Close("Remote HTLC add would put them under remote reserve value")); + } + + if !self.channel_outbound { + // `+1` for this HTLC, `2 *` and `+1` fee spike buffer we keep for the remote. This deviates from the + // spec because in the spec, the fee spike buffer requirement doesn't exist on the receiver's side, + // only on the sender's. + // Note that when we eventually remove support for fee updates and switch to anchor output fees, + // we will drop the `2 *`, since we no longer be as sensitive to fee spikes. But, keep the extra +1 + // as we should still be able to afford adding this HTLC plus one more future HTLC, regardless of + // being sensitive to fee spikes. + let remote_fee_cost_incl_stuck_buffer_msat = 2 * self.next_remote_commit_tx_fee_msat(1 + 1); + if pending_remote_value_msat - msg.amount_msat - chan_reserve_msat < remote_fee_cost_incl_stuck_buffer_msat { + // Note that if the pending_forward_status is not updated here, then it's because we're already failing + // the HTLC, i.e. its status is already set to failing. + log_info!(logger, "Attempting to fail HTLC due to fee spike buffer violation"); + pending_forward_status = create_pending_htlc_status(self, pending_forward_status, 0x1000|7); + } + } else { + // Check that they won't violate our local required channel reserve by adding this HTLC. + + // +1 for this HTLC. + let local_commit_tx_fee_msat = self.next_local_commit_tx_fee_msat(1); + if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + local_commit_tx_fee_msat { + return Err(ChannelError::Close("Cannot receive value that would put us under local channel reserve value")); + } } + if self.next_remote_htlc_id != msg.htlc_id { return Err(ChannelError::Close("Remote skipped HTLC ID")); } @@ -1721,7 +1866,7 @@ impl Channel { } if self.channel_state & ChannelState::LocalShutdownSent as u32 != 0 { - if let PendingHTLCStatus::Forward(_) = pending_forward_state { + if let PendingHTLCStatus::Forward(_) = pending_forward_status { panic!("ChannelManager shouldn't be trying to add a forwardable HTLC after we've started closing"); } } @@ -1733,7 +1878,7 @@ impl Channel { amount_msat: msg.amount_msat, payment_hash: msg.payment_hash, cltv_expiry: msg.cltv_expiry, - state: InboundHTLCState::RemoteAnnounced(pending_forward_state), + state: InboundHTLCState::RemoteAnnounced(pending_forward_status), }); Ok(()) } @@ -1842,7 +1987,7 @@ impl Channel { //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 num_htlcs = local_commitment_tx.1; - let total_fee: u64 = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; + let total_fee = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; let remote_reserve_we_require = Channel::::get_remote_channel_reserve_satoshis(self.channel_value_satoshis); if self.channel_value_satoshis - self.value_to_self_msat / 1000 < total_fee + remote_reserve_we_require { @@ -1899,7 +2044,7 @@ impl Channel { let mut monitor_update = ChannelMonitorUpdate { update_id: self.latest_monitor_update_id, updates: vec![ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo { - commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source), + commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source), htlc_outputs: htlcs_and_sigs }] }; @@ -2322,7 +2467,7 @@ impl Channel { /// Adds a pending update to this channel. See the doc for send_htlc for /// further details on the optionness of the return value. /// You MUST call send_commitment prior to any other calls on this Channel - fn send_update_fee(&mut self, feerate_per_kw: u64) -> Option { + fn send_update_fee(&mut self, feerate_per_kw: u32) -> Option { if !self.channel_outbound { panic!("Cannot send fee from inbound channel"); } @@ -2343,11 +2488,11 @@ impl Channel { Some(msgs::UpdateFee { channel_id: self.channel_id, - feerate_per_kw: feerate_per_kw as u32, + feerate_per_kw: feerate_per_kw, }) } - pub fn send_update_fee_and_commit(&mut self, feerate_per_kw: u64, logger: &L) -> Result, ChannelError> where L::Target: Logger { + pub fn send_update_fee_and_commit(&mut self, feerate_per_kw: u32, logger: &L) -> Result, ChannelError> where L::Target: Logger { match self.send_update_fee(feerate_per_kw) { Some(update_fee) => { let (commitment_signed, monitor_update) = self.send_commitment_no_status_check(logger)?; @@ -2508,7 +2653,7 @@ impl Channel { return Err(ChannelError::Close("Peer sent update_fee when we needed a channel_reestablish")); } Channel::::check_remote_fee(fee_estimator, msg.feerate_per_kw)?; - self.pending_update_fee = Some(msg.feerate_per_kw as u64); + self.pending_update_fee = Some(msg.feerate_per_kw); self.update_time_counter += 1; Ok(()) } @@ -2726,7 +2871,7 @@ impl Channel { proposed_feerate = self.feerate_per_kw; } let tx_weight = Self::get_closing_transaction_weight(&self.get_closing_scriptpubkey(), self.their_shutdown_scriptpubkey.as_ref().unwrap()); - let proposed_total_fee_satoshis = proposed_feerate * tx_weight / 1000; + let proposed_total_fee_satoshis = proposed_feerate as u64 * tx_weight / 1000; let (closing_tx, total_fee_satoshis) = self.build_closing_transaction(proposed_total_fee_satoshis, false); let our_sig = self.local_keys @@ -2825,7 +2970,7 @@ impl Channel { tx.input[0].witness.push(Vec::new()); // First is the multisig dummy - let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()).serialize(); + let our_funding_key = self.local_keys.pubkeys().funding_pubkey.serialize(); let their_funding_key = self.their_funding_pubkey().serialize(); if our_funding_key[..] < their_funding_key[..] { tx.input[0].witness.push(our_sig.serialize_der().to_vec()); @@ -2888,7 +3033,7 @@ impl Channel { macro_rules! propose_new_feerate { ($new_feerate: expr) => { let closing_tx_max_weight = Self::get_closing_transaction_weight(&self.get_closing_scriptpubkey(), self.their_shutdown_scriptpubkey.as_ref().unwrap()); - let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate * closing_tx_max_weight / 1000, false); + let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate as u64 * closing_tx_max_weight / 1000, false); let our_sig = self.local_keys .sign_closing_transaction(&closing_tx, &self.secp_ctx) .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction"))?; @@ -2901,10 +3046,10 @@ impl Channel { } } - let proposed_sat_per_kw = msg.fee_satoshis * 1000 / closing_tx.get_weight() as u64; + let proposed_sat_per_kw = msg.fee_satoshis * 1000 / closing_tx.get_weight() as u64; if self.channel_outbound { let our_max_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal); - if proposed_sat_per_kw > our_max_feerate { + if (proposed_sat_per_kw as u32) > our_max_feerate { if let Some((last_feerate, _, _)) = self.last_sent_closing_fee { if our_max_feerate <= last_feerate { return Err(ChannelError::Close("Unable to come to consensus about closing feerate, remote wanted something higher than our Normal feerate")); @@ -2914,7 +3059,7 @@ impl Channel { } } else { let our_min_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background); - if proposed_sat_per_kw < our_min_feerate { + if (proposed_sat_per_kw as u32) < our_min_feerate { if let Some((last_feerate, _, _)) = self.last_sent_closing_fee { if our_min_feerate >= last_feerate { return Err(ChannelError::Close("Unable to come to consensus about closing feerate, remote wanted something lower than our Background feerate")); @@ -2996,7 +3141,7 @@ impl Channel { } #[cfg(test)] - pub fn get_feerate(&self) -> u64 { + pub fn get_feerate(&self) -> u32 { self.feerate_per_kw } @@ -3038,6 +3183,7 @@ impl Channel { res }, their_max_htlc_value_in_flight_msat: self.their_max_htlc_value_in_flight_msat, + their_dust_limit_msat: self.their_dust_limit_satoshis * 1000, } } @@ -3067,15 +3213,15 @@ impl Channel { // 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 * cmp::max(HTLC_TIMEOUT_TX_WEIGHT, HTLC_SUCCESS_TX_WEIGHT) / 1000; + let mut res = self.feerate_per_kw as u64 * cmp::max(HTLC_TIMEOUT_TX_WEIGHT, HTLC_SUCCESS_TX_WEIGHT) / 1000; if self.channel_outbound { // + the marginal fee increase cost to us in the commitment transaction: - res += self.feerate_per_kw * COMMITMENT_TX_WEIGHT_PER_HTLC / 1000; + 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) * SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT / 1000; + res += fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal) as u64 * SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT / 1000; res as u32 } @@ -3154,7 +3300,7 @@ impl Channel { /// /// May return some HTLCs (and their payment_hash) which have timed out and should be failed /// back. - pub fn block_connected(&mut self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) -> Result<(Option, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> { + pub fn block_connected(&mut self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[usize]) -> Result<(Option, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> { let mut timed_out_htlcs = Vec::new(); self.holding_cell_htlc_updates.retain(|htlc_update| { match htlc_update { @@ -3205,6 +3351,10 @@ impl Channel { } } } + if height > 0xff_ff_ff || (*index_in_block) > 0xff_ff_ff { + panic!("Block was bogus - either height 16 million or had > 16 million transactions"); + } + assert!(txo_idx <= 0xffff); // txo_idx is a (u16 as usize), so this is just listed here for completeness self.funding_tx_confirmations = 1; self.short_channel_id = Some(((height as u64) << (5*8)) | ((*index_in_block as u64) << (2*8)) | @@ -3287,9 +3437,7 @@ impl Channel { // Methods to get unprompted messages to send to the remote end (or where we already returned // something in the handler for the message that prompted this message): - pub fn get_open_channel(&self, chain_hash: BlockHash, fee_estimator: &F) -> msgs::OpenChannel - where F::Target: FeeEstimator - { + pub fn get_open_channel(&self, chain_hash: BlockHash) -> msgs::OpenChannel { if !self.channel_outbound { panic!("Tried to open a channel for an inbound channel?"); } @@ -3302,6 +3450,7 @@ impl Channel { } let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number); + let local_keys = self.local_keys.pubkeys(); msgs::OpenChannel { chain_hash: chain_hash, @@ -3312,14 +3461,14 @@ impl Channel { max_htlc_value_in_flight_msat: Channel::::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis), channel_reserve_satoshis: Channel::::get_remote_channel_reserve_satoshis(self.channel_value_satoshis), htlc_minimum_msat: self.our_htlc_minimum_msat, - feerate_per_kw: fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) as u32, + feerate_per_kw: self.feerate_per_kw as u32, to_self_delay: self.our_to_self_delay, max_accepted_htlcs: OUR_MAX_HTLCS, - funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), - revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()), - payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()), - delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()), - htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()), + funding_pubkey: local_keys.funding_pubkey, + revocation_basepoint: local_keys.revocation_basepoint, + payment_point: local_keys.payment_point, + delayed_payment_basepoint: local_keys.delayed_payment_basepoint, + htlc_basepoint: local_keys.htlc_basepoint, first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret), 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() }) @@ -3338,6 +3487,7 @@ impl Channel { } let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number); + let local_keys = self.local_keys.pubkeys(); msgs::AcceptChannel { temporary_channel_id: self.channel_id, @@ -3348,11 +3498,11 @@ impl Channel { minimum_depth: self.minimum_depth, to_self_delay: self.our_to_self_delay, max_accepted_htlcs: OUR_MAX_HTLCS, - funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), - revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()), - payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()), - delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()), - htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()), + funding_pubkey: local_keys.funding_pubkey, + revocation_basepoint: local_keys.revocation_basepoint, + payment_point: local_keys.payment_point, + delayed_payment_basepoint: local_keys.delayed_payment_basepoint, + htlc_basepoint: local_keys.htlc_basepoint, first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret), shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() }) } @@ -3431,7 +3581,6 @@ impl Channel { } let were_node_one = our_node_id.serialize()[..] < self.their_node_id.serialize()[..]; - let our_bitcoin_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()); let msg = msgs::UnsignedChannelAnnouncement { features: ChannelFeatures::known(), @@ -3439,8 +3588,8 @@ impl Channel { short_channel_id: self.get_short_channel_id().unwrap(), node_id_1: if were_node_one { our_node_id } else { self.get_their_node_id() }, node_id_2: if were_node_one { self.get_their_node_id() } else { our_node_id }, - bitcoin_key_1: if were_node_one { our_bitcoin_key } else { self.their_funding_pubkey().clone() }, - bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { our_bitcoin_key }, + bitcoin_key_1: if were_node_one { self.local_keys.pubkeys().funding_pubkey } else { self.their_funding_pubkey().clone() }, + bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { self.local_keys.pubkeys().funding_pubkey }, excess_data: Vec::new(), }; @@ -3547,29 +3696,56 @@ impl Channel { return Err(ChannelError::Ignore("Cannot send value that would put us over the max HTLC value in flight our peer will accept")); } + if !self.channel_outbound { + // Check that we won't violate the remote channel reserve by adding this HTLC. + + let remote_balance_msat = self.channel_value_satoshis * 1000 - self.value_to_self_msat; + let remote_chan_reserve_msat = Channel::::get_remote_channel_reserve_satoshis(self.channel_value_satoshis); + // 1 additional HTLC corresponding to this HTLC. + let remote_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(1); + if remote_balance_msat < remote_chan_reserve_msat + remote_commit_tx_fee_msat { + return Err(ChannelError::Ignore("Cannot send value that would put them under remote channel reserve value")); + } + } + + let pending_value_to_self_msat = self.value_to_self_msat - htlc_outbound_value_msat; + if pending_value_to_self_msat < amount_msat { + return Err(ChannelError::Ignore("Cannot send value that would overdraw remaining funds")); + } + + // The `+1` is for the HTLC currently being added to the commitment tx and + // the `2 *` and `+1` are for the fee spike buffer. + let local_commit_tx_fee_msat = if self.channel_outbound { + 2 * self.next_local_commit_tx_fee_msat(1 + 1) + } else { 0 }; + if pending_value_to_self_msat - amount_msat < local_commit_tx_fee_msat { + return Err(ChannelError::Ignore("Cannot send value that would not leave enough to pay for fees")); + } + // Check self.local_channel_reserve_satoshis (the amount we must keep as // reserve for the remote to have something to claim if we misbehave) - if self.value_to_self_msat < self.local_channel_reserve_satoshis * 1000 + amount_msat + htlc_outbound_value_msat { + let chan_reserve_msat = self.local_channel_reserve_satoshis * 1000; + if pending_value_to_self_msat - amount_msat - local_commit_tx_fee_msat < chan_reserve_msat { return Err(ChannelError::Ignore("Cannot send value that would put us under local channel reserve value")); } // Now update local state: if (self.channel_state & (ChannelState::AwaitingRemoteRevoke as u32)) == (ChannelState::AwaitingRemoteRevoke as u32) { self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::AddHTLC { - amount_msat: amount_msat, - payment_hash: payment_hash, - cltv_expiry: cltv_expiry, + amount_msat, + payment_hash, + cltv_expiry, source, - onion_routing_packet: onion_routing_packet, + onion_routing_packet, }); return Ok(None); } self.pending_outbound_htlcs.push(OutboundHTLCOutput { htlc_id: self.next_local_htlc_id, - amount_msat: amount_msat, + amount_msat, payment_hash: payment_hash.clone(), - cltv_expiry: cltv_expiry, + cltv_expiry, state: OutboundHTLCState::LocalAnnounced(Box::new(onion_routing_packet.clone())), source, }); @@ -3577,10 +3753,10 @@ impl Channel { let res = msgs::UpdateAddHTLC { channel_id: self.channel_id, htlc_id: self.next_local_htlc_id, - amount_msat: amount_msat, - payment_hash: payment_hash, - cltv_expiry: cltv_expiry, - onion_routing_packet: onion_routing_packet, + amount_msat, + payment_hash, + cltv_expiry, + onion_routing_packet, }; self.next_local_htlc_id += 1; @@ -4307,13 +4483,12 @@ mod tests { use bitcoin::hashes::Hash; use bitcoin::hash_types::{Txid, WPubkeyHash}; use std::sync::Arc; - use rand::{thread_rng,Rng}; struct TestFeeEstimator { - fee_est: u64 + fee_est: u32 } impl FeeEstimator for TestFeeEstimator { - fn get_est_sat_per_1000_weight(&self, _: ConfirmationTarget) -> u64 { + fn get_est_sat_per_1000_weight(&self, _: ConfirmationTarget) -> u32 { self.fee_est } } @@ -4355,14 +4530,34 @@ mod tests { PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&hex::decode(hex).unwrap()[..]).unwrap()) } + // 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] + fn test_open_channel_msg_fee() { + let original_fee = 253; + let mut fee_est = TestFeeEstimator{fee_est: original_fee }; + let secp_ctx = Secp256k1::new(); + let seed = [42; 32]; + let network = Network::Testnet; + let keys_provider = test_utils::TestKeysInterface::new(&seed, network); + + 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(); + + // Now change the fee so we can check that the fee in the open_channel message is the + // same as the old fee. + fee_est.fee_est = 500; + let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.bitcoin_hash()); + assert_eq!(open_channel_msg.feerate_per_kw, original_fee); + } + #[test] fn channel_reestablish_no_updates() { let feeest = TestFeeEstimator{fee_est: 15000}; let logger = test_utils::TestLogger::new(); let secp_ctx = Secp256k1::new(); - let mut seed = [0; 32]; - let mut rng = thread_rng(); - rng.fill_bytes(&mut seed); + let seed = [42; 32]; let network = Network::Testnet; let keys_provider = test_utils::TestKeysInterface::new(&seed, network); @@ -4374,7 +4569,7 @@ mod tests { let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_a_node_id, 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(genesis_block(network).header.bitcoin_hash(), &&feeest); + let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.bitcoin_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(); @@ -4442,7 +4637,7 @@ mod tests { (0, 0) ); - assert_eq!(PublicKey::from_secret_key(&secp_ctx, chan_keys.funding_key()).serialize()[..], + assert_eq!(chan_keys.pubkeys().funding_pubkey.serialize()[..], hex::decode("023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb").unwrap()[..]); let keys_provider = Keys { chan_keys: chan_keys.clone() }; @@ -4477,11 +4672,11 @@ mod tests { // We can't just use build_local_transaction_keys here as the per_commitment_secret is not // derived from a commitment_seed, so instead we copy it here and call // build_commitment_transaction. - let delayed_payment_base = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.delayed_payment_base_key()); + let delayed_payment_base = &chan.local_keys.pubkeys().delayed_payment_basepoint; let per_commitment_secret = SecretKey::from_slice(&hex::decode("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap(); let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret); - let htlc_basepoint = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.htlc_base_key()); - let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap(); + let htlc_basepoint = &chan.local_keys.pubkeys().htlc_basepoint; + let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap(); chan.their_pubkeys = Some(their_pubkeys); @@ -4512,7 +4707,7 @@ mod tests { })* assert_eq!(unsigned_tx.1.len(), per_htlc.len()); - localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc); + localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &chan_keys.pubkeys().funding_pubkey, chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc); let local_sig = chan_keys.sign_local_commitment(&localtx, &chan.secp_ctx).unwrap(); assert_eq!(Signature::from_der(&hex::decode($our_sig_hex).unwrap()[..]).unwrap(), local_sig);