Refactor Blockchain Bridge Simulate to Support Multiple Generic Tracers #8148
+142
−49
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Deeptanshu-sankhwar
force-pushed
the
refactor/simulate-generic-tracers
branch
from
16a582f to
5f088c6
Compare
Deeptanshu-sankhwar
force-pushed
the
refactor/simulate-generic-tracers
branch
from
5f088c6 to
2b7ad66
Compare
added 4 commits
February 3, 2025 01:40
…t for SimulateBlockTracer, GethLikeBlockMemoryTracer, and ParityLikeBlockTracer while ensuring flexibility in SimulateBlockResult<T> and updating test cases accordingly.
Deeptanshu-sankhwar
force-pushed
the
refactor/simulate-generic-tracers
branch
from
2b7ad66 to
bd3accd
Compare
LukaszRozmej
requested changes
Feb 3, 2025
Comment on lines
+210
to
+213
| if (tracer is SimulateBlockTracer simulateTracer) | ||
| { | ||
| result.Items = simulateTracer.Results as IReadOnlyList<SimulateBlockResult<T>>; | ||
| } |
There was a problem hiding this comment.
Instead of this make generic constraint where T : ISimulateTracer and IBlockTracer<TTrace> interface should have Results of this type.
Comment on lines
+214
to
+218
| else | ||
| { | ||
| throw new InvalidOperationException($"Tracer type {typeof(T).Name} does not support 'Results'"); | ||
| } | ||
|
|
There was a problem hiding this comment.
You also don't need this then as the generic constraint would guard it
Comment on lines
+171
to
+176
| public SimulateOutput<T> Simulate<T>( | ||
| BlockHeader header, | ||
| SimulatePayload<TransactionWithSourceDetails> payload, | ||
| CancellationToken cancellationToken, | ||
| ITracerFactory<T> tracerFactory) | ||
| where T : class |
There was a problem hiding this comment.
Suggested change
| public SimulateOutput<T> Simulate<T>( | |
| BlockHeader header, | |
| SimulatePayload<TransactionWithSourceDetails> payload, | |
| CancellationToken cancellationToken, | |
| ITracerFactory<T> tracerFactory) | |
| where T : class | |
| public SimulateOutput<TTrace> Simulate<TTracer, TTrace>( | |
| BlockHeader header, | |
| SimulatePayload<TransactionWithSourceDetails> payload, | |
| CancellationToken cancellationToken, | |
| ITracerFactory<TTracer> tracerFactory) | |
| where TTracer : IBlockTracer<TTrace> |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This refactor makes
Simulatein the Blockchain Bridge generic, allowing it to supportSimulateBlockTracer,GethLikeBlockMemoryTracer, andParityLikeBlockTracerdynamically.Key Changes
Generic
SimulateImplementationSimulateBlockResult<T>to support multiple tracers.Simulateto a generic function to improve flexibility.Factory Method for Tracer Selection
Changes to Data Structures
SimulatePayloadandSimulateBlockResultto work generically.