diff --git a/src/Nethermind/Nethermind.Consensus/Processing/BlockProcessor.cs b/src/Nethermind/Nethermind.Consensus/Processing/BlockProcessor.cs index 365f1bef5fa..9170e9a8758 100644 --- a/src/Nethermind/Nethermind.Consensus/Processing/BlockProcessor.cs +++ b/src/Nethermind/Nethermind.Consensus/Processing/BlockProcessor.cs @@ -235,7 +235,7 @@ protected virtual TxReceipt[] ProcessBlock( _beaconBlockRootHandler.ApplyContractStateChanges(block, spec, _stateProvider); _blockhashStore.ApplyHistoryBlockHashes(block.Header); - _stateProvider.Commit(spec); + _stateProvider.Commit(spec, commitStorageRoots: false); TxReceipt[] receipts = _blockTransactionsExecutor.ProcessTransactions(block, options, ReceiptsTracer, spec); @@ -249,7 +249,7 @@ protected virtual TxReceipt[] ProcessBlock( _withdrawalProcessor.ProcessWithdrawals(block, spec); ReceiptsTracer.EndBlockTrace(); - _stateProvider.Commit(spec); + _stateProvider.Commit(spec, commitStorageRoots: true); if (ShouldComputeStateRoot(block.Header)) { diff --git a/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs b/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs index 4b97a503590..4c98d6eee6e 100644 --- a/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs +++ b/src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs @@ -125,7 +125,7 @@ protected virtual TransactionResult Execute(Transaction tx, in BlockExecutionCon if (!(result = BuyGas(tx, header, spec, tracer, opts, effectiveGasPrice, out UInt256 premiumPerGas, out UInt256 senderReservedGasPayment))) return result; if (!(result = IncrementNonce(tx, header, spec, tracer, opts))) return result; - if (commit) WorldState.Commit(spec, tracer.IsTracingState ? tracer : NullTxTracer.Instance); + if (commit) WorldState.Commit(spec, tracer.IsTracingState ? tracer : NullTxTracer.Instance, commitStorageRoots: false); ExecutionEnvironment env = BuildExecutionEnvironment(tx, in blCtx, spec, effectiveGasPrice); @@ -153,7 +153,7 @@ protected virtual TransactionResult Execute(Transaction tx, in BlockExecutionCon } else if (commit) { - WorldState.Commit(spec, tracer.IsTracingState ? tracer : NullStateTracer.Instance); + WorldState.Commit(spec, tracer.IsTracingState ? tracer : NullStateTracer.Instance, commitStorageRoots: !spec.IsEip658Enabled); } if (tracer.IsTracingReceipt) diff --git a/src/Nethermind/Nethermind.State/IWorldState.cs b/src/Nethermind/Nethermind.State/IWorldState.cs index b2ffdfc812f..f0c8f82ff73 100644 --- a/src/Nethermind/Nethermind.State/IWorldState.cs +++ b/src/Nethermind/Nethermind.State/IWorldState.cs @@ -99,9 +99,9 @@ public interface IWorldState : IJournal, IReadOnlyStateProvider /* snapshots */ - void Commit(IReleaseSpec releaseSpec, bool isGenesis = false); + void Commit(IReleaseSpec releaseSpec, bool isGenesis = false, bool commitStorageRoots = true); - void Commit(IReleaseSpec releaseSpec, IWorldStateTracer? traver, bool isGenesis = false); + void Commit(IReleaseSpec releaseSpec, IWorldStateTracer? tracer, bool isGenesis = false, bool commitStorageRoots = true); void CommitTree(long blockNumber); } diff --git a/src/Nethermind/Nethermind.State/PartialStorageProviderBase.cs b/src/Nethermind/Nethermind.State/PartialStorageProviderBase.cs index e3e4bc676c8..20af7e01c72 100644 --- a/src/Nethermind/Nethermind.State/PartialStorageProviderBase.cs +++ b/src/Nethermind/Nethermind.State/PartialStorageProviderBase.cs @@ -145,9 +145,9 @@ public void Restore(int snapshot) /// /// Commit persistent storage /// - public void Commit() + public void Commit(bool commitStorageRoots = true) { - Commit(NullStateTracer.Instance); + Commit(NullStateTracer.Instance, commitStorageRoots); } protected readonly struct ChangeTrace @@ -172,7 +172,7 @@ public ChangeTrace(byte[]? after) /// Commit persistent storage /// /// State tracer - public void Commit(IStorageTracer tracer) + public void Commit(IStorageTracer tracer, bool commitStorageRoots = true) { if (_currentPosition == Snapshot.EmptyPosition) { @@ -182,6 +182,16 @@ public void Commit(IStorageTracer tracer) { CommitCore(tracer); } + + if (commitStorageRoots) + { + CommitStorageRoots(); + } + } + + protected virtual void CommitStorageRoots() + { + // Commit storage roots } /// diff --git a/src/Nethermind/Nethermind.State/PersistentStorageProvider.cs b/src/Nethermind/Nethermind.State/PersistentStorageProvider.cs index 820655d53b8..c6abed3a839 100644 --- a/src/Nethermind/Nethermind.State/PersistentStorageProvider.cs +++ b/src/Nethermind/Nethermind.State/PersistentStorageProvider.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Threading.Tasks; using Nethermind.Core; using Nethermind.Core.Collections; using Nethermind.Core.Crypto; @@ -25,6 +26,7 @@ internal class PersistentStorageProvider : PartialStorageProviderBase private readonly ILogManager? _logManager; internal readonly IStorageTreeFactory _storageTreeFactory; private readonly ResettableDictionary _storages = new(); + private readonly HashSet
_toUpdateRoots = new(); /// /// EIP-1283 @@ -53,6 +55,7 @@ public override void Reset() _storages.Reset(); _originalValues.Clear(); _committedThisRound.Clear(); + _toUpdateRoots.Clear(); } /// @@ -179,16 +182,17 @@ protected override void CommitCore(IStorageTracer tracer) } } - // TODO: it seems that we are unnecessarily recalculating root hashes all the time in storage? foreach (Address address in toUpdateRoots) { // since the accounts could be empty accounts that are removing (EIP-158) if (_stateProvider.AccountExists(address)) { - Hash256 root = RecalculateRootHash(address); - - // _logger.Warn($"Recalculating storage root {address}->{root} ({toUpdateRoots.Count})"); - _stateProvider.UpdateStorageRoot(address, root); + _toUpdateRoots.Add(address); + } + else + { + _toUpdateRoots.Remove(address); + _storages.Remove(address); } } @@ -202,6 +206,68 @@ protected override void CommitCore(IStorageTracer tracer) } } + protected override void CommitStorageRoots() + { + if (_toUpdateRoots.Count == 0) + { + return; + } + + // Is overhead of parallel foreach worth it? + if (_toUpdateRoots.Count <= 4) + { + UpdateRootHashesSingleThread(); + } + else + { + UpdateRootHashesMultiThread(); + } + + void UpdateRootHashesSingleThread() + { + foreach (KeyValuePair kvp in _storages) + { + if (!_toUpdateRoots.Contains(kvp.Key)) + { + // Wasn't updated don't recalculate + continue; + } + + StorageTree storageTree = kvp.Value; + storageTree.UpdateRootHash(); + _stateProvider.UpdateStorageRoot(address: kvp.Key, storageTree.RootHash); + } + } + + void UpdateRootHashesMultiThread() + { + // We can recalculate the roots in parallel as they are all independent tries + Parallel.ForEach(_storages, kvp => + { + if (!_toUpdateRoots.Contains(kvp.Key)) + { + // Wasn't updated don't recalculate + return; + } + StorageTree storageTree = kvp.Value; + storageTree.UpdateRootHash(); + }); + + // Update the storage roots in the main thread non in parallel + foreach (KeyValuePair kvp in _storages) + { + if (!_toUpdateRoots.Contains(kvp.Key)) + { + continue; + } + + // Update the storage root for the Account + _stateProvider.UpdateStorageRoot(address: kvp.Key, kvp.Value.RootHash); + } + + } + } + private void SaveToTree(HashSet
toUpdateRoots, Change change) { if (_originalValues.TryGetValue(change.StorageCell, out byte[] initialValue) && @@ -223,14 +289,16 @@ private void SaveToTree(HashSet
toUpdateRoots, Change change) /// Current block number public void CommitTrees(long blockNumber) { - // _logger.Warn($"Storage block commit {blockNumber}"); foreach (KeyValuePair storage in _storages) { + if (!_toUpdateRoots.Contains(storage.Key)) + { + continue; + } storage.Value.Commit(blockNumber); } - // TODO: maybe I could update storage roots only now? - + _toUpdateRoots.Clear(); // only needed here as there is no control over cached storage size otherwise _storages.Reset(); } @@ -305,6 +373,7 @@ public override void ClearStorage(Address address) // by means of CREATE 2 - notice that the cached trie may carry information about items that were not // touched in this block, hence were not zeroed above // TODO: how does it work with pruning? + _toUpdateRoots.Remove(address); _storages[address] = new StorageTree(_trieStore.GetTrieStore(address.ToAccountPath), Keccak.EmptyTreeHash, _logManager); } diff --git a/src/Nethermind/Nethermind.State/WorldState.cs b/src/Nethermind/Nethermind.State/WorldState.cs index bb88cb42eff..c55ec0cc0e6 100644 --- a/src/Nethermind/Nethermind.State/WorldState.cs +++ b/src/Nethermind/Nethermind.State/WorldState.cs @@ -193,16 +193,16 @@ public bool HasStateForRoot(Hash256 stateRoot) return _trieStore.HasRoot(stateRoot); } - public void Commit(IReleaseSpec releaseSpec, bool isGenesis = false) + public void Commit(IReleaseSpec releaseSpec, bool isGenesis = false, bool commitStorageRoots = true) { - _persistentStorageProvider.Commit(); - _transientStorageProvider.Commit(); + _persistentStorageProvider.Commit(commitStorageRoots); + _transientStorageProvider.Commit(commitStorageRoots); _stateProvider.Commit(releaseSpec, isGenesis); } - public void Commit(IReleaseSpec releaseSpec, IWorldStateTracer tracer, bool isGenesis = false) + public void Commit(IReleaseSpec releaseSpec, IWorldStateTracer tracer, bool isGenesis = false, bool commitStorageRoots = true) { - _persistentStorageProvider.Commit(tracer); - _transientStorageProvider.Commit(tracer); + _persistentStorageProvider.Commit(tracer, commitStorageRoots); + _transientStorageProvider.Commit(tracer, commitStorageRoots); _stateProvider.Commit(releaseSpec, tracer, isGenesis); }