Fix using the wrong commitment point for revocation

src/ckb/channel.rs

@@ -3243,50 +3243,60 @@ impl ChannelActorState {
     }
 
     fn build_current_commitment_transaction_witnesses(&self, local: bool) -> Vec<u8> {
-        let commitment_number = self.get_current_commitment_number(local);
-        debug!(
-            "Building current commitment transaction witnesses for {} party, commitment number: {}",
-            if local { "local" } else { "remote" },
-            commitment_number
-        );
-        self.build_commitment_transaction_witnesses(local, commitment_number)
-    }
-
-    // We need this function both for building new commitment transaction and revoking old commitment transaction.
-    fn build_commitment_transaction_witnesses(
-        &self,
-        local: bool,
-        commitment_number: u64,
-    ) -> Vec<u8> {
+        let local_commitment_number = self.get_local_commitment_number();
+        let remote_commitment_number = self.get_remote_commitment_number();
+        let commitment_number = if local {
+            local_commitment_number
+        } else {
+            remote_commitment_number
+        };
         debug!(
-            "Building {} commitment transaction witnesses for commitment number {}",
+            "Building {} commitment transaction #{}'s witnesses (commitment numbers: local {}, remote {})",
             if local { "local" } else { "remote" },
-            commitment_number
+            commitment_number,
+            local_commitment_number,
+            remote_commitment_number
         );
         let (delayed_epoch, delayed_payment_key, revocation_key) = {
-            let (delay, commitment_point, base_delayed_payment_key, base_revocation_key) = if local
-            {
+            let (
+                delay,
+                // delayed_payment and revocation keys.
+                // The two fields below are used to derive a pubkey for the delayed payment.
+                // The delayed_payment_commitment_point is held by the broadcaster.
+                delayed_payment_commitment_point,
+                delayed_payment_base_key,
+                // The two fields below are used to derive a pubkey for the revocation.
+                // Unlike delayed_payment_commitment_point above, the revocation key is held
+                // by the counter-signatory until this commitment transaction is revoked.
+                revocation_commitment_point,
+                revocation_base_key,
+            ) = if local {
                 (
                     self.get_local_channel_parameters().selected_contest_delay,
-                    self.get_local_commitment_point(commitment_number),
+                    self.get_local_commitment_point(remote_commitment_number),
                     self.get_local_channel_parameters()
                         .delayed_payment_base_key(),
+                    self.get_remote_commitment_point(local_commitment_number),
                     self.get_local_channel_parameters().revocation_base_key(),
                 )
             } else {
                 (
                     self.get_remote_channel_parameters().selected_contest_delay,
-                    self.get_remote_commitment_point(commitment_number),
+                    self.get_remote_commitment_point(local_commitment_number),
                     self.get_remote_channel_parameters()
                         .delayed_payment_base_key(),
+                    self.get_local_commitment_point(remote_commitment_number),
                     self.get_remote_channel_parameters().revocation_base_key(),
                 )
             };
-            debug!("Got base witness parameters: delayed time: {:?}, delayed_payment_key: {:?}, revocation_key: {:?}", delay, base_delayed_payment_key, base_revocation_key);
+            debug!("Got base witness parameters: delayed time: {:?}, delayed_payment_key: {:?}, revocation_key: {:?}", delay, delayed_payment_base_key, revocation_base_key);
             (
                 delay,
-                derive_delayed_payment_pubkey(base_delayed_payment_key, &commitment_point),
-                derive_revocation_pubkey(base_revocation_key, &commitment_point),
+                derive_delayed_payment_pubkey(
+                    delayed_payment_base_key,
+                    &delayed_payment_commitment_point,
+                ),
+                derive_revocation_pubkey(revocation_base_key, &revocation_commitment_point),
             )
         };