From dd5df1ea3a439d64f8b31f794cbf99c91b870a9b Mon Sep 17 00:00:00 2001 From: Alexey Osipov Date: Wed, 19 Jun 2024 14:30:15 +0300 Subject: [PATCH 1/8] Throw on non-emppty storage Save from metrics failures Update tests --- .../Pyspecs/EipTests.cs | 24 -------------- .../Pyspecs/VMTests.cs | 24 -------------- .../Pyspecs/WithdrawalsTests.cs | 24 -------------- .../Ethereum.Test.Base/GeneralTestBase.cs | 6 +++- .../LoadBlockchainTestsStrategy.cs | 2 +- .../LoadEipTestsStrategy.cs | 2 +- .../LoadGeneralStateTestFileStrategy.cs | 2 +- .../LoadGeneralStateTestsStrategy.cs | 3 +- .../LoadLegacyBlockchainTestsStrategy.cs | 2 +- .../LoadLegacyGeneralStateTestsStrategy.cs | 3 +- .../TransactionProcessor.cs | 33 ++++++++++++------- .../Nethermind.Evm/VirtualMachine.cs | 10 ++---- src/tests | 2 +- 13 files changed, 35 insertions(+), 102 deletions(-) delete mode 100644 src/Nethermind/Ethereum.Blockchain.Block.Test/Pyspecs/EipTests.cs delete mode 100644 src/Nethermind/Ethereum.Blockchain.Block.Test/Pyspecs/VMTests.cs delete mode 100644 src/Nethermind/Ethereum.Blockchain.Block.Test/Pyspecs/WithdrawalsTests.cs diff --git a/src/Nethermind/Ethereum.Blockchain.Block.Test/Pyspecs/EipTests.cs b/src/Nethermind/Ethereum.Blockchain.Block.Test/Pyspecs/EipTests.cs deleted file mode 100644 index 824fd6f2a48..00000000000 --- a/src/Nethermind/Ethereum.Blockchain.Block.Test/Pyspecs/EipTests.cs +++ /dev/null @@ -1,24 +0,0 @@ -// SPDX-FileCopyrightText: 2023 Demerzel Solutions Limited -// SPDX-License-Identifier: LGPL-3.0-only - -using System.Collections.Generic; -using System.Threading.Tasks; -using Ethereum.Test.Base; -using NUnit.Framework; - -namespace Ethereum.Blockchain.Block.Test.Pyspecs; - -[TestFixture] -[Parallelizable(ParallelScope.All)] -public class EipTests : BlockchainTestBase -{ - [TestCaseSource(nameof(LoadTests))] - public async Task Test(BlockchainTest test) => await RunTest(test); - - public static IEnumerable LoadTests() - { - var loader = new TestsSourceLoader(new LoadBlockchainTestsStrategy(), "Pyspecs/eips"); - - return (IEnumerable)loader.LoadTests(); - } -} diff --git a/src/Nethermind/Ethereum.Blockchain.Block.Test/Pyspecs/VMTests.cs b/src/Nethermind/Ethereum.Blockchain.Block.Test/Pyspecs/VMTests.cs deleted file mode 100644 index 9bdcec07bdc..00000000000 --- a/src/Nethermind/Ethereum.Blockchain.Block.Test/Pyspecs/VMTests.cs +++ /dev/null @@ -1,24 +0,0 @@ -// SPDX-FileCopyrightText: 2023 Demerzel Solutions Limited -// SPDX-License-Identifier: LGPL-3.0-only - -using System.Collections.Generic; -using System.Threading.Tasks; -using Ethereum.Test.Base; -using NUnit.Framework; - -namespace Ethereum.Blockchain.Block.Test.Pyspecs; - -[TestFixture] -[Parallelizable(ParallelScope.All)] -public class VMTests : BlockchainTestBase -{ - [TestCaseSource(nameof(LoadTests))] - public async Task Test(BlockchainTest test) => await RunTest(test); - - public static IEnumerable LoadTests() - { - var loader = new TestsSourceLoader(new LoadBlockchainTestsStrategy(), "Pyspecs/vm"); - - return (IEnumerable)loader.LoadTests(); - } -} diff --git a/src/Nethermind/Ethereum.Blockchain.Block.Test/Pyspecs/WithdrawalsTests.cs b/src/Nethermind/Ethereum.Blockchain.Block.Test/Pyspecs/WithdrawalsTests.cs deleted file mode 100644 index 28582549fee..00000000000 --- a/src/Nethermind/Ethereum.Blockchain.Block.Test/Pyspecs/WithdrawalsTests.cs +++ /dev/null @@ -1,24 +0,0 @@ -// SPDX-FileCopyrightText: 2023 Demerzel Solutions Limited -// SPDX-License-Identifier: LGPL-3.0-only - -using System.Collections.Generic; -using System.Threading.Tasks; -using Ethereum.Test.Base; -using NUnit.Framework; - -namespace Ethereum.Blockchain.Block.Test.Pyspecs; - -[TestFixture] -[Parallelizable(ParallelScope.All)] -public class WithdrawalsTests : BlockchainTestBase -{ - [TestCaseSource(nameof(LoadTests))] - public async Task Test(BlockchainTest test) => await RunTest(test); - - public static IEnumerable LoadTests() - { - var loader = new TestsSourceLoader(new LoadBlockchainTestsStrategy(), "Pyspecs/withdrawals"); - - return (IEnumerable)loader.LoadTests(); - } -} diff --git a/src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs b/src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs index f3fabfa27da..37f363431e6 100644 --- a/src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs +++ b/src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs @@ -137,10 +137,13 @@ protected EthereumTestResult RunTest(GeneralStateTest test, ITxTracer txTracer) bool isValid = _txValidator.IsWellFormed(test.Transaction, spec) && IsValidBlock(block, specProvider); if (isValid) + { transactionProcessor.Execute(test.Transaction, new BlockExecutionContext(header), txTracer); + } + stopwatch.Stop(); - stateProvider.Commit(specProvider.GenesisSpec); + stateProvider.Commit(specProvider.GetSpec((ForkActivation)1)); stateProvider.CommitTree(1); // '@winsvega added a 0-wei reward to the miner , so we had to add that into the state test execution phase. He needed it for retesteth.' @@ -148,6 +151,7 @@ protected EthereumTestResult RunTest(GeneralStateTest test, ITxTracer txTracer) { stateProvider.CreateAccount(test.CurrentCoinbase, 0); } + stateProvider.Commit(specProvider.GetSpec((ForkActivation)1)); stateProvider.RecalculateStateRoot(); diff --git a/src/Nethermind/Ethereum.Test.Base/LoadBlockchainTestsStrategy.cs b/src/Nethermind/Ethereum.Test.Base/LoadBlockchainTestsStrategy.cs index 7fbc3fa0153..dfee892bdd6 100644 --- a/src/Nethermind/Ethereum.Test.Base/LoadBlockchainTestsStrategy.cs +++ b/src/Nethermind/Ethereum.Test.Base/LoadBlockchainTestsStrategy.cs @@ -38,7 +38,7 @@ private string GetBlockchainTestsDirectory() char pathSeparator = Path.AltDirectorySeparatorChar; string currentDirectory = AppDomain.CurrentDomain.BaseDirectory; - return currentDirectory.Remove(currentDirectory.LastIndexOf("src")) + $"src{pathSeparator}tests{pathSeparator}BlockchainTests"; + return Path.Combine(currentDirectory.Remove(currentDirectory.LastIndexOf("src")), "src", "tests", "BlockchainTests"); } private IEnumerable LoadTestsFromDirectory(string testDir, string wildcard) diff --git a/src/Nethermind/Ethereum.Test.Base/LoadEipTestsStrategy.cs b/src/Nethermind/Ethereum.Test.Base/LoadEipTestsStrategy.cs index 82b5f344d2c..08af330a4e0 100644 --- a/src/Nethermind/Ethereum.Test.Base/LoadEipTestsStrategy.cs +++ b/src/Nethermind/Ethereum.Test.Base/LoadEipTestsStrategy.cs @@ -39,7 +39,7 @@ private string GetGeneralStateTestsDirectory() char pathSeparator = Path.AltDirectorySeparatorChar; string currentDirectory = AppDomain.CurrentDomain.BaseDirectory; - return currentDirectory.Remove(currentDirectory.LastIndexOf("src")) + $"src{pathSeparator}tests{pathSeparator}EIPTests{pathSeparator}StateTests"; + return Path.Combine(currentDirectory.Remove(currentDirectory.LastIndexOf("src")), "src", "tests", "EIPTests", "StateTests"); } private IEnumerable LoadTestsFromDirectory(string testDir, string wildcard) diff --git a/src/Nethermind/Ethereum.Test.Base/LoadGeneralStateTestFileStrategy.cs b/src/Nethermind/Ethereum.Test.Base/LoadGeneralStateTestFileStrategy.cs index 1f94da85149..dd86d693566 100644 --- a/src/Nethermind/Ethereum.Test.Base/LoadGeneralStateTestFileStrategy.cs +++ b/src/Nethermind/Ethereum.Test.Base/LoadGeneralStateTestFileStrategy.cs @@ -51,7 +51,7 @@ private string GetGeneralStateTestsDirectory() char pathSeparator = Path.AltDirectorySeparatorChar; string currentDirectory = AppDomain.CurrentDomain.BaseDirectory; - return currentDirectory.Remove(currentDirectory.LastIndexOf("src")) + $"src{pathSeparator}tests{pathSeparator}GeneralStateTests"; + return Path.Combine(currentDirectory.Remove(currentDirectory.LastIndexOf("src")), "src", "tests", "GeneralStateTests"); } } } diff --git a/src/Nethermind/Ethereum.Test.Base/LoadGeneralStateTestsStrategy.cs b/src/Nethermind/Ethereum.Test.Base/LoadGeneralStateTestsStrategy.cs index 265b432d853..5373e3b01af 100644 --- a/src/Nethermind/Ethereum.Test.Base/LoadGeneralStateTestsStrategy.cs +++ b/src/Nethermind/Ethereum.Test.Base/LoadGeneralStateTestsStrategy.cs @@ -36,10 +36,9 @@ public IEnumerable Load(string testsDirectoryName, string wildcar private string GetGeneralStateTestsDirectory() { - char pathSeparator = Path.AltDirectorySeparatorChar; string currentDirectory = AppDomain.CurrentDomain.BaseDirectory; - return currentDirectory.Remove(currentDirectory.LastIndexOf("src")) + $"src{pathSeparator}tests{pathSeparator}GeneralStateTests"; + return Path.Combine(currentDirectory.Remove(currentDirectory.LastIndexOf("src")), "src", "tests", "GeneralStateTests"); } private IEnumerable LoadTestsFromDirectory(string testDir, string wildcard) diff --git a/src/Nethermind/Ethereum.Test.Base/LoadLegacyBlockchainTestsStrategy.cs b/src/Nethermind/Ethereum.Test.Base/LoadLegacyBlockchainTestsStrategy.cs index 9c7be901503..e47919bb884 100644 --- a/src/Nethermind/Ethereum.Test.Base/LoadLegacyBlockchainTestsStrategy.cs +++ b/src/Nethermind/Ethereum.Test.Base/LoadLegacyBlockchainTestsStrategy.cs @@ -38,7 +38,7 @@ private string GetLegacyBlockchainTestsDirectory() char pathSeparator = Path.AltDirectorySeparatorChar; string currentDirectory = AppDomain.CurrentDomain.BaseDirectory; - return currentDirectory.Remove(currentDirectory.LastIndexOf("src")) + $"src{pathSeparator}tests{pathSeparator}LegacyTests{pathSeparator}Constantinople{pathSeparator}BlockchainTests"; + return Path.Combine(currentDirectory.Remove(currentDirectory.LastIndexOf("src")), "src", "tests", "LegacyTests", "Constantinople", "BlockchainTests"); } private IEnumerable LoadTestsFromDirectory(string testDir, string wildcard) diff --git a/src/Nethermind/Ethereum.Test.Base/LoadLegacyGeneralStateTestsStrategy.cs b/src/Nethermind/Ethereum.Test.Base/LoadLegacyGeneralStateTestsStrategy.cs index c457994963a..2d815e13047 100644 --- a/src/Nethermind/Ethereum.Test.Base/LoadLegacyGeneralStateTestsStrategy.cs +++ b/src/Nethermind/Ethereum.Test.Base/LoadLegacyGeneralStateTestsStrategy.cs @@ -35,10 +35,9 @@ public IEnumerable Load(string testsDirectoryName, string wildcar private string GetLegacyGeneralStateTestsDirectory() { - char pathSeparator = Path.AltDirectorySeparatorChar; string currentDirectory = AppDomain.CurrentDomain.BaseDirectory; - return currentDirectory.Remove(currentDirectory.LastIndexOf("src")) + $"src{pathSeparator}tests{pathSeparator}LegacyTests{pathSeparator}Constantinople{pathSeparator}GeneralStateTests"; + return Path.Combine(currentDirectory.Remove(currentDirectory.LastIndexOf("src")), "src", "tests", "LegacyTests", "Constantinople", "GeneralStateTests"); } private IEnumerable LoadTestsFromDirectory(string testDir, string wildcard) diff --git a/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs b/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs index 95744638d44..c5ba370cdd4 100644 --- a/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs +++ b/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs @@ -187,20 +187,29 @@ private static void UpdateMetrics(ExecutionOptions opts, UInt256 effectiveGasPri { if (opts is ExecutionOptions.Commit or ExecutionOptions.None) { - float gasPrice = (float)((double)effectiveGasPrice / 1_000_000_000.0); - Metrics.MinGasPrice = Math.Min(gasPrice, Metrics.MinGasPrice); - Metrics.MaxGasPrice = Math.Max(gasPrice, Metrics.MaxGasPrice); + try + { + float gasPrice = (float)((double)effectiveGasPrice / 1_000_000_000.0); + if (float.IsInfinity(gasPrice)) + { + return; + } - Metrics.BlockMinGasPrice = Math.Min(gasPrice, Metrics.BlockMinGasPrice); - Metrics.BlockMaxGasPrice = Math.Max(gasPrice, Metrics.BlockMaxGasPrice); + Metrics.MinGasPrice = Math.Min(gasPrice, Metrics.MinGasPrice); + Metrics.MaxGasPrice = Math.Max(gasPrice, Metrics.MaxGasPrice); - Metrics.AveGasPrice = (Metrics.AveGasPrice * Metrics.Transactions + gasPrice) / (Metrics.Transactions + 1); - Metrics.EstMedianGasPrice += Metrics.AveGasPrice * 0.01f * float.Sign(gasPrice - Metrics.EstMedianGasPrice); - Metrics.Transactions++; + Metrics.BlockMinGasPrice = Math.Min(gasPrice, Metrics.BlockMinGasPrice); + Metrics.BlockMaxGasPrice = Math.Max(gasPrice, Metrics.BlockMaxGasPrice); - Metrics.BlockAveGasPrice = (Metrics.BlockAveGasPrice * Metrics.BlockTransactions + gasPrice) / (Metrics.BlockTransactions + 1); - Metrics.BlockEstMedianGasPrice += Metrics.BlockAveGasPrice * 0.01f * float.Sign(gasPrice - Metrics.BlockEstMedianGasPrice); - Metrics.BlockTransactions++; + Metrics.AveGasPrice = (Metrics.AveGasPrice * Metrics.Transactions + gasPrice) / (Metrics.Transactions + 1); + Metrics.EstMedianGasPrice += Metrics.AveGasPrice * 0.01f * float.Sign(gasPrice - Metrics.EstMedianGasPrice); + Metrics.Transactions++; + + Metrics.BlockAveGasPrice = (Metrics.BlockAveGasPrice * Metrics.BlockTransactions + gasPrice) / (Metrics.BlockTransactions + 1); + Metrics.BlockEstMedianGasPrice += Metrics.BlockAveGasPrice * 0.01f * float.Sign(gasPrice - Metrics.BlockEstMedianGasPrice); + Metrics.BlockTransactions++; + } + catch { } } } @@ -588,7 +597,7 @@ protected void PrepareAccountForContractDeployment(Address contractAddress, IRel // TODO: verify what should happen if code info is a precompile // (but this would generally be a hash collision) - if (codeIsNotEmpty || accountNonceIsNotZero) + if (codeIsNotEmpty || accountNonceIsNotZero || WorldState.GetStorageRoot(contractAddress) != Keccak.EmptyTreeHash) { if (Logger.IsTrace) { diff --git a/src/Nethermind/Nethermind.Evm/VirtualMachine.cs b/src/Nethermind/Nethermind.Evm/VirtualMachine.cs index 12ced688ccb..a2b3ff5ba68 100644 --- a/src/Nethermind/Nethermind.Evm/VirtualMachine.cs +++ b/src/Nethermind/Nethermind.Evm/VirtualMachine.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Runtime.CompilerServices; using Nethermind.Core; using Nethermind.Core.Crypto; @@ -27,15 +28,8 @@ namespace Nethermind.Evm; -using System.Collections.Frozen; -using System.Linq; -using System.Runtime.InteropServices; -using System.Threading; - using Int256; -using Nethermind.Core.Collections; - public class VirtualMachine : IVirtualMachine { public const int MaxCallDepth = 1024; @@ -2410,7 +2404,7 @@ private EvmExceptionType InstructionSelfDestruct(EvmState vmState, ref bool accountExists = _state.AccountExists(contractAddress); if (accountExists && (_codeInfoRepository.GetCachedCodeInfo(_worldState, contractAddress, spec).MachineCode.Length != 0 || - _state.GetNonce(contractAddress) != 0)) + _state.GetNonce(contractAddress) != 0 || _worldState.GetStorageRoot(contractAddress) != Keccak.EmptyTreeHash)) { /* we get the snapshot before this as there is a possibility with that we will touch an empty account and remove it even if the REVERT operation follows */ if (typeof(TLogger) == typeof(IsTracing)) _logger.Trace($"Contract collision at {contractAddress}"); diff --git a/src/tests b/src/tests index 661356317ac..ebbaa54c7a9 160000 --- a/src/tests +++ b/src/tests @@ -1 +1 @@ -Subproject commit 661356317ac6df52208d54187e692472a25a01f8 +Subproject commit ebbaa54c7a90e74313b846369fe87e9bd3a58369 From 1ff18963ddea68221fc52c1389306cd983a69ecd Mon Sep 17 00:00:00 2001 From: Alexey Osipov Date: Thu, 20 Jun 2024 17:13:28 +0300 Subject: [PATCH 2/8] Refactor, clean up --- .../Nethermind.Evm/AddressExtensions.cs | 11 ++++++ .../TransactionProcessor.cs | 20 +++-------- .../Nethermind.Evm/VirtualMachine.cs | 35 ++++++++++--------- .../IReadOnlyStateProvider.cs | 2 +- 4 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/Nethermind/Nethermind.Evm/AddressExtensions.cs b/src/Nethermind/Nethermind.Evm/AddressExtensions.cs index c52f5453184..e2e68f71181 100644 --- a/src/Nethermind/Nethermind.Evm/AddressExtensions.cs +++ b/src/Nethermind/Nethermind.Evm/AddressExtensions.cs @@ -4,8 +4,10 @@ using System; using Nethermind.Core; using Nethermind.Core.Crypto; +using Nethermind.Core.Specs; using Nethermind.Int256; using Nethermind.Serialization.Rlp; +using Nethermind.State; namespace Nethermind.Evm { @@ -36,5 +38,14 @@ public static Address From(Address deployingAddress, ReadOnlySpan salt, Re ValueHash256 contractAddressKeccak = ValueKeccak.Compute(bytes); return new Address(in contractAddressKeccak); } + + + /// See https://eips.ethereum.org/EIPS/eip-7610 + public static bool IsNonZeroAccount(this Address contractAddress, IReleaseSpec spec, ICodeInfoRepository codeInfoRepository, IWorldState state) + { + return codeInfoRepository.GetCachedCodeInfo(state, contractAddress, spec).MachineCode.Length != 0 || + state.GetNonce(contractAddress) != 0 || + state.GetStorageRoot(contractAddress) != Keccak.EmptyTreeHash; + } } } diff --git a/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs b/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs index c5ba370cdd4..07582365c8a 100644 --- a/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs +++ b/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs @@ -589,26 +589,14 @@ protected virtual void PayFees(Transaction tx, BlockHeader header, IReleaseSpec protected void PrepareAccountForContractDeployment(Address contractAddress, IReleaseSpec spec) { - if (WorldState.AccountExists(contractAddress)) + if (WorldState.AccountExists(contractAddress) && contractAddress.IsNonZeroAccount(spec, _codeInfoRepository, WorldState)) { - CodeInfo codeInfo = _codeInfoRepository.GetCachedCodeInfo(WorldState, contractAddress, spec); - bool codeIsNotEmpty = codeInfo.MachineCode.Length != 0; - bool accountNonceIsNotZero = WorldState.GetNonce(contractAddress) != 0; - - // TODO: verify what should happen if code info is a precompile - // (but this would generally be a hash collision) - if (codeIsNotEmpty || accountNonceIsNotZero || WorldState.GetStorageRoot(contractAddress) != Keccak.EmptyTreeHash) + if (Logger.IsTrace) { - if (Logger.IsTrace) - { - Logger.Trace($"Contract collision at {contractAddress}"); - } - - ThrowTransactionCollisionException(); + Logger.Trace($"Contract collision at {contractAddress}"); } - // we clean any existing storage (in case of a previously called self destruct) - WorldState.UpdateStorageRoot(contractAddress, Keccak.EmptyTreeHash); + ThrowTransactionCollisionException(); } } diff --git a/src/Nethermind/Nethermind.Evm/VirtualMachine.cs b/src/Nethermind/Nethermind.Evm/VirtualMachine.cs index a2b3ff5ba68..0f1bf70241b 100644 --- a/src/Nethermind/Nethermind.Evm/VirtualMachine.cs +++ b/src/Nethermind/Nethermind.Evm/VirtualMachine.cs @@ -137,8 +137,7 @@ internal sealed class VirtualMachine : IVirtualMachine where TLogger : private readonly IBlockhashProvider _blockhashProvider; private readonly ISpecProvider _specProvider; private readonly ILogger _logger; - private IWorldState _worldState; - private IWorldState _state = null!; + private IWorldState _state; private readonly Stack _stateStack = new(); private (Address Address, bool ShouldDelete) _parityTouchBugAccount = (Address.FromNumber(3), false); private ReadOnlyMemory _returnDataBuffer = Array.Empty(); @@ -163,7 +162,6 @@ public TransactionSubstate Run(EvmState state, IWorldState worl { _txTracer = txTracer; _state = worldState; - _worldState = worldState; IReleaseSpec spec = _specProvider.GetSpec(state.Env.TxExecutionContext.BlockExecutionContext.Header.Number, state.Env.TxExecutionContext.BlockExecutionContext.Header.Timestamp); EvmState currentState = state; @@ -237,7 +235,7 @@ public TransactionSubstate Run(EvmState state, IWorldState worl if (callResult.IsException) { if (typeof(TTracingActions) == typeof(IsTracing)) _txTracer.ReportActionError(callResult.ExceptionType); - _worldState.Restore(currentState.Snapshot); + _state.Restore(currentState.Snapshot); RevertParityTouchBugAccount(spec); @@ -415,7 +413,7 @@ public TransactionSubstate Run(EvmState state, IWorldState worl { if (typeof(TLogger) == typeof(IsTracing)) _logger.Trace($"exception ({ex.GetType().Name}) in {currentState.ExecutionType} at depth {currentState.Env.CallDepth} - restoring snapshot"); - _worldState.Restore(currentState.Snapshot); + _state.Restore(currentState.Snapshot); RevertParityTouchBugAccount(spec); @@ -1327,7 +1325,7 @@ private CallResult ExecuteCode externalCode = _codeInfoRepository.GetCachedCodeInfo(_worldState, address, spec).MachineCode; + ReadOnlyMemory externalCode = _codeInfoRepository.GetCachedCodeInfo(_state, address, spec).MachineCode; slice = externalCode.SliceWithZeroPadding(b, (int)result); vmState.Memory.Save(in a, in slice); if (typeof(TTracingInstructions) == typeof(IsTracing)) @@ -2037,7 +2035,7 @@ static void ThrowOperationCanceledException() => [MethodImpl(MethodImplOptions.NoInlining)] private void InstructionExtCodeSize(Address address, ref EvmStack stack, IReleaseSpec spec) where TTracingInstructions : struct, IIsTracing { - ReadOnlyMemory accountCode = _codeInfoRepository.GetCachedCodeInfo(_worldState, address, spec).MachineCode; + ReadOnlyMemory accountCode = _codeInfoRepository.GetCachedCodeInfo(_state, address, spec).MachineCode; UInt256 result = (UInt256)accountCode.Span.Length; stack.PushUInt256(in result); } @@ -2115,7 +2113,7 @@ private EvmExceptionType InstructionCall( !UpdateMemoryCost(vmState, ref gasAvailable, in outputOffset, outputLength) || !UpdateGas(gasExtra, ref gasAvailable)) return EvmExceptionType.OutOfGas; - CodeInfo codeInfo = _codeInfoRepository.GetCachedCodeInfo(_worldState, codeSource, spec); + CodeInfo codeInfo = _codeInfoRepository.GetCachedCodeInfo(_state, codeSource, spec); codeInfo.AnalyseInBackgroundIfRequired(); if (spec.Use63Over64Rule) @@ -2156,7 +2154,7 @@ private EvmExceptionType InstructionCall( return EvmExceptionType.None; } - Snapshot snapshot = _worldState.TakeSnapshot(); + Snapshot snapshot = _state.TakeSnapshot(); _state.SubtractFromBalance(caller, transferValue, spec); if (codeInfo.IsEmpty && typeof(TTracingInstructions) != typeof(IsTracing) && !_txTracer.IsTracingActions) @@ -2400,11 +2398,11 @@ private EvmExceptionType InstructionSelfDestruct(EvmState vmState, ref _state.IncrementNonce(env.ExecutingAccount); - Snapshot snapshot = _worldState.TakeSnapshot(); + Snapshot snapshot = _state.TakeSnapshot(); bool accountExists = _state.AccountExists(contractAddress); - if (accountExists && (_codeInfoRepository.GetCachedCodeInfo(_worldState, contractAddress, spec).MachineCode.Length != 0 || - _state.GetNonce(contractAddress) != 0 || _worldState.GetStorageRoot(contractAddress) != Keccak.EmptyTreeHash)) + + if (accountExists && contractAddress.IsNonZeroAccount(spec, _codeInfoRepository, _state)) { /* we get the snapshot before this as there is a possibility with that we will touch an empty account and remove it even if the REVERT operation follows */ if (typeof(TLogger) == typeof(IsTracing)) _logger.Trace($"Contract collision at {contractAddress}"); @@ -2413,11 +2411,7 @@ private EvmExceptionType InstructionSelfDestruct(EvmState vmState, ref return (EvmExceptionType.None, null); } - if (accountExists) - { - _state.UpdateStorageRoot(contractAddress, Keccak.EmptyTreeHash); - } - else if (_state.IsDeadAccount(contractAddress)) + if (_state.IsDeadAccount(contractAddress)) { _state.ClearStorage(contractAddress); } @@ -2458,6 +2452,13 @@ private EvmExceptionType InstructionSelfDestruct(EvmState vmState, ref return (EvmExceptionType.None, callState); } + private static bool IsNonEmptyAccount(IReleaseSpec spec, Address contractAddress, ICodeInfoRepository _codeInfoRepository, IWorldState state) + { + return _codeInfoRepository.GetCachedCodeInfo(state, contractAddress, spec).MachineCode.Length != 0 || + state.GetNonce(contractAddress) != 0 || + state.GetStorageRoot(contractAddress) != Keccak.EmptyTreeHash; + } + [SkipLocalsInit] private EvmExceptionType InstructionLog(EvmState vmState, ref EvmStack stack, ref long gasAvailable, Instruction instruction) where TTracing : struct, IIsTracing diff --git a/src/Nethermind/Nethermind.State/IReadOnlyStateProvider.cs b/src/Nethermind/Nethermind.State/IReadOnlyStateProvider.cs index 28760632939..7bfe0fa81e6 100644 --- a/src/Nethermind/Nethermind.State/IReadOnlyStateProvider.cs +++ b/src/Nethermind/Nethermind.State/IReadOnlyStateProvider.cs @@ -3,7 +3,6 @@ using Nethermind.Core; using Nethermind.Core.Crypto; -using Nethermind.Int256; using Nethermind.Trie; namespace Nethermind.State @@ -33,6 +32,7 @@ public interface IReadOnlyStateProvider : IAccountStateProvider bool IsDeadAccount(Address address); bool IsEmptyAccount(Address address); + bool HasStateForRoot(Hash256 stateRoot); } } From 4c602588dec0de59fdee2f3bcb28c87fec246a2a Mon Sep 17 00:00:00 2001 From: Alexey Osipov Date: Fri, 21 Jun 2024 11:17:10 +0300 Subject: [PATCH 3/8] Add overflow check --- .../TransactionProcessor.cs | 31 +++++++------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs b/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs index 07582365c8a..9c22348b6b5 100644 --- a/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs +++ b/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs @@ -187,29 +187,22 @@ private static void UpdateMetrics(ExecutionOptions opts, UInt256 effectiveGasPri { if (opts is ExecutionOptions.Commit or ExecutionOptions.None) { - try - { - float gasPrice = (float)((double)effectiveGasPrice / 1_000_000_000.0); - if (float.IsInfinity(gasPrice)) - { - return; - } + // log2(3e38) ~ 127.8, if effectiveGasPrice is more than 128bit long it may lead to an overflow + float gasPrice = effectiveGasPrice[2] != 0 || effectiveGasPrice[3] != 0 ? 3e29f : (float)((double)effectiveGasPrice / 1_000_000_000.0); - Metrics.MinGasPrice = Math.Min(gasPrice, Metrics.MinGasPrice); - Metrics.MaxGasPrice = Math.Max(gasPrice, Metrics.MaxGasPrice); + Metrics.MinGasPrice = Math.Min(gasPrice, Metrics.MinGasPrice); + Metrics.MaxGasPrice = Math.Max(gasPrice, Metrics.MaxGasPrice); - Metrics.BlockMinGasPrice = Math.Min(gasPrice, Metrics.BlockMinGasPrice); - Metrics.BlockMaxGasPrice = Math.Max(gasPrice, Metrics.BlockMaxGasPrice); + Metrics.BlockMinGasPrice = Math.Min(gasPrice, Metrics.BlockMinGasPrice); + Metrics.BlockMaxGasPrice = Math.Max(gasPrice, Metrics.BlockMaxGasPrice); - Metrics.AveGasPrice = (Metrics.AveGasPrice * Metrics.Transactions + gasPrice) / (Metrics.Transactions + 1); - Metrics.EstMedianGasPrice += Metrics.AveGasPrice * 0.01f * float.Sign(gasPrice - Metrics.EstMedianGasPrice); - Metrics.Transactions++; + Metrics.AveGasPrice = (Metrics.AveGasPrice * Metrics.Transactions + gasPrice) / (Metrics.Transactions + 1); + Metrics.EstMedianGasPrice += Metrics.AveGasPrice * 0.01f * float.Sign(gasPrice - Metrics.EstMedianGasPrice); + Metrics.Transactions++; - Metrics.BlockAveGasPrice = (Metrics.BlockAveGasPrice * Metrics.BlockTransactions + gasPrice) / (Metrics.BlockTransactions + 1); - Metrics.BlockEstMedianGasPrice += Metrics.BlockAveGasPrice * 0.01f * float.Sign(gasPrice - Metrics.BlockEstMedianGasPrice); - Metrics.BlockTransactions++; - } - catch { } + Metrics.BlockAveGasPrice = (Metrics.BlockAveGasPrice * Metrics.BlockTransactions + gasPrice) / (Metrics.BlockTransactions + 1); + Metrics.BlockEstMedianGasPrice += Metrics.BlockAveGasPrice * 0.01f * float.Sign(gasPrice - Metrics.BlockEstMedianGasPrice); + Metrics.BlockTransactions++; } } From 0554aefc381eb40ecab63bd004799c84a6b767d5 Mon Sep 17 00:00:00 2001 From: Alexey Osipov Date: Fri, 21 Jun 2024 12:59:15 +0300 Subject: [PATCH 4/8] Early return --- src/Nethermind/Nethermind.Evm/AddressExtensions.cs | 2 +- .../TransactionProcessing/TransactionProcessor.cs | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Nethermind/Nethermind.Evm/AddressExtensions.cs b/src/Nethermind/Nethermind.Evm/AddressExtensions.cs index e2e68f71181..85bd9f7d4d7 100644 --- a/src/Nethermind/Nethermind.Evm/AddressExtensions.cs +++ b/src/Nethermind/Nethermind.Evm/AddressExtensions.cs @@ -40,7 +40,7 @@ public static Address From(Address deployingAddress, ReadOnlySpan salt, Re } - /// See https://eips.ethereum.org/EIPS/eip-7610 + // See https://eips.ethereum.org/EIPS/eip-7610 public static bool IsNonZeroAccount(this Address contractAddress, IReleaseSpec spec, ICodeInfoRepository codeInfoRepository, IWorldState state) { return codeInfoRepository.GetCachedCodeInfo(state, contractAddress, spec).MachineCode.Length != 0 || diff --git a/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs b/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs index 9c22348b6b5..a4367ad17f3 100644 --- a/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs +++ b/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs @@ -185,10 +185,9 @@ protected virtual TransactionResult Execute(Transaction tx, in BlockExecutionCon private static void UpdateMetrics(ExecutionOptions opts, UInt256 effectiveGasPrice) { - if (opts is ExecutionOptions.Commit or ExecutionOptions.None) + if (opts is ExecutionOptions.Commit or ExecutionOptions.None && (effectiveGasPrice[2] | effectiveGasPrice[3]) == 0) { - // log2(3e38) ~ 127.8, if effectiveGasPrice is more than 128bit long it may lead to an overflow - float gasPrice = effectiveGasPrice[2] != 0 || effectiveGasPrice[3] != 0 ? 3e29f : (float)((double)effectiveGasPrice / 1_000_000_000.0); + float gasPrice = (float)((double)effectiveGasPrice / 1_000_000_000.0); Metrics.MinGasPrice = Math.Min(gasPrice, Metrics.MinGasPrice); Metrics.MaxGasPrice = Math.Max(gasPrice, Metrics.MaxGasPrice); @@ -584,10 +583,7 @@ protected void PrepareAccountForContractDeployment(Address contractAddress, IRel { if (WorldState.AccountExists(contractAddress) && contractAddress.IsNonZeroAccount(spec, _codeInfoRepository, WorldState)) { - if (Logger.IsTrace) - { - Logger.Trace($"Contract collision at {contractAddress}"); - } + if (Logger.IsTrace) Logger.Trace($"Contract collision at {contractAddress}"); ThrowTransactionCollisionException(); } From 1f96c33589f70dfa3e43c69a113c04ef87ef844e Mon Sep 17 00:00:00 2001 From: Alexey Osipov Date: Fri, 21 Jun 2024 18:52:30 +0300 Subject: [PATCH 5/8] Fix whitespace --- src/Nethermind/Nethermind.Evm/AddressExtensions.cs | 1 - src/Nethermind/Nethermind.Evm/VirtualMachine.cs | 7 ------- 2 files changed, 8 deletions(-) diff --git a/src/Nethermind/Nethermind.Evm/AddressExtensions.cs b/src/Nethermind/Nethermind.Evm/AddressExtensions.cs index 85bd9f7d4d7..d19f03056a5 100644 --- a/src/Nethermind/Nethermind.Evm/AddressExtensions.cs +++ b/src/Nethermind/Nethermind.Evm/AddressExtensions.cs @@ -39,7 +39,6 @@ public static Address From(Address deployingAddress, ReadOnlySpan salt, Re return new Address(in contractAddressKeccak); } - // See https://eips.ethereum.org/EIPS/eip-7610 public static bool IsNonZeroAccount(this Address contractAddress, IReleaseSpec spec, ICodeInfoRepository codeInfoRepository, IWorldState state) { diff --git a/src/Nethermind/Nethermind.Evm/VirtualMachine.cs b/src/Nethermind/Nethermind.Evm/VirtualMachine.cs index 0f1bf70241b..01e891b07f5 100644 --- a/src/Nethermind/Nethermind.Evm/VirtualMachine.cs +++ b/src/Nethermind/Nethermind.Evm/VirtualMachine.cs @@ -2452,13 +2452,6 @@ private EvmExceptionType InstructionSelfDestruct(EvmState vmState, ref return (EvmExceptionType.None, callState); } - private static bool IsNonEmptyAccount(IReleaseSpec spec, Address contractAddress, ICodeInfoRepository _codeInfoRepository, IWorldState state) - { - return _codeInfoRepository.GetCachedCodeInfo(state, contractAddress, spec).MachineCode.Length != 0 || - state.GetNonce(contractAddress) != 0 || - state.GetStorageRoot(contractAddress) != Keccak.EmptyTreeHash; - } - [SkipLocalsInit] private EvmExceptionType InstructionLog(EvmState vmState, ref EvmStack stack, ref long gasAvailable, Instruction instruction) where TTracing : struct, IIsTracing From ad2d98b647639327ddb49db27fbbfe45dc17a266 Mon Sep 17 00:00:00 2001 From: Alexey Osipov Date: Fri, 21 Jun 2024 19:33:34 +0300 Subject: [PATCH 6/8] Return Eip2537Tests --- .../Ethereum.Blockchain.Test/Eip2537Tests.cs | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/Nethermind/Ethereum.Blockchain.Test/Eip2537Tests.cs b/src/Nethermind/Ethereum.Blockchain.Test/Eip2537Tests.cs index 0127e687e11..23fe74ca1a3 100644 --- a/src/Nethermind/Ethereum.Blockchain.Test/Eip2537Tests.cs +++ b/src/Nethermind/Ethereum.Blockchain.Test/Eip2537Tests.cs @@ -1,26 +1,26 @@ // SPDX-FileCopyrightText: 2024 Demerzel Solutions Limited // SPDX-License-Identifier: LGPL-3.0-only -// using System.Collections.Generic; -// using Ethereum.Test.Base; -// using NUnit.Framework; +using System.Collections.Generic; +using Ethereum.Test.Base; +using NUnit.Framework; -// namespace Ethereum.Blockchain.Test -// { -// [TestFixture] -// [Parallelizable(ParallelScope.All)] -// public class Eip2537Tests : GeneralStateTestBase -// { -// [TestCaseSource(nameof(LoadTests))] -// public void Test(GeneralStateTest test) -// { -// Assert.True(RunTest(test).Pass); -// } +namespace Ethereum.Blockchain.Test +{ + [TestFixture] + [Parallelizable(ParallelScope.All)] + public class Eip2537Tests : GeneralStateTestBase + { + [TestCaseSource(nameof(LoadTests))] + public void Test(GeneralStateTest test) + { + Assert.True(RunTest(test).Pass); + } -// public static IEnumerable LoadTests() -// { -// var loader = new TestsSourceLoader(new LoadGeneralStateTestsStrategy(), "../EIPTests/StateTests/stEIP2537"); -// return (IEnumerable)loader.LoadTests(); -// } -// } -// } + public static IEnumerable LoadTests() + { + var loader = new TestsSourceLoader(new LoadGeneralStateTestsStrategy(), "../EIPTests/StateTests/stEIP2537"); + return (IEnumerable)loader.LoadTests(); + } + } +} From 73dc77e886615b73d9c46051f4d054f531a95393 Mon Sep 17 00:00:00 2001 From: Alexey Osipov Date: Mon, 24 Jun 2024 14:51:55 +0300 Subject: [PATCH 7/8] Wait an async --- src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs b/src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs index 37f363431e6..e9055e9a4d4 100644 --- a/src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs +++ b/src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs @@ -109,7 +109,7 @@ protected EthereumTestResult RunTest(GeneralStateTest test, ITxTracer txTracer) Stopwatch stopwatch = Stopwatch.StartNew(); IReleaseSpec? spec = specProvider.GetSpec((ForkActivation)test.CurrentNumber); - if (spec is Cancun) KzgPolynomialCommitments.InitializeAsync(); + if (spec is Cancun) KzgPolynomialCommitments.InitializeAsync().Wait(); if (test.Transaction.ChainId is null) test.Transaction.ChainId = MainnetSpecProvider.Instance.ChainId; From 42d7d07c107cb0bf9309f113486125bfb192f730 Mon Sep 17 00:00:00 2001 From: Alexey Osipov Date: Mon, 24 Jun 2024 17:22:41 +0300 Subject: [PATCH 8/8] Move async; clean up a bit --- .../Ethereum.Test.Base/GeneralTestBase.cs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs b/src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs index e9055e9a4d4..e583a78b423 100644 --- a/src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs +++ b/src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs @@ -25,6 +25,7 @@ using Nethermind.State; using Nethermind.Trie.Pruning; using NUnit.Framework; +using System.Threading.Tasks; namespace Ethereum.Test.Base { @@ -32,7 +33,7 @@ public abstract class GeneralStateTestBase { private static ILogger _logger = new(new ConsoleAsyncLogger(LogLevel.Info)); private static ILogManager _logManager = LimboLogs.Instance; - private static UInt256 _defaultBaseFeeForStateTest = 0xA; + private static readonly UInt256 _defaultBaseFeeForStateTest = 0xA; private readonly TxValidator _txValidator = new(MainnetSpecProvider.Instance.ChainId); [SetUp] @@ -40,7 +41,10 @@ public void Setup() { } - protected void Setup(ILogManager logManager) + [OneTimeSetUp] + public Task OneTimeSetUp() => KzgPolynomialCommitments.InitializeAsync(); + + protected static void Setup(ILogManager logManager) { _logManager = logManager ?? LimboLogs.Instance; _logger = _logManager.GetClassLogger(); @@ -95,7 +99,7 @@ protected EthereumTestResult RunTest(GeneralStateTest test, ITxTracer txTracer) test.CurrentNumber, test.CurrentGasLimit, test.CurrentTimestamp, - Array.Empty()); + []); header.BaseFeePerGas = test.Fork.IsEip1559Enabled ? test.CurrentBaseFee ?? _defaultBaseFeeForStateTest : UInt256.Zero; header.StateRoot = test.PostHash; header.Hash = header.CalculateHash(); @@ -109,8 +113,6 @@ protected EthereumTestResult RunTest(GeneralStateTest test, ITxTracer txTracer) Stopwatch stopwatch = Stopwatch.StartNew(); IReleaseSpec? spec = specProvider.GetSpec((ForkActivation)test.CurrentNumber); - if (spec is Cancun) KzgPolynomialCommitments.InitializeAsync().Wait(); - if (test.Transaction.ChainId is null) test.Transaction.ChainId = MainnetSpecProvider.Instance.ChainId; if (test.ParentBlobGasUsed is not null && test.ParentExcessBlobGas is not null) @@ -123,7 +125,7 @@ protected EthereumTestResult RunTest(GeneralStateTest test, ITxTracer txTracer) number: test.CurrentNumber - 1, gasLimit: test.CurrentGasLimit, timestamp: test.CurrentTimestamp, - extraData: Array.Empty() + extraData: [] ) { BlobGasUsed = (ulong)test.ParentBlobGasUsed, @@ -203,7 +205,7 @@ private bool IsValidBlock(Block block, ISpecProvider specProvider) private List RunAssertions(GeneralStateTest test, IWorldState stateProvider) { - List differences = new(); + List differences = []; if (test.PostHash != stateProvider.StateRoot) { differences.Add($"STATE ROOT exp: {test.PostHash}, actual: {stateProvider.StateRoot}");