-
Notifications
You must be signed in to change notification settings - Fork 40
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
refactor: trace_decoder::decoding #469
Conversation
6312836
to
f65fbd0
Compare
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.
There are some nice changes and some potentially mildly controversial ones. See some comments below, mostly to do with general Rust coding conventions and such.
Note that the change in indentation makes it harder to see which are the genuine changes introduced in this PR. Therefore, some of the comments may refer to stuff that only incidentally shows up in the diff.
@Nashtare you mentioned some edge cases from John? |
@muursh They mostly revolve around revert cases (either full txn revert or some chile context reversion). There seem to be some issues with txn reversions in mainnet blocks as well (cf #475, #421), but not sure yet whether this can be handled without changes on the Jerigon side or not, still looking at them. |
I'm aiming for no behavioural changes with this PR - have I inadvertently broken something? |
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.
Only few non blocking nitpicks
* mark: 0xaatif/refactor-trace-decoder-decoding * refactor: remove inappropriate static methods * refactor: remove inappropriate method * refactor: remove TraceParsingErrorReason * refactor: remove LocatedError * refactor: remove EMPTY_ACCOUNT_BYTES_RLPED * refactor: remove update_val_if_some * refactor: impl Display for TrieType * wibble * refactor: inline TraceParsingResult * wibble * wibble: order * refactor: remove unused variable * wibble: inline TrieRoots etc * wibble * review: WithHash -> CustomFmt<T>
#275
Change of pace to do some simple cleanup:
ProcessedBlockTrace
, I've made those functions now.anyhow
. The same details will be exposed to end users.std::error::Error::source
, but no changes should be required for errors which [panic
] or are returned frommain
.