-
Notifications
You must be signed in to change notification settings - Fork 22
remove unnec. changeset parameters, unused validation f(x)'s, and fix… #1438
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
base: main
Are you sure you want to change the base?
Conversation
… setDomains address parameter bug
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
Refactors EVM deployment changesets to remove the MCMSReaderRegistry parameter and simplify constructors, while shifting MCMS validation responsibility elsewhere and requiring explicit addresses for SetDomains.
- Removes MCMSReaderRegistry from all changeset constructors and uses nil in output builders.
- Eliminates MCMS.Validate calls in verify functions (now all no-ops).
- Adjusts SetDomains to take explicit token pool addresses per chain instead of datastore lookup.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| usdc_token_pool_proxy_deploy_test.go | Updates tests to use new parameterless constructor. |
| usdc_token_pool_proxy_deploy.go | Removes registry parameter and MCMS validation from deploy/verify logic. |
| usdc_token_pool_deploy_test.go | Updates tests for parameterless changeset constructors. |
| usdc_token_pool_deploy.go | Removes registry parameter and MCMS validation in verify. |
| usdc_token_pool_cctp_v2_deploy_test.go | Updates tests to new constructor signature. |
| usdc_token_pool_cctp_v2_deploy.go | Removes registry parameter; minor conditional simplification; drops MCMS validation. |
| update_pool_addresses_test.go | Updates test to use new constructor signature. |
| update_pool_addresses.go | Removes registry parameter and MCMS validation; passes nil to output builder. |
| update_lock_release_pool_addresses_test.go | Updates test to new constructor signature. |
| update_lock_release_pool_addresses.go | Removes registry parameter and MCMS validation; passes nil to output builder. |
| update_lock_or_burn_mechanism_test.go | Test updated; no MCMS input provided after validation removal. |
| update_lock_or_burn_mechanism.go | Removes registry parameter and MCMS validation; passes nil to output builder. |
| siloed_usdc_token_pool_deploy_test.go | Updates test for parameterless constructor. |
| siloed_usdc_token_pool_deploy.go | Removes registry parameter and leaves verify as no-op. |
| set_domains_test.go | Adds explicit address field usage and removes registry + MCMS fields in test. |
| set_domains.go | Requires explicit address per chain; removes datastore lookup and MCMS validation. |
| erc20_lockbox_deploy.go | Removes MCMS from input type; builds output with empty MCMS input. |
| erc20_lock_box_deploy_test.go | Updates test to match removed MCMS field. |
| configure_allowed_callers_test.go | Updates test and simplifies MCMS input (validation removed). |
| configure_allowed_callers.go | Removes registry parameter and MCMS validation; passes nil to output builder. |
| apply_authorized_caller_updates.go | Removes registry parameter and MCMS validation; verify now no-op. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
chains/evm/deployment/v1_6_4/changesets/erc20_lockbox_deploy.go
Outdated
Show resolved
Hide resolved
chains/evm/deployment/v1_6_4/changesets/apply_authorized_caller_updates.go
Show resolved
Hide resolved
…fier, and remove superfluous input parameters
|
This pull request refactors several deployment changesets in the EVM chain to simplify their interfaces and remove unnecessary dependencies on the
MCMSReaderRegistry. The changesets now accept input directly, rather than requiring a registry parameter, and related tests have been updated to reflect the new function signatures. Additionally, theSetDomainschangeset now requires the token pool address to be specified explicitly for each chain, improving flexibility for different token pool types.Refactoring changesets to remove MCMSReaderRegistry dependency:
ApplyAuthorizedCallerUpdatesChangeset,ConfigureAllowedCallersChangeset,ERC20LockboxDeployChangeset,SiloedUSDCTokenPoolDeployChangeset, andSetDomainsChangeset) now take no parameters and do not require anMCMSReaderRegistry, simplifying their usage and signatures. [1] [2] [3] [4] [5]Apply,Verify, and builder functions have been updated to remove the registry parameter and usenilwhere appropriate. [1] [2] [3] [4] [5]Test code updates:
changesets_utils.GetRegistry()and to use the new parameterless changeset constructors. [1] [2] [3] [4]SetDomains changeset improvements:
SetDomainsPerChainInputstruct now requires an explicitAddressfield for each chain, rather than looking up the address from the datastore, supporting token pools of different types. [1] [2] [3]Cleanup of unused imports and code:
datastore,changesets_utils, andmcms_typesfrom several files to streamline the codebase. [1] [2] [3] [4] [5] [6] [7]Input validation changes:
MCMSinput has been removed from theVerifyfunctions, delegating responsibility for input validation elsewhere. [1] [2] [3]