feat: add support for eip 1967 proxy decoding of implementation contract [DPT-108]#559
Conversation
🦋 Changeset detectedLatest commit: f56aae9 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 |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for decoding proposals that use EIP-1967 proxies, enabling the analyzer to correctly handle transactions routed through proxy contracts by detecting proxies, querying their implementation addresses, and decoding using the implementation contract's ABI.
Key Changes:
- Added EIP-1967 proxy detection and fallback decoding mechanism in the EVM analyzer
- Extended interfaces and function signatures to pass
context.Contextanddeployment.Environmentthroughout the analyzer stack - Updated CLI commands and tests to provide the necessary context and environment parameters
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
experimental/analyzer/evm_analyzer.go |
Implements EIP-1967 proxy detection, storage querying, and fallback decoding logic |
experimental/analyzer/evm_analyzer_test.go |
Adds comprehensive tests for proxy fallback mechanism |
experimental/analyzer/report_builder.go |
Updates function signatures to accept context and environment parameters |
experimental/analyzer/describe.go |
Updates describe functions to pass context and environment |
experimental/analyzer/upf/upf.go |
Updates UPF conversion functions with new parameters |
chain/evm/evm_chain.go |
Adds StorageAt method to OnchainClient interface |
chain/evm/provider/rpcclient/multiclient.go |
Implements retrying StorageAt method |
engine/cld/legacy/cli/mcmsv2/mcms_v2.go |
Updates CLI commands to pass context and environment |
engine/cld/legacy/cli/commands/*.go |
Updates command implementations with new parameters |
.changeset/floppy-trams-kiss.md |
Documents the new feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Check if this is a "method not found" error - could be a proxy with wrong ABI | ||
| if isMethodNotFoundError(err) { | ||
| // Try EIP-1967 proxy fallback: query implementation slot and retry with implementation ABI | ||
| fallbackResult, fallbackABI, fallbackABIStr, fallbackErr := tryEIP1967ProxyFallback( |
There was a problem hiding this comment.
There are quite a few different proxy “standards” in Ethereum.
Just leaving a note in case you plan to add more detection methods in the future.
| case chainsel.FamilyEVM: | ||
| decoder := mcmsanalyzer.NewTxCallDecoder(nil) // FIXME: reuse instance | ||
| analyzeResult, _, abi, err := mcmsanalyzer.AnalyzeEVMTransaction(proposalCtx, decoder, uint64(mcmsOp.ChainSelector), mcmsOp.Transaction) | ||
| analyzeResult, _, abi, err := mcmsanalyzer.AnalyzeEVMTransaction(context.Background(), proposalCtx, env, decoder, uint64(mcmsOp.ChainSelector), mcmsOp.Transaction) |
There was a problem hiding this comment.
the context should be passed by the caller (all the way up to UpfConvertProposal
| return nil | ||
| } | ||
|
|
||
| func (m *mockProposalContext) SetRenderer(renderer Renderer) {} |
There was a problem hiding this comment.
why not use mockery generated mocks for mockStorageClient or mockProposalContext?
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
ecPablo
dismissed stale reviews from gustavogama-cll and ChrisAmora
via
cb7d39a
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 14 out of 14 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 14 out of 14 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
experimental/analyzer/evm_analyzer_test.go:1
- Struct assignment creates a shallow copy, meaning the original chain's fields are copied but nested pointers (like
Client) still point to the same underlying objects. ModifyingmockChain.Clientalso modifiesevmChain.Client. This could cause unexpected side effects in tests. Create a proper deep copy or use a pointer to avoid modifying the original chain.
package analyzer
experimental/analyzer/evm_analyzer.go:1
- [nitpick] Use
implAddress.Hex() == (common.Address{}).Hex()or create a named zero address constant for better readability and consistency with Ethereum best practices. The current comparison works but is less explicit about comparing zero addresses.
package analyzer
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
# Conflicts: # .mockery.yml
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
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.65.0 ### Minor Changes - [#568](#568) [`109b6f8`](109b6f8) Thanks [@giogam](https://github.com/giogam)! - feat: adds HMAC authentication support for catalog remote - [#559](#559) [`57ee135`](57ee135) Thanks [@ecPablo](https://github.com/ecPablo)! - Add support to decode proposals that use EIP-1967 proxies - [#562](#562) [`aa38817`](aa38817) Thanks [@jkongie](https://github.com/jkongie)! - Removes the import of a root `go.mod` from a scaffolded domain - [#567](#567) [`d06057a`](d06057a) Thanks [@JohnChangUK](https://github.com/JohnChangUK)! - Sui MCMS upgrade ### Patch Changes - [#530](#530) [`dc2c113`](dc2c113) Thanks [@graham-chainlink](https://github.com/graham-chainlink)! - fix: make config files and chain credentials optional --------- Co-authored-by: app-token-issuer-engops[bot] <144731339+app-token-issuer-engops[bot]@users.noreply.github.com>
This pull request adds support for decoding proposals that use EIP-1967 proxies, enabling the analyzer to correctly handle transactions routed through proxy contracts. It introduces logic to detect EIP-1967 proxies, query their implementation addresses, and decode transactions using the implementation contract's ABI. Several interfaces and functions have been updated to support this feature, and related CLI commands and tests have been refactored to pass the necessary context and environment.
EIP-1967 Proxy Decoding Support
experimental/analyzer/evm_analyzer.goto detect EIP-1967 proxies, query the implementation address from the proxy's storage slot, and retry transaction decoding using the implementation's ABI. This includes helper functions for error detection, storage querying, and address book lookup. [1] [2] [3]Interface and Function Refactoring
OnchainClientinterface inchain/evm/evm_chain.goto include aStorageAtmethod for reading contract storage slots, required for EIP-1967 detection.StorageAtinchain/evm/provider/rpcclient/multiclient.go.context.Context,ProposalContext, anddeployment.Environmentparameters, enabling access to chain clients and address books for proxy decoding. [1] [2] [3] [4] [5] [6]CLI and Test Updates
Documentation
chainlink-deployments-frameworkpackage.