Represent nested structure for revert errors when solidity catches and rethrows an error#98
Merged
EnriqueL8 merged 22 commits intohyperledger:mainfrom Apr 22, 2026
Merged
Conversation
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
…compatibility Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
fa13b54 to
7a8f952
Compare
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Contributor
Author
|
Some e2e examples of before / after serializations: BeforeAfterAssociated solidity snippet: |
peterbroadhurst
requested changes
Apr 20, 2026
Contributor
peterbroadhurst
left a comment
There was a problem hiding this comment.
Thanks @davecrighton - this looks like a great piece of work.
I do think we need to go through my questions/comments in the PR and close out on some opinions about this pattern before we merge.
| // tried when decoding revert data, even if the caller's ABI is empty. | ||
| var defaultErrorEntries = ABI{ | ||
| {Type: Error, Name: "Error", Inputs: ParameterArray{{Name: "reason", Type: "string"}}}, | ||
| {Type: Error, Name: "Panic", Inputs: ParameterArray{{Name: "code", Type: "uint256"}}}, |
Contributor
There was a problem hiding this comment.
Great!
Found the detail on the difference and read up on hit here: https://docs.soliditylang.org/en/latest/control-structures.html#panic-via-assert-and-error-via-require
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
…an ABI after the matched selector Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
30bc777 to
77652c2
Compare
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
EnriqueL8
approved these changes
Apr 21, 2026
Contributor
EnriqueL8
left a comment
There was a problem hiding this comment.
Awesome @davecrighton , thanks for all the back and forth on this one
Merged
9 tasks
This file contains hidden or 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
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.
Proposed changes
This PR introduces a new type to represent an EVM
RevertError, along with parsing and serialization support for revert payloads that contain nested errors. These show up when Solidity catches and re-throws to wrap the original revert with extra context, for example:See hyperledger/firefly#1717 for more on the scenario and how the related pull requests fit together.
The type exposes
String()for a sensible default serialization, while still acting as a structured representation so callers can apply a more sophisticated serialization scheme if they want.While parsing, we scan error data in 4-byte chunks and compare against known ABI selectors for error types. When a selector matches, existing parsing decodes the following bytes as an ABI-encoded error. Unwrapping stops at a maximum depth of 10.
If we cannot decode, or if any error string contains non-printable characters, output is hex-encoded instead—avoiding cases where non-printable bytes are passed through to callers that may persist them (for example, into a database).
For additional protection in the main Firefly repo against inserting null bytes from error messages into the database, see hyperledger/firefly#1716.
Types of changes
Please make sure to follow these points
Screenshots (If Applicable)
Other Information
Any message for the reviewer or kick off the discussion by explaining why you considered this particular solution, any alternatives etc.