-
Notifications
You must be signed in to change notification settings - Fork 3
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
Tag evm only get selector as deprecated #78
Conversation
…to deprecate-evm-only-get-selector
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.
great, thanks. i was tripping over this yesterday
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.
actually, please also depreciate the other funcs that are EVM specific and superceded by GetXXX
SelectorFromChainId
NameFromChainId
???```
@@ -69,6 +69,7 @@ func EvmChainIdToChainSelector() map[uint64]uint64 { | |||
return copyMap | |||
} | |||
|
|||
// Deprecated, this only supports EVM chains, use the chain agnostic `GetChainIDFromSelector` instead |
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.
hm are we sure we want to direct to that and not some new EVMChainIdFromSelector which does have the uint64 typing? I think if ChainIdFromSelector was used in an EVM specific context (say in the EVM relayer) it'd be annoying to have to strconv
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.
in that case I guess users can still rely on the evm specific getter. That's why I think it will be important to at some point create submodules for all these methods, to make the switch explicit.
I'm going to move forward with the merge, however I'm happy to continue this topic in slack
Tag the
ChainIdFromSelector
method as deprecated since it only supports evm chains, and we are expanding support to non evm.If needed, people can still use it and we don't immediately need to migrate away from it.
If someone needs to also validate that a given chain is evm, they can also use the
GetSelectorFamily
with the same selector.