refactor: introduce shared action layer (CctActions + EoaExecutor) under the setup scripts, with parity tests#4
Merged
Conversation
…der the setup scripts, with parity tests
|
👋 aelmanaa, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
14 tasks
SyedAsadKazmi
approved these changes
Jul 8, 2026
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.
What improves for the user
Before: every script builds calldata inline, so the same encoding logic is duplicated across scripts and there is no way to reuse an operation outside its one script.
After: every README command works identically (parity-tested, byte for byte), and each operation's calldata is built in exactly one audited place — which is what makes future governance execution modes (multisig batch, timelock) and lifecycle flows possible without re-implementing any operation. The visible change for a user is zero; the change for a reviewer is that one file now answers "what does this operation send on-chain."
What this PR does
This PR sets the pattern a follow-up rollout PR replicates across the remaining script groups (configure, hooks/allowlist, operations) — please review it line by line; design feedback lands here, before the pattern is stamped across the repo.
src/actions/CctActions.sol— the shared action layer. ACall {target, value, data}struct plus one pure builder per setup write operation, each encoded withabi.encodeCallon the real contract interfaces (never a hand-written selector):registerAdminViaOwnerandregisterAdminViaGetCCIPAdmin(two builders for the twoRegistryModuleOwnerCustomclaim paths — the owner-vs-getCCIPAdmin probe stays script-side inClaimAdmin),acceptAdminRole,applyChainUpdates(takes the already-encoded remote pool/token bytes, so chain-family encoding stays inChainHandlers),setPool,transferAdminRole,transferOwnership,acceptOwnership, plus the token-admin handoff builders the transfer-ownership scripts need (beginDefaultAdminTransfer,acceptDefaultAdminTransfer,grantRole/revokeRole/handOffRole).Includes
registerAndAcceptAdmin*one-batch conveniences: the claim sets the registry's pending administrator to the calling account, so claim + accept execute correctly as ONE atomic batch (proven bytest_RegistrationPair_ExecutesAsOneBatch).src/base/EoaExecutor.s.sol— the EOA execution mode: takes aCall[],vm.startBroadcast(), executes each call in order (bubbling the underlying revert reason unchanged), and logs each target + selector.ClaimAdmin,AcceptAdminRole,ApplyChainUpdates(both JSON and env modes),SetPool,TransferTokenAdminRole,transfer-ownership/TransferOwnership,transfer-ownership/AcceptOwnership. Each parses inputs exactly as today, calls the matching builder, and hands the result to the executor. External behavior, env vars, and console output are unchanged (the only console addition is the executor's target+selector line). The read-only setup scripts (GetSupportedChains,GetTokenConfig,GetTypeAndVersion) are untouched.script/input/apply-chain-updates.jsonexample fixed (closes the known issue from the CI PR): theARBITRUM_SEPOLIAentry had nodestChainSelectorandHelperConfigdoes not know that chain, so JSON mode with the committed example reverted (Chain selector is 0 for remoteChains[1]). The entry now usesPLUME_TESTNET(a chainHelperConfigserves), and a test runs JSON mode with the committed example itself, not only a test fixture.Parity-test methodology (how "the refactor changed nothing" is proven)
The OLD (pre-refactor) behavior was captured first: each parity test replays the exact calls the pre-refactor script made (direct typed calls from the same signer), snapshots the resulting fork state, reverts, then runs the REFACTORED script and asserts the identical end state. Where encoding matters, the builder output is additionally pinned byte-for-byte against a hand-written
abi.encodeCallexpectation.New suite
test/actions/SetupActions.t.sol(8 tests), extendingBaseForkTest(public-RPC default, zero config):applyChainUpdatescalldata parity for an EVM remote (abi.encode(address)) AND an SVM remote (raw 32 bytes from an independent base58 decode) vs hand-encodedabi.encodeCall(TokenPool.applyChainUpdates, (...)).setPoolcalldata parity + fork registry-state parity (old inlineregistry.setPoolvs the refactored script).ClaimAdmin+AcceptAdminRolefork-state parity (TokenConfig administrator end state), plus the registration pair as ONE batch.TransferOwnership/AcceptOwnershiptwo-step on a fork: poolOwnable2Step(both scripts exercised), and the token default-admin two-step (beginDefaultAdminTransfer→ warp past the transfer schedule → accept). Note: the ownership scenarios run inside one test function on purpose — forge runs tests in parallel andvm.setEnvis process-wide, so tests setting differentENTITY_TYPE/ADDRESS/NEW_OWNERvalues would race (same lesson as the CI PR's parallel-suite note; the same fix serializes the two JSON-input-file phases inApplyChainUpdates.t.sol).test/setup/ApplyChainUpdates.t.solkeeps the pre-existing fixture-JSON state assertions passing against the refactored script, and gains the committed-example phase (asserts MANTLE_SEPOLIA, PLUME_TESTNET, SOLANA_DEVNET all configured, incl. the two-remote-pool entry).Acceptance checklist
script/setup/README command runs unchanged against a fork — each refactored script was dry-run (no--broadcast) on a local Sepolia fork with the README's exact invocation, before and after the refactor; the console output diff is empty except the executor's addedExecuting call i/n: target … selector …line, and the end state (registry pool, TAR administrator, pool owner, token default admin) is identical. Sample dry-run log delta below.ApplyChainUpdatesandSetPool(and the claim/accept + ownership flows).apply-chain-updates.jsonexample runs clean in JSON mode (known issue closed; pre-fix it exited 1 withChain selector is 0 for remoteChains[1], post-fix the same invocation completes simulation, exit 0).forge build,forge fmt --check,forge lint,forge testall green locally: 22 tests (14 existing kept passing + 8 new tests inSetupActions.t.sol; the pre-existing JSON-mode test additionally gained the committed-example phase).Sample dry-run log delta (ClaimAdmin, old vs refactored — README command, no --broadcast)
Same shape for every other setup script:
AcceptAdminRole(0x156194da),ApplyChainUpdates(0xe8a1da17),SetPool(0x4e847fc7),TransferTokenAdminRole(0xddadfa8e),TransferOwnershippool (0xf2fde38b) / token (0x634e93da),AcceptOwnershippool (0x79ba5097) / token (0xcefc1429).Test plan
forge buildforge fmt --checkforge lintforge testOut of scope: deploy scripts (they create contracts and stay broadcast scripts by nature — not
Call[]material); all configure/hooks/operations scripts (the follow-up rollout PR); anything under.github/.