feat: add execution error decode command #604
Conversation
🦋 Changeset detectedLatest commit: 42702ae The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
# Conflicts: # go.mod # go.sum
There was a problem hiding this comment.
Pull request overview
This PR adds functionality to decode EVM execution errors using ABI registries, including a new CLI command, comprehensive error decoding helpers, and dependency updates. The implementation enables users to decode transaction error data from JSON files by leveraging the domain's ABI registry for human-readable error messages.
Key changes:
- Added
tryDecodeExecutionErrorand supporting functions to decode execution errors with or without an ABI registry - Introduced
error-decode-evmCLI command that reads JSON files containing transaction errors and decodes them using the environment's ABI registry - Updated several dependencies including mcms, chain-selectors, and testing framework packages
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Updated dependency versions for mcms, chain-selectors, chainlink-testing-framework, and other packages |
| engine/cld/legacy/cli/mcmsv2/mcms_v2.go | Added new CLI command error-decode-evm to decode transaction errors from JSON files |
| engine/cld/legacy/cli/mcmsv2/err_decode_helpers_test.go | Added comprehensive test coverage for the new error decoding functionality |
| engine/cld/legacy/cli/mcmsv2/err_decode_helpers.go | Implemented error decoding helpers including tryDecodeExecutionError and supporting functions |
| chain/evm/provider/rpcclient/multiclient.go | Enhanced error logging by joining original error with decoded error data |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
looks good overall, but we should apply the fixes suggested by Copilot before merging.
As a future enhancement, I'd suggest making the error-file optional and accepting the encoded error from stdin by default.
Oh, and we're also missing a changeset.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Create ProposalContext to get EVM registry | ||
| proposalCtx, err := analyzer.NewDefaultProposalContext(env) | ||
| if err != nil { | ||
| lggr.Warnf("Failed to create default proposal context: %v. Proceeding without ABI registry.", err) |
There was a problem hiding this comment.
After warning about the failed proposal context, the code proceeds but doesn't set proposalCtx to nil or handle the case where it might be in a partially initialized state. Consider explicitly setting proposalCtx = nil after this warning to ensure consistent behavior in the checks below.
| lggr.Warnf("Failed to create default proposal context: %v. Proceeding without ABI registry.", err) | |
| lggr.Warnf("Failed to create default proposal context: %v. Proceeding without ABI registry.", err) | |
| proposalCtx = nil |
| } | ||
| } | ||
|
|
||
| if hasSelector && !hasData { |
There was a problem hiding this comment.
The logic checks hasSelector && !hasData on line 673, but when hasData is true and decoding fails (lines 668-670), it doesn't try the selector. Consider attempting selector-based decoding as a fallback when data-based decoding fails, by changing line 673 to just if hasSelector to cover both cases where data decoding failed and where there's no data.
| if hasSelector && !hasData { | |
| if hasSelector { |
|
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## chainlink-deployments-framework@0.71.0 ### Minor Changes - [#604](#604) [`c0b7401`](c0b7401) Thanks [@ecPablo](https://github.com/ecPablo)! - add error decode command --------- Co-authored-by: app-token-issuer-engops[bot] <144731339+app-token-issuer-engops[bot]@users.noreply.github.com>
3 participants
Adds execution error decode command. This command takes a JSON for an mcms-tools results json and attempts to decode the error revert reason using the ABI registries injected by the proposal context.
TODO: Remove local replace once mcmslib is released with smartcontractkit/mcms#547
.
AI Summary
This pull request introduces significant improvements to error decoding for EVM transaction failures, adds a new CLI command for error decoding, and includes dependency updates. The most notable changes are the addition of robust helpers for decoding EVM execution errors, comprehensive tests for these helpers, and a new CLI command to decode errors from JSON files using the ABI registry. Several dependencies in
go.modhave also been updated.EVM Error Decoding Enhancements
DecodedExecutionErrorstruct andtryDecodeExecutionErrorfunction inerr_decode_helpers.goto extract and decode revert reasons and underlying reasons from EVM execution errors, handling both custom and standard errors, and supporting decoding with or without an ABI registry.decodeRevertDataanddecodeRevertDataFromByteshelpers to convert hex or byte revert data into human-readable error messages, falling back to custom error selectors if no decoder is available.CLI Improvements
error-decode-evminmcms_v2.gothat reads a JSON file containing transaction error data, loads the ABI registry from the specified environment, decodes the error using the new helpers, and prints the decoded revert and underlying reasons. [1] [2]Testing
Test_tryDecodeExecutionErrorinerr_decode_helpers_test.gothat covers decoding custom errors, standard errors, selectors, underlying reasons, and nil/empty cases for the new error decoding logic.Dependency Updates
go.mod, includinggithub.com/smartcontractkit/chain-selectors,github.com/smartcontractkit/chainlink-testing-framework/framework,golang.org/x/oauth2, and others. Also, replaced thegithub.com/smartcontractkit/mcmsdependency to use a local path. [1] [2] [3] [4] [5] [6] [7]Minor Improvements
MultiClient.retryWithBackupsmethod for more informative logs when retryable errors occur.maybeDataErrfor code clarity.These changes collectively improve the developer experience when diagnosing and understanding EVM transaction failures, both programmatically and via the CLI.