Merge pull request #1047 from TheBlueMatt/2021-08-985-followups
[rust-lightning] / lightning / src / ln / channel.rs
index 03968118a16a59aa481cd148a81309f30cfdcc95..58384f3855e6ceddd7a5a4da646a5e1454e0c45c 100644 (file)
@@ -422,22 +422,18 @@ pub(super) struct Channel<Signer: Sign> {
        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.
+       // Because it follows the same commitment flow as HTLCs, `FeeUpdateState` is either `Outbound`
+       // or matches a subset of the `InboundHTLCOutput` variants. It is then updated/used when
+       // generating new commitment transactions with exactly the same criteria as inbound/outbound
+       // HTLCs with similar state.
        pending_update_fee: Option<(u32, FeeUpdateState)>,
-       // update_fee() during ChannelState::AwaitingRemoteRevoke is hold in
-       // holdina_cell_update_fee then moved to pending_udpate_fee when revoke_and_ack
-       // is received. holding_cell_update_fee is updated when there are additional
-       // update_fee() during ChannelState::AwaitingRemoteRevoke.
+       // If a `send_update_fee()` call is made with ChannelState::AwaitingRemoteRevoke set, we place
+       // it here instead of `pending_update_fee` in the same way as we place outbound HTLC updates in
+       // `holding_cell_htlc_updates` instead of `pending_outbound_htlcs`. It is released into
+       // `pending_update_fee` with the same criteria as outbound HTLC updates but can be updated by
+       // further `send_update_fee` calls, dropping the previous holding cell update entirely.
        holding_cell_update_fee: Option<u32>,
        next_holder_htlc_id: u64,
        next_counterparty_htlc_id: u64,
@@ -567,7 +563,9 @@ 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;
 
@@ -1206,6 +1204,11 @@ impl<Signer: Sign> Channel<Signer> {
 
                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);
@@ -1227,6 +1230,9 @@ impl<Signer: Sign> Channel<Signer> {
                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,
@@ -1791,6 +1797,9 @@ impl<Signer: Sign> Channel<Signer> {
                        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();
@@ -1865,6 +1874,9 @@ impl<Signer: Sign> Channel<Signer> {
                        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();
@@ -2502,6 +2514,8 @@ impl<Signer: Sign> Channel<Signer> {
                );
 
                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...
@@ -2738,8 +2752,10 @@ impl<Signer: Sign> Channel<Signer> {
                        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()));
                        }
                }
@@ -2761,6 +2777,11 @@ impl<Signer: Sign> Channel<Signer> {
                        *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;
@@ -3851,6 +3872,8 @@ impl<Signer: Sign> Channel<Signer> {
                // 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);
                }
@@ -5099,9 +5122,10 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                if self.is_outbound() {
                        self.pending_update_fee.map(|(a, _)| a).write(writer)?;
                } else if let Some((feerate, FeeUpdateState::AwaitingRemoteRevokeToAnnounce)) = self.pending_update_fee {
-                       // As for inbound HTLCs, if the update was only announced and never committed, drop it.
                        Some(feerate).write(writer)?;
                } else {
+                       // As for inbound HTLCs, if the update was only announced and never committed in a
+                       // commitment_signed, drop it.
                        None::<u32>.write(writer)?;
                }
                self.holding_cell_update_fee.write(writer)?;