fix(eth_createAccessList): preserve reverted-frame accesses and exclude precompiles

src/Nethermind/Nethermind.Evm.Test/Tracing/AccessTxTracerTests.cs

@@ -8,8 +8,10 @@
 using Nethermind.Blockchain.Tracing;
 using Nethermind.Core;
 using Nethermind.Core.Collections;
+using Nethermind.Core.Eip2930;
 using Nethermind.Core.Specs;
 using Nethermind.Core.Test.Builders;
+using Nethermind.Evm.State;
 using Nethermind.Evm.TransactionProcessing;
 using Nethermind.Int256;
 using Nethermind.Specs;
@@ -109,6 +111,86 @@ public void ReportAccess_AddressAIsSetToOptimizedAndHasStorageCell_AccessListHas
             Assert.That(sut.AccessList.Select(static x => x.StorageKeys), Has.Exactly(1).Contains(new UInt256(1)));
         }
 
+        [Test]
+        public void Reverted_call_target_address_is_still_captured_in_access_list()
+        {
+            // Code deployed at recipient: CALL AddressC, then REVERT
+            byte[] code = Prepare.EvmCode
+                .Call(TestItem.AddressC, 50000)
+                .PushData(0)
+                .PushData(0)
+                .Op(Instruction.REVERT)
+                .Done;
+
+            AccessList list = ExecuteRevertedFrameScenario(code);
+
+            list.Select(static t => t.Address).Should().Contain(TestItem.AddressC,
+                because: "addresses accessed inside reverted frames must survive for eth_createAccessList");
+        }
+
+        [Test]
+        public void Reverted_sub_frame_sload_storage_key_is_still_captured_in_access_list()
+        {
+            // Code at AddressC: SLOAD slot 7 then REVERT
+            byte[] addressCCode = Prepare.EvmCode
+                .PushData(7)
+                .Op(Instruction.SLOAD)
+                .PushData(0)
+                .PushData(0)
+                .Op(Instruction.REVERT)
+                .Done;
+
+            TestState.CreateAccount(TestItem.AddressC, 0);
+            TestState.InsertCode(TestItem.AddressC, addressCCode, SpecProvider.GenesisSpec);
+            TestState.Commit(SpecProvider.GenesisSpec);
+            TestState.CommitTree(0);
+
+            // Recipient code: CALL AddressC then STOP
+            byte[] recipientCode = Prepare.EvmCode
+                .Call(TestItem.AddressC, 50000)
+                .Op(Instruction.STOP)
+                .Done;
+
+            AccessList list = ExecuteRevertedFrameScenario(recipientCode);
+
+            // AddressC slot 7 must appear despite the REVERT inside AddressC's sub-frame
+            list.Should().ContainSingle(e => e.Address == TestItem.AddressC)
+                .Which.StorageKeys.Should().Contain(new UInt256(7),
+                    because: "storage cells first accessed inside a reverted sub-frame must still be captured");
+        }
+
+        [Test]
+        public void Outer_committed_and_inner_reverted_call_both_captured_in_access_list()
+        {
+            TestState.CreateAccount(TestItem.AddressE, 0);
+            TestState.InsertCode(TestItem.AddressE, Prepare.EvmCode.Op(Instruction.STOP).Done, SpecProvider.GenesisSpec);
+
+            // AddressC code: CALL AddressE then REVERT
+            byte[] addressCCode = Prepare.EvmCode
+                .Call(TestItem.AddressE, 20000)
+                .PushData(0)
+                .PushData(0)
+                .Op(Instruction.REVERT)
+                .Done;
+
+            TestState.CreateAccount(TestItem.AddressC, 0);
+            TestState.InsertCode(TestItem.AddressC, addressCCode, SpecProvider.GenesisSpec);
+            TestState.Commit(SpecProvider.GenesisSpec);
+            TestState.CommitTree(0);
+
+            // Recipient code: CALL AddressC (succeeds at EVM level but AddressC reverts internally)
+            byte[] recipientCode = Prepare.EvmCode
+                .Call(TestItem.AddressC, 50000)
+                .Op(Instruction.STOP)
+                .Done;
+
+            AccessList list = ExecuteRevertedFrameScenario(recipientCode);
+
+            Address[] addresses = list.Select(static t => t.Address).ToArray();
+            addresses.Should().Contain(TestItem.AddressC, because: "committed outer CALL target must be in access list");
+            addresses.Should().Contain(TestItem.AddressE, because: "address accessed inside inner reverted frame must still be in access list");
+        }
+
         protected override ISpecProvider SpecProvider => new TestSpecProvider(Berlin.Instance);
 
         protected (AccessTxTracer trace, Block block, Transaction transaction) ExecuteAndTraceAccessCall(SenderRecipientAndMiner addresses, params byte[] code)
@@ -118,5 +200,13 @@ public void ReportAccess_AddressAIsSetToOptimizedAndHasStorageCell_AccessListHas
             _processor.Execute(transaction, new BlockExecutionContext(block.Header, Spec), tracer);
             return (tracer, block, transaction);
         }
+
+        private AccessList ExecuteRevertedFrameScenario(byte[] recipientCode)
+        {
+            (Block block, Transaction tx) = PrepareTx(BlockNumber, 100000, recipientCode, SenderRecipientAndMiner.Default);
+            AccessTxTracer tracer = new(SenderRecipientAndMiner.Default.Sender, SenderRecipientAndMiner.Default.Recipient, SenderRecipientAndMiner.Default.Miner);
+            _processor.Execute(tx, new BlockExecutionContext(block.Header, Spec), tracer);
+            return tracer.AccessList!;
+        }
     }
 }

src/Nethermind/Nethermind.Evm/StackAccessTracker.cs

@@ -11,14 +11,17 @@
 
 namespace Nethermind.Evm;
 
-public struct StackAccessTracker() : IDisposable
+public struct StackAccessTracker(bool isTracingAccess) : IDisposable
 {
+    public StackAccessTracker() : this(false) { }
+
     public readonly JournalSet<Address> AccessedAddresses => _trackingState.AccessedAddresses;
     public readonly JournalSet<StorageCell> AccessedStorageCells => _trackingState.AccessedStorageCells;
     public readonly JournalCollection<LogEntry> Logs => _trackingState.Logs;
     public readonly JournalSet<Address> DestroyList => _trackingState.DestroyList;
     public readonly HashSet<AddressAsKey> CreateList => _trackingState.CreateList;
 
+    private readonly bool _isTracingAccess = isTracingAccess;
     private TrackingState _trackingState = TrackingState.RentState();
 
     private int _addressesSnapshots;
@@ -65,8 +68,13 @@ public void TakeSnapshot()
 
     public readonly void Restore()
     {
-        _trackingState.AccessedAddresses.Restore(_addressesSnapshots);
-        _trackingState.AccessedStorageCells.Restore(_storageKeysSnapshots);
+        // When tracing access, don't restore the access sets on sub-frame revert.
+        // The generated list will pre-warm all touched addresses.
+        if (!_isTracingAccess)
+        {
+            _trackingState.AccessedAddresses.Restore(_addressesSnapshots);
+            _trackingState.AccessedStorageCells.Restore(_storageKeysSnapshots);
+        }
         _trackingState.DestroyList.Restore(_destroyListSnapshots);
         _trackingState.Logs.Restore(_logsSnapshots);
     }

src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs

@@ -216,7 +216,7 @@ private TransactionResult Execute(Transaction tx, ITxTracer tracer, ExecutionOpt
             if (commit) WorldState.Commit(spec, tracer.IsTracingState ? tracer : NullTxTracer.Instance, commitRoots: false);
 
             // substate.Logs contains a reference to accessTracker.Logs so we can't Dispose until end of the method
-            using StackAccessTracker accessTracker = new();
+            using StackAccessTracker accessTracker = new(tracer.IsTracingAccess);
 
             int delegationRefunds = !spec.IsEip7702Enabled || !tx.HasAuthorizationList ? 0 : ProcessDelegations(tx, spec, accessTracker);
 

src/Nethermind/Nethermind.Facade.Test/BlockchainBridgeTests.cs

@@ -28,6 +28,7 @@
 using Nethermind.Facade.Proxy.Models.Simulate;
 using Nethermind.Facade.Simulate;
 using Nethermind.Core.Specs;
+using Nethermind.Core.Precompiles;
 using Nethermind.State;
 
 namespace Nethermind.Facade.Test;
@@ -335,6 +336,55 @@ public void BlobBaseFee_is_set_for_non_blob_transaction([ValueSource(nameof(Brid
             Arg.Is<BlockExecutionContext>(blkCtx => blkCtx.BlobBaseFee == expectedBlobBaseFeeHash));
     }
 
+    [TestCase(true)]
+    [TestCase(false)]
+    public void CreateAccessList_filters_precompile_addresses_with_empty_storage_keys(bool optimize)
+    {
+        CallOutput callOutput = InvokeCreateAccessListWithMockedAccess(
+            optimize,
+            [PrecompiledAddresses.ECRecover, TestItem.AddressC],
+            [new StorageCell(TestItem.AddressC, UInt256.One)]);
+
+        callOutput.AccessList.Should().NotBeNull();
+        callOutput.AccessList!.Any(e => e.Address == PrecompiledAddresses.ECRecover).Should().BeFalse();
+        callOutput.AccessList.Any(e => e.Address == TestItem.AddressC).Should().BeTrue();
+    }
+
+    [Test]
+    public void CreateAccessList_keeps_precompile_address_when_storage_key_is_present()
+    {
+        CallOutput callOutput = InvokeCreateAccessListWithMockedAccess(
+            optimize: false,
+            [PrecompiledAddresses.ECRecover],
+            [new StorageCell(PrecompiledAddresses.ECRecover, UInt256.One)]);
+
+        callOutput.AccessList.Should().NotBeNull();
+        callOutput.AccessList!.Any(e => e.Address == PrecompiledAddresses.ECRecover).Should().BeTrue();
+    }
+
+    private CallOutput InvokeCreateAccessListWithMockedAccess(
+        bool optimize,
+        Address[] accessedAddresses,
+        StorageCell[] accessedCells)
+    {
+        BlockHeader header = Build.A.BlockHeader.TestObject;
+        Transaction tx = Build.A.Transaction
+            .WithSenderAddress(TestItem.AddressA)
+            .WithTo(TestItem.AddressB)
+            .TestObject;
+
+        _transactionProcessor.CallAndRestore(Arg.Any<Transaction>(), Arg.Any<ITxTracer>())
+            .Returns(callInfo =>
+            {
+                ITxTracer tracer = callInfo.ArgAt<ITxTracer>(1);
+                tracer.ReportAccess(accessedAddresses, accessedCells);
+                tracer.MarkAsSuccess(TestItem.AddressB, new GasConsumed(21000, 0), Array.Empty<byte>(), Array.Empty<LogEntry>());
+                return TransactionResult.Ok;
+            });
+
+        return _blockchainBridge.CreateAccessList(header, tx, null, optimize, null, default);
+    }
+
     [Test]
     public void Call_tx_returns_InsufficientSenderBalanceError()
     {

src/Nethermind/Nethermind.Facade/BlockchainBridge.cs

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: LGPL-3.0-only
 
 using System;
+using System.Collections.Frozen;
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using Nethermind.Blockchain;
@@ -244,9 +245,13 @@ private CallOutput ConvergeAccessList(BlockProcessingComponents components, Bloc
         {
             // Loop-invariant: the addresses to filter from the discovered AL depend only on header
             // and tx, neither of which change between iterations. Compute once and reuse.
-            Address[] addressesToOptimize = BuildAddressesToOptimize(header, tx, optimize);
+            FrozenSet<AddressAsKey> precompiles = specProvider.GetSpec(header).Precompiles;
+            int bufferSize = (optimize ? 3 : 1) + precompiles.Count;
+            Address[] addressBuffer = new Address[bufferSize];
+            FillAddressesToOptimize(addressBuffer, header, tx, optimize, precompiles);
+
             AccessList? previousAccessList = tx.AccessList;
-            AccessTxTracer accessTracer = new(addressesToOptimize);
+            AccessTxTracer accessTracer = new(addressBuffer);
             CallOutputTracer outputTracer = new();
             CancellationTxTracer tracer = new CompositeTxTracer(outputTracer, accessTracer).WithCancellation(cancellationToken);
             TransactionResult result;
@@ -273,16 +278,27 @@ private CallOutput ConvergeAccessList(BlockProcessingComponents components, Bloc
             };
         }
 
-        private Address[] BuildAddressesToOptimize(BlockHeader header, Transaction tx, bool optimize)
+        private void FillAddressesToOptimize(Span<Address> buffer, BlockHeader header, Transaction tx, bool optimize, FrozenSet<AddressAsKey> precompiles)
         {
+            int idx;
             if (!optimize)
-                return [header.GasBeneficiary];
+            {
+                buffer[0] = header.GasBeneficiary;
+                idx = 1;
+            }
+            else
+            {
+                // EIP-2930: sender, recipient and gas beneficiary are implicitly accessed,
+                // so excluding them keeps the returned access list minimal.
+                UInt256 senderNonce = tx.IsContractCreation ? stateReader.GetNonce(header, tx.SenderAddress) : UInt256.Zero;
+                buffer[0] = tx.SenderAddress;
+                buffer[1] = tx.GetRecipient(senderNonce);
+                buffer[2] = header.GasBeneficiary;
+                idx = 3;
+            }
 
-            // EIP-2930: sender, recipient and gas beneficiary are implicitly accessed,
-            // so excluding them keeps the returned access list minimal.
-            UInt256 senderNonce = tx.IsContractCreation ? stateReader.GetNonce(header, tx.SenderAddress) : UInt256.Zero;
-            Address recipient = tx.GetRecipient(senderNonce);
-            return [tx.SenderAddress, recipient, header.GasBeneficiary];
+            foreach (AddressAsKey p in precompiles)
+                buffer[idx++] = p.Value;
         }
 
         private static bool HasConverged(AccessList? previous, AccessList? discovered)