-
Notifications
You must be signed in to change notification settings - Fork 25
Move chaintype package to common #1795
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
Conversation
|
👋 cedric-cordenier, 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! |
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
This PR moves the chaintype package to a common location to enable its use in the corekeys package. The change involves creating a new chaintype package with types and utilities for managing different blockchain types.
Changes:
- Introduces a new
chaintypepackage withChainTypestring type and constants for supported chains (EVM, Cosmos, Solana, StarkNet, Aptos, Tron, TON, Sui, Offchain) - Implements conversion functions between
ChainTypeand numeric representations - Provides validation utilities for supported chain types
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
✅ API Diff Results - No breaking changes |
baad729 to
1bcf679
Compare
| ) | ||
|
|
||
| // ChainType denotes the chain or network to work with | ||
| type ChainType string |
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.
Don't we have a chain family enum for this somewhere in common already?
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.
I can't find one exactly, but we use string fields and reference chain selector consts in a few places. Could we do that here too?
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.
Hmmm let me take a look at the chain selector consts to see if that'll work.
Wdym by string fields though? Can I point me to an example?
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.
Ah looks like I can just use FamilyEVM in chain-selectors? Is that what you had in mind?
This is needed to use these types in the corekeys package which I'm creating atm.