Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small refactor of TransactionProcessor after comment #7495

Open
wants to merge 6 commits into
base: pectra_fix_7702_after_merge
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/Nethermind/Nethermind.Evm/CodeInfoRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ public void InsertCode(IWorldState state, ReadOnlyMemory<byte> code, Address cod
_codeCache.Set(codeHash, codeInfo);
}

/// <summary>
/// Insert a delegation to <paramref name="codeSource"/> into <paramref name="authority"/>
/// </summary>
public void SetDelegation(IWorldState state, Address codeSource, Address authority, IReleaseSpec spec)
{
byte[] authorizedBuffer = new byte[Eip7702Constants.DelegationHeader.Length + Address.Size];
Expand Down Expand Up @@ -164,7 +167,6 @@ public ValueHash256 GetExecutableCodeHash(IWorldState worldState, Address addres

/// <remarks>
/// Parses delegation code to extract the contained address.
/// <b>Assumes </b><paramref name="code"/> <b>is delegation code!</b>
/// </remarks>
private static bool TryGetDelegatedAddress(ReadOnlySpan<byte> code, [NotNullWhen(true)] out Address? address)
{
Expand Down
19 changes: 18 additions & 1 deletion src/Nethermind/Nethermind.Evm/EvmState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,25 @@ public void ReturnStacks(byte[] dataStack, int[] returnStack)

public int ReturnStackHead = 0;
private bool _canRestore = true;

public EvmState(
long gasAvailable,
ExecutionEnvironment env,
ExecutionType executionType,
bool isTopLevel,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be removed as parameter and hardcoded to true, not to use this for not top level states

Snapshot snapshot,
JournalSet<Address> warmAddresses)
: this(
gasAvailable,
env,
executionType,
isTopLevel,
snapshot,
false)
{
_accessedAddresses = warmAddresses;
}

internal EvmState(
long gasAvailable,
ExecutionEnvironment env,
ExecutionType executionType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@
// SPDX-License-Identifier: LGPL-3.0-only

using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Linq;
using System.Runtime.CompilerServices;
using Nethermind.Core;
using Nethermind.Core.Collections;
using Nethermind.Core.Crypto;
using Nethermind.Core.Specs;
using Nethermind.Crypto;
Expand Down Expand Up @@ -41,7 +43,7 @@ public abstract class TransactionProcessorBase : ITransactionProcessor
private readonly ICodeInfoRepository _codeInfoRepository;
private SystemTransactionProcessor? _systemTransactionProcessor;
private readonly ILogManager _logManager;
private readonly HashSet<Address> _accessedAddresses = [];
private readonly WarmAddressBuilder _warmAddressBuilder = new();

[Flags]
protected enum ExecutionOptions
Expand Down Expand Up @@ -150,13 +152,13 @@ protected virtual TransactionResult Execute(Transaction tx, in BlockExecutionCon

if (commit) WorldState.Commit(spec, tracer.IsTracingState ? tracer : NullTxTracer.Instance, commitStorageRoots: false);

_accessedAddresses.Clear();
int delegationRefunds = ProcessDelegations(tx, spec, _accessedAddresses);
JournalSet<Address> accessedAddresses = new();
int delegationRefunds = ProcessDelegations(tx, spec, accessedAddresses);

ExecutionEnvironment env = BuildExecutionEnvironment(tx, in blCtx, spec, effectiveGasPrice, _codeInfoRepository, _accessedAddresses);
ExecutionEnvironment env = BuildExecutionEnvironment(tx, in blCtx, spec, effectiveGasPrice, _codeInfoRepository, _warmAddressBuilder);

long gasAvailable = tx.GasLimit - intrinsicGas;
ExecuteEvmCall(tx, header, spec, tracer, opts, delegationRefunds, _accessedAddresses, gasAvailable, env, out TransactionSubstate? substate, out long spentGas, out byte statusCode);
ExecuteEvmCall(tx, header, spec, tracer, opts, delegationRefunds, _warmAddressBuilder.WarmAddresses, gasAvailable, env, out TransactionSubstate? substate, out long spentGas, out byte statusCode);
PayFees(tx, header, spec, tracer, substate, spentGas, premiumPerGas, statusCode);

// Finalize
Expand Down Expand Up @@ -205,7 +207,7 @@ protected virtual TransactionResult Execute(Transaction tx, in BlockExecutionCon
return TransactionResult.Ok;
}

private int ProcessDelegations(Transaction tx, IReleaseSpec spec, HashSet<Address> accessedAddresses)
private int ProcessDelegations(Transaction tx, IReleaseSpec spec, ICollection<Address> accessedAddresses)
{
int refunds = 0;
if (spec.IsEip7702Enabled && tx.HasAuthorizationList)
Expand Down Expand Up @@ -240,7 +242,7 @@ private int ProcessDelegations(Transaction tx, IReleaseSpec spec, HashSet<Addres

bool IsValidForExecution(
AuthorizationTuple authorizationTuple,
ISet<Address> accessedAddresses,
ICollection<Address> accessedAddresses,
[NotNullWhen(false)] out string? error)
{
if (authorizationTuple.Authority is null)
Expand Down Expand Up @@ -506,13 +508,13 @@ private ExecutionEnvironment BuildExecutionEnvironment(
IReleaseSpec spec,
in UInt256 effectiveGasPrice,
ICodeInfoRepository codeInfoRepository,
HashSet<Address> accessedAddresses)
WarmAddressBuilder accessedAddresses)
{
Address recipient = tx.GetRecipient(tx.IsContractCreation ? WorldState.GetNonce(tx.SenderAddress!) : 0);
if (recipient is null) ThrowInvalidDataException("Recipient has not been resolved properly before tx execution");

accessedAddresses.Add(recipient);
accessedAddresses.Add(tx.SenderAddress!);
accessedAddresses.WarmUpAddress(recipient);
accessedAddresses.WarmUpAddress(tx.SenderAddress!);

TxExecutionContext executionContext = new(in blCtx, tx.SenderAddress, effectiveGasPrice, tx.BlobVersionedHashes, codeInfoRepository);
Address? delegationAddress = null;
Expand All @@ -521,7 +523,7 @@ private ExecutionEnvironment BuildExecutionEnvironment(
: codeInfoRepository.GetCachedCodeInfo(WorldState, recipient, spec, out delegationAddress);

if (delegationAddress is not null)
accessedAddresses.Add(delegationAddress);
accessedAddresses.WarmUpAddress(delegationAddress);

codeInfo.AnalyseInBackgroundIfRequired();

Expand Down Expand Up @@ -549,7 +551,7 @@ protected virtual void ExecuteEvmCall(
ITxTracer tracer,
ExecutionOptions opts,
int delegationRefunds,
IEnumerable<Address> accessedAddresses,
JournalSet<Address> warmedAddresses,
in long gasAvailable,
in ExecutionEnvironment env,
out TransactionSubstate? substate,
Expand All @@ -576,11 +578,15 @@ protected virtual void ExecuteEvmCall(
PrepareAccountForContractDeployment(env.ExecutingAccount, _codeInfoRepository, spec);
}

ExecutionType executionType = tx.IsContractCreation ? ExecutionType.CREATE : ExecutionType.TRANSACTION;

using (EvmState state = new(unspentGas, env, executionType, true, snapshot, false))
using (EvmState state = new(
unspentGas,
env,
tx.IsContractCreation ? ExecutionType.CREATE : ExecutionType.TRANSACTION,
true,
snapshot,
warmedAddresses))
{
WarmUp(tx, header, spec, state, accessedAddresses);
WarmUp(tx, header, spec, state);

substate = !tracer.IsTracingActions
? VirtualMachine.Run<NotTracing>(state, WorldState, tracer)
Expand Down Expand Up @@ -661,21 +667,13 @@ protected virtual void PayValue(Transaction tx, IReleaseSpec spec, ExecutionOpti
WorldState.SubtractFromBalance(tx.SenderAddress!, tx.Value, spec);
}

private void WarmUp(Transaction tx, BlockHeader header, IReleaseSpec spec, EvmState state, IEnumerable<Address> accessedAddresses)
private void WarmUp(Transaction tx, BlockHeader header, IReleaseSpec spec, EvmState state)
{
if (spec.UseTxAccessLists)
{
state.WarmUp(tx.AccessList); // eip-2930
}

if (spec.UseHotAndColdStorage) // eip-2929
{
foreach (Address accessed in accessedAddresses)
{
state.WarmUp(accessed);
}
}

if (spec.AddCoinbaseToTxAccessList)
{
state.WarmUp(header.GasBeneficiary!);
Expand Down Expand Up @@ -760,6 +758,64 @@ protected virtual long Refund(Transaction tx, BlockHeader header, IReleaseSpec s
[DoesNotReturn]
[StackTraceHidden]
private static void ThrowTransactionCollisionException() => throw new TransactionCollisionException();

private sealed class WarmAddressBuilder: ICollection<Address>
{
private IReleaseSpec _spec;
public JournalSet<Address> WarmAddresses { get; private set; }

public int Count => WarmAddresses.Count;

public bool IsReadOnly => WarmAddresses.IsReadOnly;

public void WarmUpAddress(Address address)
{
Debug.Assert(WarmAddresses is not null);
if (_spec.UseHotAndColdStorage) // eip-2929
WarmAddresses.Add(address);
}
public void StartNewBuild(IReleaseSpec releaseSpec)
{
_spec = releaseSpec;
WarmAddresses = [];
}

public void Add(Address item)
{
Debug.Assert(WarmAddresses is not null);
WarmUpAddress(item);
}

public void Clear()
{
throw new NotImplementedException();
}

public bool Contains(Address item)
{
return WarmAddresses.Contains(item);
}

public void CopyTo(Address[] array, int arrayIndex)
{
throw new NotImplementedException();
}

public bool Remove(Address item)
{
throw new NotImplementedException();
}

public IEnumerator<Address> GetEnumerator()
{
return WarmAddresses.GetEnumerator();
}

IEnumerator IEnumerable.GetEnumerator()
{
return WarmAddresses.GetEnumerator();
}
}
}

public readonly struct TransactionResult(string? error)
Expand Down
Loading