Best effort to decode nester error strings from reverts#184
Open
davecrighton wants to merge 4 commits intohyperledger:mainfrom
Open
Best effort to decode nester error strings from reverts#184davecrighton wants to merge 4 commits intohyperledger:mainfrom
davecrighton wants to merge 4 commits intohyperledger:mainfrom
Conversation
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
Signed-off-by: Dave Crighton <dave.crighton@kaleido.io>
1de5d77 to
1e98a85
Compare
EnriqueL8
reviewed
Mar 4, 2026
Contributor
There was a problem hiding this comment.
first pass - @davecrighton can you sign off the commits for DCO please
peterbroadhurst
requested changes
Mar 4, 2026
Contributor
peterbroadhurst
left a comment
There was a problem hiding this comment.
To aid a review @davecrighton - please could you provide in the subject:
- A Solidity example of the coding pattern that results in the problem
- A link to somewhere that helps us understand how standard this is
- e.g. is it a link to the Solidity docs, or OpenZeppelin, or some forums on coding suggestions
- An description of what the algorithm is we're applying
- I understand from your comment this only works for
Error(string)"default" revert errors, but what are we inferring about thestringplaced in there and why
- I understand from your comment this only works for
peterbroadhurst
requested changes
Mar 4, 2026
Contributor
peterbroadhurst
left a comment
There was a problem hiding this comment.
Second request from me:
Can we make this a pkg utility function (honestly I think firefly-signer would be the ideal place, but we've started putting package utilities in firefly-evmconnect too a little recently).
This code feels important to standardize as DecodeRevertErrorBytes() and return a well structured struct return that is documented, that has a String() function.
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.
Additional change related to hyperledger/firefly#1717
This provides a best-effort case for decoding error strings from reverts where the revert message has been constructed by nesting customer error types or generic errors (see the linked issue for details).
The approach taken is:
After decoding the outer Error(string) we scan forwards in the resulting string finding the first occurence of either any known error selector provided in the ABI list or the generic error selector.
If a selector is found the remainder of the string is decoded using either standard ABI decoding for generic errors or the existing formatCustomError function for custom errors.
If no selector matches, or if decoing fails any remaining bytes are converted to a hex encoded string.
I did consider possibly additionally stripping null bytes to see if we could return a readable string in more cases but I considered it might be more confusing to get a halfway approach rather than representing an entire nested error as either always decoded or always hex.