-
Notifications
You must be signed in to change notification settings - Fork 1
feat: check MCM state before calling SetRoot or Execute #556
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
feat: check MCM state before calling SetRoot or Execute #556
Conversation
🦋 Changeset detectedLatest commit: ed28f13 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 |
ea68f1f to
774e4d7
Compare
| // Uses go-ethereum's simulated backend with default settings for fast test execution. | ||
| func NewEVMSimLoader() *ChainLoader { | ||
| selectors := getTestSelectorsByFamily(chainselectors.FamilyEVM) | ||
| selectors = append([]uint64{chainselectors.GETH_TESTNET.Selector}, selectors...) |
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.
Had to prepend GETH_TESTNET because geth's simulated backend always uses a chain id of 1337.
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.
Pull Request Overview
This PR adds state validation checks before calling SetRoot or Execute operations on MCM (Multi-Chain Multi-Sig) contracts to prevent redundant transactions and improve idempotency.
Key changes:
- Adds pre-execution state checks to verify if operations have already been executed
- Updates dependencies including mcms library, testing frameworks, and various Go packages
- Includes test coverage for both
SetRootandExecutecommand retry scenarios
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Updates dependency versions for mcms library and related packages |
| engine/test/onchain/evm_test.go | Adds GETH_TESTNET selector to test setup |
| engine/test/onchain/evm.go | Prepends GETH_TESTNET selector to EVM chain loader |
| engine/cld/legacy/cli/mcmsv2/mcms_v2_test.go | Adds comprehensive tests for SetRoot and Execute command retry scenarios |
| engine/cld/legacy/cli/mcmsv2/mcms_v2.go | Implements MCM state checks before SetRoot/Execute calls and fixes Sui entrypoint encoder initialization |
| .changeset/gentle-waves-appear.md | Documents the feature change |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
774e4d7 to
3a2fd2e
Compare
3a2fd2e to
ed28f13
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.
Pull Request Overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
engine/cld/legacy/cli/mcmsv2/mcms_v2_test.go:1
- Potential index out of bounds panic if no logs match 'operation already executed'. Add a length check before accessing index 0 or use require.Len to verify at least one log entry exists.
package mcmsv2
💡 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 ## [email protected] ### Minor Changes - [#556](#556) [`0e60a11`](0e60a11) Thanks [@gustavogama-cll](https://github.com/gustavogama-cll)! - feat(mcms): check MCM state before calling SetRoot or Execute ### Patch Changes - [#565](#565) [`ba781b4`](ba781b4) Thanks [@gustavogama-cll](https://github.com/gustavogama-cll)! - fix(mcms): check if anvil config is valid after selecting the rpc --------- Co-authored-by: app-token-issuer-engops[bot] <144731339+app-token-issuer-engops[bot]@users.noreply.github.com>




Modify the setRootCommand and executeCommand logic so that the mcm state is probed before calling the
setRootorexecutemethods. This means we shouldn't need to check for the revert messages issued by the contract when asetRootorexecutecall is retried by the GHA workflows.