Don't borsh UnsignedSoftConfirmation to compute its hash (#1434)

crates/sequencer/src/runner.rs

@@ -13,7 +13,6 @@ use citrea_evm::{CallMessage, Evm, RlpEvmTransaction, MIN_TRANSACTION_GAS};
 use citrea_primitives::basefee::calculate_next_block_base_fee;
 use citrea_primitives::types::SoftConfirmationHash;
 use citrea_stf::runtime::Runtime;
-use digest::Digest;
 use futures::channel::mpsc::{unbounded, UnboundedReceiver, UnboundedSender};
 use futures::StreamExt;
 use jsonrpsee::server::{BatchRequestConfig, RpcServiceBuilder, ServerBuilder};
@@ -470,8 +469,13 @@ where
                     timestamp,
                 );
 
-                let mut signed_soft_confirmation =
-                    self.sign_soft_confirmation_batch(&unsigned_batch, self.batch_hash)?;
+                let mut signed_soft_confirmation = if active_fork_spec
+                    >= sov_modules_api::SpecId::Fork1
+                {
+                    self.sign_soft_confirmation_batch(&unsigned_batch, self.batch_hash)?
+                } else {
+                    self.pre_fork1_sign_soft_confirmation_batch(&unsigned_batch, self.batch_hash)?
+                };
 
                 let (soft_confirmation_receipt, checkpoint) = self.stf.end_soft_confirmation(
                     active_fork_spec,
@@ -782,8 +786,38 @@ where
         soft_confirmation: &'txs UnsignedSoftConfirmation<'_>,
         prev_soft_confirmation_hash: [u8; 32],
     ) -> anyhow::Result<SignedSoftConfirmation<'txs>> {
-        let raw = borsh::to_vec(&soft_confirmation).map_err(|e| anyhow!(e))?;
+        let digest = soft_confirmation.compute_digest::<<C as sov_modules_api::Spec>::Hasher>();
+        let hash = Into::<[u8; 32]>::into(digest);
+
+        let signature = self.sov_tx_signer_priv_key.sign(&hash);
+        let pub_key = self.sov_tx_signer_priv_key.pub_key();
+        Ok(SignedSoftConfirmation::new(
+            soft_confirmation.l2_height(),
+            hash,
+            prev_soft_confirmation_hash,
+            soft_confirmation.da_slot_height(),
+            soft_confirmation.da_slot_hash(),
+            soft_confirmation.da_slot_txs_commitment(),
+            soft_confirmation.l1_fee_rate(),
+            soft_confirmation.txs().into(),
+            soft_confirmation.deposit_data(),
+            borsh::to_vec(&signature).map_err(|e| anyhow!(e))?,
+            borsh::to_vec(&pub_key).map_err(|e| anyhow!(e))?,
+            soft_confirmation.timestamp(),
+        ))
+    }
 
+    /// Old version of sign_soft_confirmation_batch
+    /// TODO: Remove derive(BorshSerialize) for UnsignedSoftConfirmation
+    ///   when removing this fn
+    /// FIXME: ^
+    fn pre_fork1_sign_soft_confirmation_batch<'txs>(
+        &mut self,
+        soft_confirmation: &'txs UnsignedSoftConfirmation<'_>,
+        prev_soft_confirmation_hash: [u8; 32],
+    ) -> anyhow::Result<SignedSoftConfirmation<'txs>> {
+        use digest::Digest;
+        let raw = borsh::to_vec(&soft_confirmation).map_err(|e| anyhow!(e))?;
         let hash = <C as sov_modules_api::Spec>::Hasher::digest(raw.as_slice()).into();
 
         let signature = self.sov_tx_signer_priv_key.sign(&raw);

crates/sovereign-sdk/module-system/sov-modules-stf-blueprint/src/lib.rs

@@ -15,7 +15,6 @@ use sov_modules_api::{
     Genesis, Signature, Spec, StateCheckpoint, UnsignedSoftConfirmation, WorkingSet, Zkvm,
 };
 use sov_rollup_interface::da::{DaDataBatchProof, SequencerCommitment};
-use sov_rollup_interface::digest::Digest;
 use sov_rollup_interface::fork::{Fork, ForkManager};
 use sov_rollup_interface::soft_confirmation::SignedSoftConfirmation;
 use sov_rollup_interface::spec::SpecId;
@@ -258,31 +257,54 @@ where
             soft_confirmation.timestamp(),
         );
 
-        let unsigned_raw = borsh::to_vec(&unsigned).unwrap();
-
         // check the claimed hash
-        if soft_confirmation.hash()
-            != Into::<[u8; 32]>::into(<C as Spec>::Hasher::digest(unsigned_raw))
-        {
-            return (
-                Err(SoftConfirmationError::InvalidSoftConfirmationHash),
-                batch_workspace.revert(),
-            );
-        }
+        if current_spec >= SpecId::Fork1 {
+            let digest = unsigned.compute_digest::<<C as Spec>::Hasher>();
+            let hash = Into::<[u8; 32]>::into(digest);
+            if soft_confirmation.hash() != hash {
+                return (
+                    Err(SoftConfirmationError::InvalidSoftConfirmationHash),
+                    batch_workspace.revert(),
+                );
+            }
 
-        // verify signature
-        if verify_soft_confirmation_signature::<C>(
-            unsigned,
-            soft_confirmation.signature(),
-            sequencer_public_key,
-        )
-        .is_err()
-        {
-            return (
-                Err(SoftConfirmationError::InvalidSoftConfirmationSignature),
-                batch_workspace.revert(),
-            );
-        }
+            // verify signature
+            if verify_soft_confirmation_signature::<C>(
+                soft_confirmation,
+                soft_confirmation.signature(),
+                sequencer_public_key,
+            )
+            .is_err()
+            {
+                return (
+                    Err(SoftConfirmationError::InvalidSoftConfirmationSignature),
+                    batch_workspace.revert(),
+                );
+            }
+        } else {
+            let digest = unsigned.pre_fork1_hash::<<C as Spec>::Hasher>();
+            let hash = Into::<[u8; 32]>::into(digest);
+            if soft_confirmation.hash() != hash {
+                return (
+                    Err(SoftConfirmationError::InvalidSoftConfirmationHash),
+                    batch_workspace.revert(),
+                );
+            }
+
+            // verify signature
+            if pre_fork1_verify_soft_confirmation_signature::<C>(
+                &unsigned,
+                soft_confirmation.signature(),
+                sequencer_public_key,
+            )
+            .is_err()
+            {
+                return (
+                    Err(SoftConfirmationError::InvalidSoftConfirmationSignature),
+                    batch_workspace.revert(),
+                );
+            }
+        };
 
         self.end_soft_confirmation_inner(
             current_spec,
@@ -802,16 +824,35 @@ where
 }
 
 fn verify_soft_confirmation_signature<C: Context>(
-    unsigned_soft_confirmation: UnsignedSoftConfirmation,
+    signed_soft_confirmation: &SignedSoftConfirmation,
+    signature: &[u8],
+    sequencer_public_key: &[u8],
+) -> Result<(), anyhow::Error> {
+    let message = signed_soft_confirmation.hash();
+
+    let signature = C::Signature::try_from(signature)?;
+
+    signature.verify(
+        &C::PublicKey::try_from(sequencer_public_key)?,
+        message.as_slice(),
+    )?;
+
+    Ok(())
+}
+
+// Old version of verify_soft_confirmation_signature
+// TODO: Remove derive(BorshSerialize) for UnsignedSoftConfirmation
+//   when removing this fn
+// FIXME: ^
+fn pre_fork1_verify_soft_confirmation_signature<C: Context>(
+    unsigned_soft_confirmation: &UnsignedSoftConfirmation,
     signature: &[u8],
     sequencer_public_key: &[u8],
 ) -> Result<(), anyhow::Error> {
     let message = borsh::to_vec(&unsigned_soft_confirmation).unwrap();
 
     let signature = C::Signature::try_from(signature)?;
 
-    // TODO: if verify function is modified to take the claimed hash in signed soft confirmation
-    // we wouldn't need to hash the thing twice
     signature.verify(
         &C::PublicKey::try_from(sequencer_public_key)?,
         message.as_slice(),

crates/sovereign-sdk/rollup-interface/src/spec.rs

@@ -15,6 +15,7 @@ mod spec {
         Copy,
         Eq,
         PartialEq,
+        PartialOrd,
         Default,
         BorshDeserialize,
         BorshSerialize,
@@ -28,6 +29,11 @@ mod spec {
         /// Genesis spec
         #[default]
         Genesis = 0,
+        /// First fork activates:
+        /// 1. the light client proof
+        /// 2. EVM cancun upgrade (with no kzg precompile)
+        /// 3. Don't use borsh when signing SoftConfirmation's
+        Fork1 = 1,
     }
 }
 
@@ -41,6 +47,7 @@ mod spec {
         Copy,
         Eq,
         PartialEq,
+        PartialOrd,
         Default,
         BorshDeserialize,
         BorshSerialize,
@@ -58,5 +65,7 @@ mod spec {
         Fork1 = 1,
         /// Second fork
         Fork2 = 2,
+        /// Third fork
+        Fork3 = 3,
     }
 }

crates/sovereign-sdk/rollup-interface/src/state_machine/soft_confirmation.rs

@@ -8,6 +8,7 @@ use alloc::vec::Vec;
 use core::fmt::Debug;
 
 use borsh::{BorshDeserialize, BorshSerialize};
+use digest::{Digest, Output};
 use serde::{Deserialize, Serialize};
 
 /// Contains raw transactions and information about the soft confirmation block
@@ -79,6 +80,31 @@ impl<'txs> UnsignedSoftConfirmation<'txs> {
     pub fn timestamp(&self) -> u64 {
         self.timestamp
     }
+    /// Compute digest for the whole UnsignedSoftConfirmation struct
+    pub fn compute_digest<D: Digest>(&self) -> Output<D> {
+        let mut hasher = D::new();
+        hasher.update(self.l2_height.to_be_bytes());
+        hasher.update(self.da_slot_height.to_be_bytes());
+        hasher.update(self.da_slot_hash);
+        hasher.update(self.da_slot_txs_commitment);
+        for tx in self.txs {
+            hasher.update(tx);
+        }
+        for deposit in &self.deposit_data {
+            hasher.update(deposit);
+        }
+        hasher.update(self.l1_fee_rate.to_be_bytes());
+        hasher.update(self.timestamp.to_be_bytes());
+        hasher.finalize()
+    }
+    /// Old version of compute_digest
+    // TODO: Remove derive(BorshSerialize) for UnsignedSoftConfirmation
+    //   when removing this fn
+    // FIXME: ^
+    pub fn pre_fork1_hash<D: Digest>(&self) -> Output<D> {
+        let raw = borsh::to_vec(&self).unwrap();
+        D::digest(raw.as_slice())
+    }
 }
 
 /// Signed version of the `UnsignedSoftConfirmation`