fix(op-reth): non-deposit txs have depositReceiptVersion=1 (#6784)

crates/primitives/src/transaction/signature.rs

@@ -32,7 +32,7 @@ impl Signature {
     /// Returns the signature for the optimism deposit transactions, which don't include a
     /// signature.
     #[cfg(feature = "optimism")]
-    pub(crate) const fn optimism_deposit_tx_signature() -> Self {
+    pub const fn optimism_deposit_tx_signature() -> Self {
         Signature { r: U256::ZERO, s: U256::ZERO, odd_y_parity: false }
     }
 }

crates/revm/Cargo.toml

@@ -28,12 +28,12 @@ tracing.workspace = true
 
 [dev-dependencies]
 reth-trie.workspace = true
-reth-node-ethereum.workspace = true
 
 [features]
 optimism = [
     "revm/optimism",
     "reth-primitives/optimism",
+    "reth-provider/optimism",
     "reth-consensus-common/optimism",
     "reth-interfaces/optimism",
 ]

crates/revm/src/lib.rs

@@ -35,6 +35,10 @@ pub mod stack;
 #[cfg(feature = "optimism")]
 pub mod optimism;
 
+/// Common test helpers
+#[cfg(test)]
+pub mod test_utils;
+
 // Convenience re-exports.
 pub use revm::{self, *};
 pub use revm_inspectors::*;

crates/revm/src/optimism/processor.rs

@@ -179,10 +179,10 @@ where
                 // receipt hashes should be computed when set. The state transition process ensures
                 // this is only set for post-Canyon deposit transactions.
                 #[cfg(feature = "optimism")]
-                deposit_receipt_version: self
-                    .chain_spec()
-                    .is_fork_active_at_timestamp(Hardfork::Canyon, block.timestamp)
-                    .then_some(1),
+                deposit_receipt_version: (transaction.is_deposit() &&
+                    self.chain_spec()
+                        .is_fork_active_at_timestamp(Hardfork::Canyon, block.timestamp))
+                .then_some(1),
             });
         }
 
@@ -202,3 +202,199 @@ where
         Some(self.evm.context.evm.db.bundle_size_hint())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::{
+        database::StateProviderDatabase,
+        test_utils::{StateProviderTest, TestEvmConfig},
+    };
+    use reth_primitives::{
+        Account, Address, Block, ChainSpecBuilder, Header, Signature, StorageKey, StorageValue,
+        Transaction, TransactionKind, TransactionSigned, TxEip1559, BASE_MAINNET,
+    };
+    use revm::L1_BLOCK_CONTRACT;
+    use std::{collections::HashMap, str::FromStr, sync::Arc};
+
+    fn create_op_state_provider() -> StateProviderTest {
+        let mut db = StateProviderTest::default();
+
+        let l1_block_contract_account =
+            Account { balance: U256::ZERO, bytecode_hash: None, nonce: 1 };
+
+        let mut l1_block_storage = HashMap::new();
+        // base fee
+        l1_block_storage.insert(StorageKey::with_last_byte(1), StorageValue::from(1000000000));
+        // l1 fee overhead
+        l1_block_storage.insert(StorageKey::with_last_byte(5), StorageValue::from(188));
+        // l1 fee scalar
+        l1_block_storage.insert(StorageKey::with_last_byte(6), StorageValue::from(684000));
+        // l1 free scalars post ecotone
+        l1_block_storage.insert(
+            StorageKey::with_last_byte(3),
+            StorageValue::from_str(
+                "0x0000000000000000000000000000000000001db0000d27300000000000000005",
+            )
+            .unwrap(),
+        );
+
+        db.insert_account(L1_BLOCK_CONTRACT, l1_block_contract_account, None, l1_block_storage);
+
+        db
+    }
+
+    fn create_op_evm_processor<'a>(
+        chain_spec: Arc<ChainSpec>,
+        db: StateProviderTest,
+    ) -> EVMProcessor<'a, TestEvmConfig> {
+        let mut executor = EVMProcessor::new_with_db(
+            chain_spec,
+            StateProviderDatabase::new(db),
+            TestEvmConfig::default(),
+        );
+        executor.evm.context.evm.db.load_cache_account(L1_BLOCK_CONTRACT).unwrap();
+        executor
+    }
+
+    #[test]
+    fn op_deposit_fields_pre_canyon() {
+        let header = Header {
+            timestamp: 1,
+            number: 1,
+            gas_limit: 1_000_000,
+            gas_used: 42_000,
+            ..Header::default()
+        };
+
+        let mut db = create_op_state_provider();
+
+        let addr = Address::ZERO;
+        let account = Account { balance: U256::MAX, ..Account::default() };
+        db.insert_account(addr, account, None, HashMap::new());
+
+        let chain_spec =
+            Arc::new(ChainSpecBuilder::from(&*BASE_MAINNET).regolith_activated().build());
+
+        let tx = TransactionSigned::from_transaction_and_signature(
+            Transaction::Eip1559(TxEip1559 {
+                chain_id: chain_spec.chain.id(),
+                nonce: 0,
+                gas_limit: 21_000,
+                to: TransactionKind::Call(addr),
+                ..Default::default()
+            }),
+            Signature::default(),
+        );
+
+        let tx_deposit = TransactionSigned::from_transaction_and_signature(
+            Transaction::Deposit(reth_primitives::TxDeposit {
+                from: addr,
+                to: TransactionKind::Call(addr),
+                gas_limit: 21_000,
+                ..Default::default()
+            }),
+            Signature::default(),
+        );
+
+        let mut executor = create_op_evm_processor(chain_spec, db);
+
+        // Attempt to execute a block with one deposit and one non-deposit transaction
+        executor
+            .execute(
+                &BlockWithSenders {
+                    block: Block {
+                        header: header.clone(),
+                        body: vec![tx, tx_deposit],
+                        ommers: vec![],
+                        withdrawals: None,
+                    },
+                    senders: vec![addr, addr],
+                },
+                U256::ZERO,
+            )
+            .unwrap();
+
+        let tx_receipt = executor.receipts[0][0].as_ref().unwrap();
+        let deposit_receipt = executor.receipts[0][1].as_ref().unwrap();
+
+        // deposit_receipt_version is not present in pre canyon transactions
+        assert!(deposit_receipt.deposit_receipt_version.is_none());
+        assert!(tx_receipt.deposit_receipt_version.is_none());
+
+        // deposit_nonce is present only in deposit transactions
+        assert!(deposit_receipt.deposit_nonce.is_some());
+        assert!(tx_receipt.deposit_nonce.is_none());
+    }
+
+    #[test]
+    fn op_deposit_fields_post_canyon() {
+        // ensure_create2_deployer will fail if timestamp is set to less then 2
+        let header = Header {
+            timestamp: 2,
+            number: 1,
+            gas_limit: 1_000_000,
+            gas_used: 42_000,
+            ..Header::default()
+        };
+
+        let mut db = create_op_state_provider();
+        let addr = Address::ZERO;
+        let account = Account { balance: U256::MAX, ..Account::default() };
+
+        db.insert_account(addr, account, None, HashMap::new());
+
+        let chain_spec =
+            Arc::new(ChainSpecBuilder::from(&*BASE_MAINNET).canyon_activated().build());
+
+        let tx = TransactionSigned::from_transaction_and_signature(
+            Transaction::Eip1559(TxEip1559 {
+                chain_id: chain_spec.chain.id(),
+                nonce: 0,
+                gas_limit: 21_000,
+                to: TransactionKind::Call(addr),
+                ..Default::default()
+            }),
+            Signature::default(),
+        );
+
+        let tx_deposit = TransactionSigned::from_transaction_and_signature(
+            Transaction::Deposit(reth_primitives::TxDeposit {
+                from: addr,
+                to: TransactionKind::Call(addr),
+                gas_limit: 21_000,
+                ..Default::default()
+            }),
+            Signature::optimism_deposit_tx_signature(),
+        );
+
+        let mut executor = create_op_evm_processor(chain_spec, db);
+
+        // attempt to execute an empty block with parent beacon block root, this should not fail
+        executor
+            .execute(
+                &BlockWithSenders {
+                    block: Block {
+                        header: header.clone(),
+                        body: vec![tx, tx_deposit],
+                        ommers: vec![],
+                        withdrawals: None,
+                    },
+                    senders: vec![addr, addr],
+                },
+                U256::ZERO,
+            )
+            .expect("Executing a block while canyon is active should not fail");
+
+        let tx_receipt = executor.receipts[0][0].as_ref().unwrap();
+        let deposit_receipt = executor.receipts[0][1].as_ref().unwrap();
+
+        // deposit_receipt_version is set to 1 for post canyon deposit transations
+        assert_eq!(deposit_receipt.deposit_receipt_version, Some(1));
+        assert!(tx_receipt.deposit_receipt_version.is_none());
+
+        // deposit_nonce is present only in deposit transactions
+        assert!(deposit_receipt.deposit_nonce.is_some());
+        assert!(tx_receipt.deposit_nonce.is_none());
+    }
+}