diff --git a/src/Nethermind/Nethermind.Evm.Test/Tracing/AccessTxTracerTests.cs b/src/Nethermind/Nethermind.Evm.Test/Tracing/AccessTxTracerTests.cs index 6e6c5333d61d..8c631eb5e7be 100644 --- a/src/Nethermind/Nethermind.Evm.Test/Tracing/AccessTxTracerTests.cs +++ b/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!; + } } } diff --git a/src/Nethermind/Nethermind.Evm/StackAccessTracker.cs b/src/Nethermind/Nethermind.Evm/StackAccessTracker.cs index 78b840dc5363..e9f37761f45d 100644 --- a/src/Nethermind/Nethermind.Evm/StackAccessTracker.cs +++ b/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
AccessedAddresses => _trackingState.AccessedAddresses; public readonly JournalSet AccessedStorageCells => _trackingState.AccessedStorageCells; public readonly JournalCollection Logs => _trackingState.Logs; public readonly JournalSet
DestroyList => _trackingState.DestroyList; public readonly HashSet 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); } diff --git a/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs b/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs index b99e63e19849..fa32f6c3c066 100644 --- a/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs +++ b/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); diff --git a/src/Nethermind/Nethermind.Facade.Test/BlockchainBridgeTests.cs b/src/Nethermind/Nethermind.Facade.Test/BlockchainBridgeTests.cs index 8b28afa7ea1a..6f88176d1346 100644 --- a/src/Nethermind/Nethermind.Facade.Test/BlockchainBridgeTests.cs +++ b/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(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(), Arg.Any()) + .Returns(callInfo => + { + ITxTracer tracer = callInfo.ArgAt(1); + tracer.ReportAccess(accessedAddresses, accessedCells); + tracer.MarkAsSuccess(TestItem.AddressB, new GasConsumed(21000, 0), Array.Empty(), Array.Empty()); + return TransactionResult.Ok; + }); + + return _blockchainBridge.CreateAccessList(header, tx, null, optimize, null, default); + } + [Test] public void Call_tx_returns_InsufficientSenderBalanceError() { diff --git a/src/Nethermind/Nethermind.Facade/BlockchainBridge.cs b/src/Nethermind/Nethermind.Facade/BlockchainBridge.cs index 50563fd4c09f..1ab12e29a73a 100644 --- a/src/Nethermind/Nethermind.Facade/BlockchainBridge.cs +++ b/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 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
buffer, BlockHeader header, Transaction tx, bool optimize, FrozenSet 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)