From a99c95c4e05e431bd30a60cb39bbeedb5e74119f Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Wed, 31 Jan 2024 14:20:20 +0000 Subject: [PATCH] Clean unitialized memory when passing it to tracer (#6640) --- .../EvmPooledMemoryTests.cs | 266 +++++++++++++++++- .../Nethermind.Evm/EvmPooledMemory.cs | 56 ++-- .../Nethermind.Evm/VirtualMachine.cs | 4 +- 3 files changed, 306 insertions(+), 20 deletions(-) diff --git a/src/Nethermind/Nethermind.Evm.Test/EvmPooledMemoryTests.cs b/src/Nethermind/Nethermind.Evm.Test/EvmPooledMemoryTests.cs index 0585f41925a..288e0fdaab2 100644 --- a/src/Nethermind/Nethermind.Evm.Test/EvmPooledMemoryTests.cs +++ b/src/Nethermind/Nethermind.Evm.Test/EvmPooledMemoryTests.cs @@ -2,9 +2,24 @@ // SPDX-License-Identifier: LGPL-3.0-only using System; -using FluentAssertions; +using System.Collections.Generic; +using System.Linq; +using Nethermind.Core; +using Nethermind.Core.Crypto; +using Nethermind.Core.Specs; using Nethermind.Core.Test.Builders; +using Nethermind.Crypto; +using Nethermind.Db; +using Nethermind.Evm.Tracing; +using Nethermind.Evm.TransactionProcessing; using Nethermind.Int256; +using Nethermind.Logging; +using Nethermind.Specs; +using Nethermind.Specs.Forks; +using Nethermind.State; +using Nethermind.Trie.Pruning; + +using FluentAssertions; using NUnit.Framework; namespace Nethermind.Evm.Test @@ -89,11 +104,258 @@ public void Load_should_update_size_of_memory() } [Test] - public void GetTrace_should_not_thor_on_not_initialized_memory() + public void GetTrace_should_not_throw_on_not_initialized_memory() { EvmPooledMemory memory = new(); memory.CalculateMemoryCost(0, 32); memory.GetTrace().ToHexWordList().Should().BeEquivalentTo(new string[] { "0000000000000000000000000000000000000000000000000000000000000000" }); } + + [Test] + public void GetTrace_memory_should_not_bleed_between_txs() + { + var first = new byte[] { + 0x5b, 0x38, 0x36, 0x59, 0x59, 0x59, 0x59, 0x52, 0x3a, 0x60, 0x05, 0x30, + 0xf4, 0x05, 0x56}; + var second = new byte[] { + 0x5b, 0x36, 0x59, 0x3a, 0x34, 0x60, 0x5b, 0x59, 0x05, 0x30, 0xf4, 0x3a, + 0x56}; + + var a = run(second).ToString(); + run(first); + var b = run(second).ToString(); + + Assert.That(b, Is.EqualTo(a)); + } + + private static PrivateKey PrivateKeyD = new("0000000000000000000000000000000000000000000000000000001000000000"); + private static Address sender = new Address("0x59ede65f910076f60e07b2aeb189c72348525e72"); + + private static Address to = new Address("0x000000000000000000000000636f6e7472616374"); + private static Address coinbase = new Address("0x4444588443C3a91288c5002483449Aba1054192b"); + private static readonly EthereumEcdsa ethereumEcdsa = new(BlockchainIds.Goerli, LimboLogs.Instance); + private static string run(byte[] input) + { + long blocknr = 12965000; + long gas = 34218; + ulong ts = 123456; + MemDb stateDb = new(); + TrieStore trieStore = new( + stateDb, + LimboLogs.Instance); + IWorldState stateProvider = new WorldState( + trieStore, + new MemDb(), + LimboLogs.Instance); + ISpecProvider specProvider = new TestSpecProvider(London.Instance); + VirtualMachine virtualMachine = new( + Nethermind.Evm.Test.TestBlockhashProvider.Instance, + specProvider, + LimboLogs.Instance); + TransactionProcessor transactionProcessor = new TransactionProcessor( + specProvider, + stateProvider, + virtualMachine, + LimboLogs.Instance); + + stateProvider.CreateAccount(to, 123); + stateProvider.InsertCode(to, input, specProvider.GenesisSpec); + + stateProvider.CreateAccount(sender, 40000000); + stateProvider.Commit(specProvider.GenesisSpec); + + stateProvider.CommitTree(0); + + Transaction tx = Build.A.Transaction. + WithData(input). + WithTo(to). + WithGasLimit(gas). + WithGasPrice(0). + WithValue(0). + SignedAndResolved(ethereumEcdsa, PrivateKeyD, true). + TestObject; + Block block = Build.A.Block. + WithBeneficiary(coinbase). + WithNumber(blocknr + 1). + WithTimestamp(ts). + WithTransactions(tx). + WithGasLimit(30000000). + WithDifficulty(0). + TestObject; + MyTracer tracer = new(); + transactionProcessor.Execute( + tx, + new BlockExecutionContext(block.Header), + tracer); + return tracer.lastmemline; + } + } + + public class MyTracer : ITxTracer, IDisposable + { + public bool IsTracingReceipt => true; + public bool IsTracingActions => false; + public bool IsTracingOpLevelStorage => true; + public bool IsTracingMemory => true; + public bool IsTracingDetailedMemory { get; set; } = true; + public bool IsTracingInstructions => true; + public bool IsTracingRefunds { get; } = false; + public bool IsTracingCode => true; + public bool IsTracingStack { get; set; } = true; + public bool IsTracingState => false; + public bool IsTracingStorage => false; + public bool IsTracingBlockHash { get; } = false; + public bool IsTracingAccess { get; } = false; + public bool IsTracingFees => false; + public bool IsTracing => IsTracingReceipt || IsTracingActions || IsTracingOpLevelStorage || IsTracingMemory || IsTracingInstructions || IsTracingRefunds || IsTracingCode || IsTracingStack || IsTracingBlockHash || IsTracingAccess || IsTracingFees; + + public string lastmemline; + + public void MarkAsSuccess(Address recipient, long gasSpent, byte[] output, LogEntry[] logs, Hash256 stateRoot = null) + { + } + + public void MarkAsFailed(Address recipient, long gasSpent, byte[] output, string error, Hash256 stateRoot = null) + { + } + + public void StartOperation(int depth, long gas, Instruction opcode, int pc, bool isPostMerge = false) + { + } + + public void ReportOperationError(EvmExceptionType error) + { + } + + public void ReportOperationRemainingGas(long gas) + { + } + + public void SetOperationStack(TraceStack stack) + { + } + + public void SetOperationMemory(TraceMemory memoryTrace) + { + lastmemline = string.Concat("0x", string.Join("", memoryTrace.ToHexWordList().Select(mt => mt.Replace("0x", string.Empty)))); + } + + public void SetOperationMemorySize(ulong newSize) + { + } + + public void ReportMemoryChange(long offset, in ReadOnlySpan data) + { + } + + public void ReportStorageChange(in ReadOnlySpan key, in ReadOnlySpan value) + { + } + + public void SetOperationStorage(Address address, UInt256 storageIndex, ReadOnlySpan newValue, ReadOnlySpan currentValue) + { + } + + public void LoadOperationStorage(Address address, UInt256 storageIndex, ReadOnlySpan value) + { + } + + public void ReportSelfDestruct(Address address, UInt256 balance, Address refundAddress) + { + throw new NotSupportedException(); + } + + public void ReportBalanceChange(Address address, UInt256? before, UInt256? after) + { + throw new NotSupportedException(); + } + + public void ReportCodeChange(Address address, byte[] before, byte[] after) + { + throw new NotSupportedException(); + } + + public void ReportNonceChange(Address address, UInt256? before, UInt256? after) + { + throw new NotSupportedException(); + } + + public void ReportAccountRead(Address address) + { + throw new NotImplementedException(); + } + + public void ReportStorageChange(in StorageCell storageAddress, byte[] before, byte[] after) + { + throw new NotSupportedException(); + } + + public void ReportStorageRead(in StorageCell storageCell) + { + throw new NotImplementedException(); + } + + public void ReportAction(long gas, UInt256 value, Address @from, Address to, ReadOnlyMemory input, ExecutionType callType, bool isPrecompileCall = false) + { + throw new NotSupportedException(); + } + + public void ReportActionEnd(long gas, ReadOnlyMemory output) + { + throw new NotSupportedException(); + } + + public void ReportActionError(EvmExceptionType exceptionType) + { + throw new NotSupportedException(); + } + + public void ReportActionEnd(long gas, Address deploymentAddress, ReadOnlyMemory deployedCode) + { + throw new NotSupportedException(); + } + + public void ReportBlockHash(Hash256 blockHash) + { + throw new NotImplementedException(); + } + + public void ReportByteCode(ReadOnlyMemory byteCode) + { + throw new NotSupportedException(); + } + + public void ReportGasUpdateForVmTrace(long refund, long gasAvailable) + { + } + + public void ReportRefundForVmTrace(long refund, long gasAvailable) + { + } + + public void ReportRefund(long refund) + { + } + + public void ReportExtraGasPressure(long extraGasPressure) + { + throw new NotImplementedException(); + } + + public void ReportAccess(IReadOnlySet
accessedAddresses, IReadOnlySet accessedStorageCells) + { + throw new NotImplementedException(); + } + + public void ReportStackPush(in ReadOnlySpan stackItem) + { + } + + public void ReportFees(UInt256 fees, UInt256 burntFees) + { + throw new NotImplementedException(); + } + + public void Dispose() { } } } diff --git a/src/Nethermind/Nethermind.Evm/EvmPooledMemory.cs b/src/Nethermind/Nethermind.Evm/EvmPooledMemory.cs index 2173717d029..446ae6f87bc 100644 --- a/src/Nethermind/Nethermind.Evm/EvmPooledMemory.cs +++ b/src/Nethermind/Nethermind.Evm/EvmPooledMemory.cs @@ -2,14 +2,12 @@ // SPDX-License-Identifier: LGPL-3.0-only using System; -using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.Intrinsics; using Nethermind.Core.Buffers; -using Nethermind.Core.Extensions; using Nethermind.Evm.Tracing; using Nethermind.Int256; @@ -21,7 +19,7 @@ public class EvmPooledMemory : IEvmMemory private static readonly LargerArrayPool Pool = LargerArrayPool.Shared; - private int _lastZeroedSize; + private ulong _lastZeroedSize; private byte[]? _memory; public ulong Length { get; private set; } @@ -175,14 +173,31 @@ public ReadOnlyMemory Inspect(in UInt256 location, in UInt256 length) return new byte[(long)length]; } - if (_memory is null || location + length > _memory.Length) + if (_memory is null) { return default; } + UInt256 largeSize = location + length; + if (largeSize > _memory.Length) + { + return default; + } + + ClearForTracing((ulong)largeSize); return _memory.AsMemory((int)location, (int)length); } + private void ClearForTracing(ulong size) + { + if (_memory is not null && size > _lastZeroedSize) + { + int lengthToClear = (int)(Math.Min(size, (ulong)_memory.Length) - _lastZeroedSize); + Array.Clear(_memory, (int)_lastZeroedSize, lengthToClear); + _lastZeroedSize = size; + } + } + public long CalculateMemoryCost(in UInt256 location, in UInt256 length) { if (length.IsZero) @@ -216,7 +231,12 @@ public long CalculateMemoryCost(in UInt256 location, in UInt256 length) return 0L; } - public TraceMemory GetTrace() => new(Size, _memory ??= Array.Empty()); + public TraceMemory GetTrace() + { + ulong size = Size; + ClearForTracing(size); + return new(size, _memory); + } public void Dispose() { @@ -272,20 +292,24 @@ private void UpdateSize(ulong length, bool rentIfNeeded = true) _memory = Pool.Rent((int)Size); Array.Clear(_memory, 0, (int)Size); } - else if (Size > (ulong)_memory.LongLength) - { - byte[] beforeResize = _memory; - _memory = Pool.Rent((int)Size); - Array.Copy(beforeResize, 0, _memory, 0, _lastZeroedSize); - Array.Clear(_memory, _lastZeroedSize, (int)Size - _lastZeroedSize); - Pool.Return(beforeResize); - } - else if (Size > (ulong)_lastZeroedSize) + else { - Array.Clear(_memory, _lastZeroedSize, (int)Size - _lastZeroedSize); + int lastZeroedSize = (int)_lastZeroedSize; + if (Size > (ulong)_memory.LongLength) + { + byte[] beforeResize = _memory; + _memory = Pool.Rent((int)Size); + Array.Copy(beforeResize, 0, _memory, 0, lastZeroedSize); + Array.Clear(_memory, lastZeroedSize, (int)(Size - _lastZeroedSize)); + Pool.Return(beforeResize); + } + else if (Size > _lastZeroedSize) + { + Array.Clear(_memory, lastZeroedSize, (int)(Size - _lastZeroedSize)); + } } - _lastZeroedSize = (int)Size; + _lastZeroedSize = Size; } } diff --git a/src/Nethermind/Nethermind.Evm/VirtualMachine.cs b/src/Nethermind/Nethermind.Evm/VirtualMachine.cs index e6419803328..df4a4bbf61f 100644 --- a/src/Nethermind/Nethermind.Evm/VirtualMachine.cs +++ b/src/Nethermind/Nethermind.Evm/VirtualMachine.cs @@ -2302,8 +2302,8 @@ private EvmExceptionType InstructionCall( if (typeof(TTracingInstructions) == typeof(IsTracing)) { // very specific for Parity trace, need to find generalization - very peculiar 32 length... - ReadOnlyMemory memoryTrace = vmState.Memory.Inspect(in dataOffset, 32); - _txTracer.ReportMemoryChange(dataOffset, memoryTrace.Span); + ReadOnlyMemory? memoryTrace = vmState.Memory?.Inspect(in dataOffset, 32); + _txTracer.ReportMemoryChange(dataOffset, memoryTrace is null ? ReadOnlySpan.Empty : memoryTrace.Value.Span); } if (typeof(TLogger) == typeof(IsTracing)) _logger.Trace("FAIL - call depth");