feat: refactor usages of chain selectors API to use remote API #614

ecPablo · 2026-02-06T18:18:09Z

This PR upgrades usages of the chain selectors lib to use the new Remote API smartcontractkit/chain-selectors#182
Notes:

The SUI and Aptos chain selectors remote APIs are not implemented yet. So we are keeping the local ones. Those chains don't have many updates so it's not as important to support that in the remote API + remote support is only available on EVM in the meantime.
A lot of the file changes are just standardising on the import name to chainsel
There's a couple of context.Background that I had to hardcode in the evm sdk. This is because we still need to provide a ctx variable at the interface level, which would mean a breaking change is introduced and all chain families sdk would need to be updated. I'd prefer to leave that for a separate PR to handle communications and keep things isolated in case of reverts, etc.

Signed-off-by: Pablo <[email protected]>

changeset-bot · 2026-02-06T18:18:14Z

🦋 Changeset detected

Latest commit: 5a07554

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@smartcontractkit/mcms	Minor

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

ecPablo · 2026-02-06T22:41:22Z

sdk/evm/encoder.go

+
+	// TODO: passing this as param requires a breaking change to the Encoder interfaces
+	// which requires changes too all chain specific SDKs. Consider refactoring later.
+	evmChainID, err := getEVMChainID(context.Background(), e.ChainSelector, e.IsSim)


I want to remove this context.Background() and pass it as a param. But doing so will mean modifying the interfaces on the sdk pkg which is a pretty significant breaking change. My proposal is to go with this suboptimal approach so we can get the benefits of the remote apis now, and work on adding the ctx as separate followup so we can scope all breaking changes in a single PR and document better the changes that product teams would need to do for announcements, etc.

Signed-off-by: Pablo <[email protected]>

ecPablo · 2026-02-10T21:43:12Z

sdk/evm/encoder.go

+	// which requires changes too all chain specific SDKs. Consider refactoring later.
+	// this could even mean not using a ctx, and instead providing the chainID directly to the Encoder
+	// at construction time.
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) //nolint


This is the code that we need to remove in the followup, by introducing a ctx param in the callstack, moving the chain selectors fetch upstream and just adding a chainID parameter.

ToGethRootMetadata is a public function (though I'm not sure it should be; as usual, we expose far more than we should, imo). And, as such, isn't this a breaking change anyways?

Maybe the question should be: you chose to preserve ToGetOperation but not ToGetRootMetadata. Why the distinction?

Signed-off-by: Pablo <[email protected]>

docs/docs/usage/building-proposals.md

Signed-off-by: Pablo <[email protected]>

cl-sonarqube-production · 2026-02-12T13:00:59Z

Quality Gate passed

Issues
25 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
98.6% Coverage on New Code
1.3% Duplication on New Code

See analysis details on SonarQube

gustavogama-cll · 2026-02-12T20:16:49Z

docs/docs/usage/building-proposals.md

-  5, // Value in GWEI
-  "MyEVMContractType",
-  []string{"tag1", "tag2"},
+common.Address{},


broken indent

gustavogama-cll · 2026-02-12T20:16:56Z

docs/docs/usage/building-proposals.md

 builder.AddOperation(types.Operation{
-  ChainSelector: selector,
-  Transaction:   tx,
+ChainSelector: selector,


broken indent

gustavogama-cll · 2026-02-12T20:17:03Z

docs/docs/usage/building-proposals.md

-    IsSigner:   false,
-    IsWritable: true,
-  }
+{


broken indent

gustavogama-cll · 2026-02-12T20:17:09Z

docs/docs/usage/building-proposals.md

-  accounts,
-  "MySolanaContractType",
-  []string{"tag1", "tag2"}
+"programIDGoesHere",


broken indent

gustavogama-cll · 2026-02-12T20:17:15Z

docs/docs/usage/building-proposals.md

 toAddress := aptos.AccountAddress{}
 if err := toAddress.ParseStringRelaxed("0xe2bb72029d2e1f48b7bede6569a9226653c43ab44e1a7e0df21eac31b3c91bf4"); err != nil {
-  panic(err)
+panic(err)


broken indent

gustavogama-cll · 2026-02-12T20:17:20Z

docs/docs/usage/building-proposals.md

-  []byte("calldata"),
-  "MyAptosContractType",
-  []string("tag1", "tag2")
+"package",


broken indent

gustavogama-cll · 2026-02-12T20:17:25Z

docs/docs/usage/building-proposals.md

 )
 if err != nil {
-  panic(err)
+panic(err)


broken indent

gustavogama-cll · 2026-02-12T20:17:31Z

docs/docs/usage/building-proposals.md

 timelockBuilder.AddOperation(types.BatchOperation{
-  ChainSelector: selector,
-  Transactions:  []types.Transaction{tx},
+ChainSelector: selector,


broken indent

gustavogama-cll · 2026-02-12T20:18:39Z

e2e/ledger/ledger_test.go


 	// set root
-	executable, err := mcms.NewExecutable(proposal, executorsMap)
+	// TODO: we should pass the context once we remove background context in the remote chain selectors api


I see this TODO spread all over the place. It's doing more harm than good, imo, because it's very noisy. How about you simply add a link to the Jira ticket after the //nolint:contextcheck comment?

gustavogama-cll · 2026-02-12T20:19:54Z

e2e/ledger/ledger_test.go

+	// TODO: we should pass the context once we remove background context in the remote chain selectors api
+	executable, err := mcms.NewExecutable(proposal, executorsMap) //nolint:contextcheck


for instance

Suggested change

// TODO: we should pass the context once we remove background context in the remote chain selectors api

executable, err := mcms.NewExecutable(proposal, executorsMap) //nolint:contextcheck

executable, err := mcms.NewExecutable(proposal, executorsMap) //nolint:contextcheck //OPT-400 - remove bg ctx

gustavogama-cll · 2026-02-12T20:22:13Z

e2e/tests/ton/executable.go


-	s.ChainB = types.ChainSelector(cselectors.GETH_TESTNET.Selector)
-	s.ChainC = types.ChainSelector(cselectors.GETH_DEVNET_2.Selector)
+	s.ChainB = types.ChainSelector(chainsel.ETHEREUM_TESTNET_SEPOLIA.Selector)


isn't it a problem to use real chain selectors for the e2e tests? Why do we need to change them?

gustavogama-cll · 2026-02-12T21:37:18Z

taskfiles/lint/Taskfile.yml

    cmds:
-      - golangci-lint run
-      - golangci-lint run --build-tags="e2e,test"
+      - golangci-lint run --config=.golangci.yml


isn't .golangci.yml the default?

gustavogama-cll · 2026-02-12T21:38:17Z

types/chain_selector.go

+	// this function is used in a lot of places, and passing the context through all of them would be a big breaking change
+	// so a bigger refactor may be needed to properly pass context through all the layers that use this function
+	ctx := context.Background()
+	details, err := chainselremote.GetChainDetailsBySelector(ctx, uint64(sel))


isn't the fallback already implemented in cldf now?

gustavogama-cll · 2026-02-12T21:42:39Z

timelock_proposal.go


 // buildTimelockConverters builds a map of chain selectors to their corresponding TimelockConverter implementations.
-func (m *TimelockProposal) buildTimelockConverters() (map[types.ChainSelector]sdk.TimelockConverter, error) {
+func (m *TimelockProposal) buildTimelockConverters(_ context.Context) (map[types.ChainSelector]sdk.TimelockConverter, error) {


why are we passing a context if it won't be used?

gustavogama-cll

a couple of general comments:

I think mixing the "chainsel" renames with the remote API in the same PR was a bad idea... too much noise, and it'll make it hard to review a possible revert, later
if feeds that there's a lot of api in the mcms library that should be public; that's why I prefer to place all the implementation in /internal until there's a clear use case justifying it

feat: refactor usages of chain selectors API to use remote API

b5142fb

Signed-off-by: Pablo <[email protected]>

ecPablo commented Feb 6, 2026

View reviewed changes

ecPablo and others added 6 commits February 6, 2026 16:44

feat: standardize imports and use remote chain selector API

6148b74

Signed-off-by: Pablo <[email protected]>

fix: add TODOs for better context management.

68665f1

Signed-off-by: Pablo <[email protected]>

Merge branch 'main' into ecpablo/refactor-remote-chain-selectors

15af2cb

fix: lint errors

2584f89

Signed-off-by: Pablo <[email protected]>

fix: unit tests and bump chain selectors

65136f4

Signed-off-by: Pablo <[email protected]>

fix: revert remote api on tests

6531c81

Signed-off-by: Pablo <[email protected]>

ecPablo commented Feb 10, 2026

View reviewed changes

ecPablo added 11 commits February 10, 2026 15:45

fix: encoder_test.go to not use remote api

ea7ac71

Signed-off-by: Pablo <[email protected]>

fix: todo comments

f7c63af

Signed-off-by: Pablo <[email protected]>

chore: changeset

4560f61

Signed-off-by: Pablo <[email protected]>

fix: lint errors

37fe9ae

Signed-off-by: Pablo <[email protected]>

fix: lint errors

d54b14b

Signed-off-by: Pablo <[email protected]>

fix: lint errors

e757600

Signed-off-by: Pablo <[email protected]>

fix: lint errors

e36ecd6

Signed-off-by: Pablo <[email protected]>

fix: lint errors

b3db1ff

Signed-off-by: Pablo <[email protected]>

fix: tests

ff71d17

Signed-off-by: Pablo <[email protected]>

fix: lint errors

bc2c4e9

Signed-off-by: Pablo <[email protected]>

fix: bump selectors

3f6c5f0

Signed-off-by: Pablo <[email protected]>

ecPablo marked this pull request as ready for review February 11, 2026 15:47

ecPablo requested a review from a team as a code owner February 11, 2026 15:47

product-security-plaid-production bot requested a review from DimitriosNaikopoulos February 11, 2026 15:47

ChrisAmora previously approved these changes Feb 11, 2026

View reviewed changes

gustavogama-cll reviewed Feb 12, 2026

View reviewed changes

docs/docs/usage/building-proposals.md Outdated Show resolved Hide resolved

fix: indentation in docs example

5a07554

Signed-off-by: Pablo <[email protected]>

ecPablo dismissed ChrisAmora’s stale review via 5a07554 February 12, 2026 12:58

gustavogama-cll reviewed Feb 12, 2026

View reviewed changes

docs/docs/usage/building-proposals.md

IsSigner: false,

IsWritable: true,

}

{

Copy link

Contributor

gustavogama-cll Feb 12, 2026

Choose a reason for hiding this comment

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

broken indent

gustavogama-cll reviewed Feb 12, 2026

View reviewed changes

docs/docs/usage/building-proposals.md

)

if err != nil {

panic(err)

panic(err)

Copy link

Contributor

gustavogama-cll Feb 12, 2026

Choose a reason for hiding this comment

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

broken indent

gustavogama-cll reviewed Feb 12, 2026

View reviewed changes

		// TODO: we should pass the context once we remove background context in the remote chain selectors api
		executable, err := mcms.NewExecutable(proposal, executorsMap) //nolint:contextcheck

feat: refactor usages of chain selectors API to use remote API #614

Are you sure you want to change the base?

feat: refactor usages of chain selectors API to use remote API #614

Uh oh!

Conversation

ecPablo commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cl-sonarqube-production bot commented Feb 12, 2026

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gustavogama-cll left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ecPablo commented Feb 6, 2026 •

edited

Loading

changeset-bot bot commented Feb 6, 2026 •

edited

Loading