-
Notifications
You must be signed in to change notification settings - Fork 95
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
Use debug traces to extract the input for settlement::Observer #3245
Merged
+433
−36
Merged
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
cbf7b4c
Revert "Revert "Use traces to extract the input for `settlement::Obse…
sunce86 4f3da4f
use debug_traceTransaction instead
sunce86 e0c7919
process subcalls in the exact orders they were called
sunce86 6ca2b1d
fix comment
sunce86 3f16eae
moved the debug_traceTransaction into ethrpc
sunce86 67a1fef
actually mimic debug namespace
sunce86 5cfe089
Merge branch 'main' into transaction-trace-2
sunce86 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Loading status checks…
use debug_traceTransaction instead
commit 4f3da4fa0d5c793c1a334722672849554206e4a9
There are no files selected for viewing
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
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
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
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
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.
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.
This exact type exists twice. Once in the infra module and once in the domain module. I suspect we only need one of them.
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.
I don't want to have deserialization specifics in the
domain
.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.
Hmm, I see. I wonder if this and the
debug_trace
logic might not be better placed in theethrpc
crate.We might as well make this easily reusable with an extension trait. It's a bit awkward that the
web3
crate does not support thedebug
module out of the box but AFAICS we could add a new namespace fordebug
which can have thetrace_transaction
functionality.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.
Note that what I have right now in the code is not actually reusable in general. It's actually the pure minimum of code needed to satisfy the autopilot's needs. The reasons:
1.
CallFrame
only contains subset of data that is needed for functionality used in autopilot.2. I use only one type of calltracer here.
In general, to fully implement
debug_traceTransaction
function so it could be reused with all of it's different input's and outputs would require a lot more code which would be a copy paste fromalloy
crate.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.
Moved the current impl code into
ethrpc