Skip to content

refactor(mcms): inline set-config sequences and add idempotency keys#94

Open
graham-chainlink wants to merge 1 commit into
mainfrom
ggoh/refactor-set-config-sequence-cleanup
Open

refactor(mcms): inline set-config sequences and add idempotency keys#94
graham-chainlink wants to merge 1 commit into
mainfrom
ggoh/refactor-set-config-sequence-cleanup

Conversation

@graham-chainlink

Copy link
Copy Markdown
Collaborator

Summary

  • Remove nested SeqEVMSetConfig and SeqSolanaSetConfig wrappers and inline their logic into runEVMSetConfig and runSolanaSetConfig, keeping the existing manual OpEVMSetConfigMCM and OpSolanaSetConfigMCM operations.
  • Add per-target operation idempotency keys scoped as chainSelector:address for both EVM and Solana set-config execution.
  • Update sequence tests to exercise the MCMS runners directly via setconfig.ChainInput and pass MCMS input when running in proposal mode.

Why

This applies the set-config sequence cleanup from PR #93 without switching to operations-gen generated write operations. Inlining removes an unnecessary sequence layer while preserving direct-send vs MCMS proposal behavior, and idempotency keys make partial changeset re-runs safer when multiple MCM targets are configured on a chain.

Collapse nested EVM and Solana set-config sequences into their MCMS
runners while keeping manual operations, and scope operation idempotency
per chain and target address for safer re-runs.
@github-actions

Copy link
Copy Markdown

Release impact (release-please)

Current version 0.7.1 (on main)
After merge no version bump (no release)

PR title: refactor(mcms): inline set-config sequences and add idempotency keys

This title will not trigger a semver bump when squash-merged to main. Use feat: for a minor release, or fix: / perf: / revert: for a patch release.

Conventional commit → bump

Intent PR title prefix Bump
Bug fix fix: patch
New feature feat: minor
Breaking change feat!: / fix!: or BREAKING CHANGE: / BREAKING-CHANGE: in description major
No release chore:, docs:, ci:, refactor:, etc. none

Update the PR title before merge if you need a different bump (squash commit message = PR title).

Preview is based on this PR title only. The release-please release PR may include other unreleased commits already on main.

}

// OpEVMSetConfigMCM sets MCMS config on an EVM MCM contract via the MCMS SDK configurer.
// TODO: this may be removed if we generate operations via operations gen tool.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after discussion, we decided to not use operations-gen cli for setconfig but instead use the mcms sdk

)

// SeqEVMSetConfigInput is the input for the generic EVM set-config sequence.
type SeqEVMSetConfigInput struct {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flattening this sequence into the parent sequence as this adds unnecessary extra layer of sequence

)

// SeqSolanaSetConfigInput is the input for the generic Solana set-config sequence.
type SeqSolanaSetConfigInput struct {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flattening this sequence into the parent sequence as this adds unnecessary extra layer of sequence

@graham-chainlink graham-chainlink marked this pull request as ready for review June 19, 2026 04:19
@graham-chainlink graham-chainlink requested a review from a team as a code owner June 19, 2026 04:19
Copilot AI review requested due to automatic review settings June 19, 2026 04:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 refactors the MCMS set-config runners for both EVM and Solana by removing the nested Seq*SetConfig wrapper sequences, inlining their per-target execution logic directly into runEVMSetConfig / runSolanaSetConfig, and adding per-target idempotency keys.

Changes:

  • Inline per-target Op*SetConfigMCM execution into runEVMSetConfig and runSolanaSetConfig, removing the exported wrapper sequences.
  • Add per-target operation idempotency keys scoped as chainSelector:address for both EVM and Solana execution.
  • Update sequence tests to call the runners directly using setconfig.ChainInput and provide MCMS input when running in proposal mode.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
mcms/solana/set-config/sequence.go Removes the wrapper sequence, inlines per-target operation execution, adds idempotency keys, and keeps Solana batching behavior in MCMS mode.
mcms/solana/set-config/sequence_test.go Updates tests to call runSolanaSetConfig directly via setconfig.ChainInput, including MCMS proposal input when applicable.
mcms/solana/set-config/operation_test.go Renames the sequence test hook to the new runner-based test function.
mcms/evm/set-config/sequence.go Removes the wrapper sequence, inlines per-target operation execution, adds idempotency keys, and preserves batch construction in MCMS mode.
mcms/evm/set-config/sequence_test.go Updates tests to call runEVMSetConfig directly via setconfig.ChainInput and provide MCMS input in proposal mode.
mcms/evm/set-config/operation.go Removes an outdated TODO comment now that wrappers/ops-gen direction has been clarified by the refactor.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@cl-sonarqube-production

Copy link
Copy Markdown

@graham-chainlink graham-chainlink requested a review from ecPablo June 19, 2026 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants