-
Notifications
You must be signed in to change notification settings - Fork 690
feat(rpc): add debug_intermediateRoots #11524
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
206f0ed
fc08566
a160e9f
5d39af7
4b194fb
598cf9b
8258b4f
b3290a8
74b08eb
8dac374
399f368
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,49 @@ | ||||||
| // SPDX-FileCopyrightText: 2026 Demerzel Solutions Limited | ||||||
| // SPDX-License-Identifier: LGPL-3.0-only | ||||||
|
|
||||||
| using Nethermind.Core; | ||||||
| using Nethermind.Core.Crypto; | ||||||
| using Nethermind.Evm.State; | ||||||
| using Nethermind.Evm.Tracing; | ||||||
| using Nethermind.Evm.TransactionProcessing; | ||||||
|
|
||||||
| namespace Nethermind.Blockchain.Tracing; | ||||||
|
|
||||||
| /// <summary> | ||||||
| /// Captures the world state root after each transaction in a block. | ||||||
| /// Mirrors geth's <c>debug_intermediateRoots</c> behaviour. | ||||||
| /// </summary> | ||||||
| public class IntermediateRootsBlockTracer(IWorldState worldState) | ||||||
| : BlockTracerBase<Hash256, IntermediateRootsBlockTracer.IntermediateRootsTxTracer> | ||||||
| { | ||||||
| protected override IntermediateRootsTxTracer OnStart(Transaction? tx) => new(worldState); | ||||||
|
|
||||||
| protected override Hash256 OnEnd(IntermediateRootsTxTracer txTracer) => txTracer.StateRoot; | ||||||
|
|
||||||
| public class IntermediateRootsTxTracer(IWorldState worldState) : TxTracer | ||||||
| { | ||||||
| public override bool IsTracingReceipt => true; | ||||||
|
|
||||||
| public Hash256 StateRoot { get; private set; } = Keccak.Zero; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Low — Dead initializer
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gone in |
||||||
|
|
||||||
| public override void MarkAsSuccess(Address recipient, in GasConsumed gasSpent, byte[] output, LogEntry[] logs, Hash256? stateRoot = null) => | ||||||
| Capture(stateRoot); | ||||||
|
|
||||||
| public override void MarkAsFailed(Address recipient, in GasConsumed gasSpent, byte[] output, string? error, Hash256? stateRoot = null) => | ||||||
| Capture(stateRoot); | ||||||
|
Comment on lines
+36
to
+40
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Medium — Partial-result semantics unspecified and untested If a transaction fails mid-block (e.g. an OOG reverted tx),
Geth returns
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Documented the behaviour in the tracer's XML |
||||||
|
|
||||||
| private void Capture(Hash256? reportedStateRoot) | ||||||
| { | ||||||
| if (reportedStateRoot is not null) | ||||||
| { | ||||||
| StateRoot = reportedStateRoot; | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| // Post-Byzantium (EIP-658) the processor does not pre-compute the root, | ||||||
| // so we force a recalculation to read it from the live world state. | ||||||
| worldState.RecalculateStateRoot(); | ||||||
| StateRoot = worldState.StateRoot; | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |||||||||
| using Nethermind.Evm.State; | ||||||||||
| using Nethermind.State.OverridableEnv; | ||||||||||
| using Nethermind.Evm.Tracing; | ||||||||||
| using Nethermind.Blockchain.Tracing; | ||||||||||
| using Nethermind.Blockchain.Tracing.GethStyle; | ||||||||||
| using Nethermind.Blockchain.Tracing.GethStyle.Custom.JavaScript; | ||||||||||
| using Nethermind.Blockchain.Tracing.GethStyle.Custom.Native; | ||||||||||
|
|
@@ -123,6 +124,21 @@ public IReadOnlyCollection<GethLikeTxTrace> TraceBlock(BlockParameter blockParam | |||||||||
|
|
||||||||||
| public IReadOnlyCollection<GethLikeTxTrace> TraceBlock(Block block, GethTraceOptions options, CancellationToken cancellationToken) => TraceBlockImpl(block, options, cancellationToken); | ||||||||||
|
|
||||||||||
| public IReadOnlyCollection<Hash256> TraceBlockIntermediateRoots(Hash256 blockHash, GethTraceOptions options, CancellationToken cancellationToken) | ||||||||||
| { | ||||||||||
| ArgumentNullException.ThrowIfNull(blockHash); | ||||||||||
| ArgumentNullException.ThrowIfNull(options); | ||||||||||
|
|
||||||||||
| Block block = blockTree.FindBlock(blockHash) ?? throw new InvalidOperationException($"Cannot find block {blockHash}"); | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Medium — No bad-block fallback (compat gap vs geth)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added in |
||||||||||
| BlockHeader? parent = FindParent(block); | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. High — Genesis NRE / wrong behaviour
Add a guard before the
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice catch. Added an explicit |
||||||||||
|
|
||||||||||
| using Scope<BlockProcessingComponents> scope = blockProcessingEnv.BuildAndOverride(parent, options.StateOverrides); | ||||||||||
| IntermediateRootsBlockTracer tracer = new(scope.Component.WorldState); | ||||||||||
| scope.Component.BlockchainProcessor.Process(block, ProcessingOptions.Trace, tracer.WithCancellation(cancellationToken), cancellationToken); | ||||||||||
|
|
||||||||||
| return tracer.BuildResult(); | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Low — inconsistent exception-safety pattern All other block processing paths in Suggested fix: IntermediateRootsBlockTracer tracer = new(scope.Component.WorldState);
try
{
scope.Component.BlockchainProcessor.Process(block, ProcessingOptions.Trace, tracer.WithCancellation(cancellationToken), cancellationToken);
return tracer.BuildResult();
}
catch
{
tracer.TryDispose();
throw;
}
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point on the consistency — wrapped it in |
||||||||||
| } | ||||||||||
|
|
||||||||||
| public IEnumerable<string> TraceBlockToFile(Hash256 blockHash, GethTraceOptions options, CancellationToken cancellationToken) | ||||||||||
| { | ||||||||||
| ArgumentNullException.ThrowIfNull(blockHash); | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ public interface IGethStyleTracer | |
| IReadOnlyCollection<GethLikeTxTrace> TraceBlock(BlockParameter blockParameter, GethTraceOptions options, CancellationToken cancellationToken); | ||
| IReadOnlyCollection<GethLikeTxTrace> TraceBlock(Rlp blockRlp, GethTraceOptions options, CancellationToken cancellationToken); | ||
| IReadOnlyCollection<GethLikeTxTrace> TraceBlock(Block block, GethTraceOptions options, CancellationToken cancellationToken); | ||
| IReadOnlyCollection<Hash256> TraceBlockIntermediateRoots(Hash256 blockHash, GethTraceOptions options, CancellationToken cancellationToken); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Low — missing XML doc Per the project's coding style, all public API members should have /// <summary>Replays <paramref name="blockHash"/> and returns the world-state root after each transaction, in execution order.</summary>
IReadOnlyCollection<Hash256> TraceBlockIntermediateRoots(Hash256 blockHash, GethTraceOptions options, CancellationToken cancellationToken);
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added on the interface in |
||
| IEnumerable<string> TraceBlockToFile(Hash256 blockHash, GethTraceOptions options, CancellationToken cancellationToken); | ||
| IEnumerable<string> TraceBadBlockToFile(Hash256 blockHash, GethTraceOptions options, CancellationToken cancellationToken); | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -419,4 +419,39 @@ public void StandardTraceBlockToFile_returns_error_when_state_unavailable(bool i | |||||||||||||||||||
| actual.ErrorCode.Should().Be(ErrorCodes.ResourceUnavailable); | ||||||||||||||||||||
| actual.Result.Error.Should().Contain("No state available"); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| [Test] | ||||||||||||||||||||
| public void Debug_intermediateRoots_returns_post_tx_roots_from_bridge() | ||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can those be collapsed to testcases? |
||||||||||||||||||||
| { | ||||||||||||||||||||
| Hash256 blockHash = TestItem.KeccakA; | ||||||||||||||||||||
| BlockHeader header = Build.A.BlockHeader.WithNumber(1).TestObject; | ||||||||||||||||||||
| _blockFinder.FindHeader(blockHash).Returns(header); | ||||||||||||||||||||
| _blockchainBridge.HasStateForBlock(Arg.Is(header)).Returns(true); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Hash256[] expected = [TestItem.KeccakB, TestItem.KeccakC]; | ||||||||||||||||||||
| _debugBridge | ||||||||||||||||||||
| .GetBlockIntermediateRoots(Arg.Is(blockHash), Arg.Any<CancellationToken>(), Arg.Any<GethTraceOptions?>()) | ||||||||||||||||||||
| .Returns(expected); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| DebugRpcModule rpcModule = CreateDebugRpcModule(_debugBridge); | ||||||||||||||||||||
| ResultWrapper<IReadOnlyCollection<Hash256>> actual = rpcModule.debug_intermediateRoots(blockHash); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| actual.Result.ResultType.Should().Be(ResultType.Success); | ||||||||||||||||||||
| actual.Data.Should().BeEquivalentTo(expected); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| [Test] | ||||||||||||||||||||
| public void Debug_intermediateRoots_fails_when_state_unavailable() | ||||||||||||||||||||
| { | ||||||||||||||||||||
| Hash256 blockHash = TestItem.KeccakA; | ||||||||||||||||||||
| BlockHeader header = Build.A.BlockHeader.WithNumber(1).TestObject; | ||||||||||||||||||||
| _blockFinder.FindHeader(blockHash).Returns(header); | ||||||||||||||||||||
| _blockchainBridge.HasStateForBlock(Arg.Is(header)).Returns(false); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| DebugRpcModule rpcModule = CreateDebugRpcModule(_debugBridge); | ||||||||||||||||||||
| ResultWrapper<IReadOnlyCollection<Hash256>> actual = rpcModule.debug_intermediateRoots(blockHash); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| actual.Result.ResultType.Should().Be(ResultType.Failure); | ||||||||||||||||||||
| actual.ErrorCode.Should().Be(ErrorCodes.ResourceUnavailable); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Medium — missing integration test + Low — missing "block not found" RPC test Two gaps in coverage:
[Test]
public void Debug_intermediateRoots_fails_when_block_not_found()
{
Hash256 blockHash = TestItem.KeccakA;
_blockFinder.FindHeader(blockHash).ReturnsNull();
DebugRpcModule rpcModule = CreateDebugRpcModule(_debugBridge);
ResultWrapper<IReadOnlyCollection<Hash256>> actual = rpcModule.debug_intermediateRoots(blockHash);
actual.Result.ResultType.Should().Be(ResultType.Failure);
actual.ErrorCode.Should().Be(ErrorCodes.ResourceNotFound);
}
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Block-not-found unit test landed in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Medium — Critical test scenarios missing Both current tests mock
At minimum, add a
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Integration test in |
||||||||||||||||||||
| } | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -298,6 +298,31 @@ public ResultWrapper<IReadOnlyCollection<GethLikeTxTrace>> debug_traceBlockByHas | |
| } | ||
| } | ||
|
|
||
| public ResultWrapper<IReadOnlyCollection<Hash256>> debug_intermediateRoots(Hash256 blockHash, GethTraceOptions? options = null) | ||
| { | ||
| TryGetHeaderAndCheckState<IReadOnlyCollection<Hash256>>(blockHash, out ResultWrapper<IReadOnlyCollection<Hash256>>? headerError); | ||
| if (headerError is not null) | ||
| { | ||
| return headerError; | ||
| } | ||
|
|
||
| using CancellationTokenSource? timeout = BuildTimeoutCancellationTokenSource(); | ||
| CancellationToken cancellationToken = timeout.Token; | ||
|
|
||
| try | ||
| { | ||
| IReadOnlyCollection<Hash256> roots = debugBridge.GetBlockIntermediateRoots(blockHash, cancellationToken, options); | ||
|
|
||
| if (_logger.IsTrace) _logger.Trace($"{nameof(debug_intermediateRoots)} request {blockHash}, roots: {roots.Count}"); | ||
|
|
||
| return ResultWrapper<IReadOnlyCollection<Hash256>>.Success(roots); | ||
| } | ||
| catch (InvalidOperationException ex) | ||
| { | ||
| return ResultWrapper<IReadOnlyCollection<Hash256>>.Fail(ex.Message, ErrorCodes.ResourceNotFound); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. High — Overly broad exception mapping
Compare to the existing
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed it was too coarse — dropped the catch in |
||
| } | ||
|
|
||
| public ResultWrapper<GethLikeTxTrace[]> debug_traceBlockFromFile(string fileName, GethTraceOptions options = null) => throw new NotImplementedException(); | ||
|
|
||
| public ResultWrapper<object> debug_dumpBlock(BlockParameter blockParameter) => throw new NotImplementedException(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -50,6 +50,9 @@ public interface IDebugRpcModule : IRpcModule | |||||
| [JsonRpcMethod(Description = "Similar to debug_traceBlock, this method accepts a block hash and replays the block that is already present in the database.", IsImplemented = true, IsSharable = false)] | ||||||
| ResultWrapper<IReadOnlyCollection<GethLikeTxTrace>> debug_traceBlockByHash(Hash256 blockHash, GethTraceOptions options = null); | ||||||
|
|
||||||
| [JsonRpcMethod(Description = "Replays the block already present in the database and returns the world state root after each transaction.", IsImplemented = true, IsSharable = false)] | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Low — Weak The current description omits details that callers need to understand the contract. Geth's docs are equally sparse. Suggested richer description:
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rewrote it in |
||||||
| ResultWrapper<IReadOnlyCollection<Hash256>> debug_intermediateRoots(Hash256 blockHash, GethTraceOptions? options = null); | ||||||
|
|
||||||
| [JsonRpcMethod(Description = "", IsImplemented = false, IsSharable = false)] | ||||||
| ResultWrapper<GethLikeTxTrace[]> debug_traceBlockFromFile(string fileName, GethTraceOptions options = null); | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Low — Unused
usingNethermind.Evm.TransactionProcessingis not referenced anywhere in this file. Remove it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left this one in —
GasConsumedlives inNethermind.Evm.TransactionProcessingand it's the type in theMarkAsSuccess/MarkAsFailedsignatures. Tried removing it and the build went red.