Conversation
|
@jiexi Do you mind renaming the title of this PR so that it will be easier to understand at-a-glance in the commit history later on? |
mcmire
left a comment
There was a problem hiding this comment.
Hey @jiexi, I had some comments below.
I know your PR on the extension side is in flux but I think to understand this PR better it'd be nice to know how you plan on using these functions and tools — could you talk about that a bit?
hmalik88
left a comment
There was a problem hiding this comment.
What's the motivation behind re-doing the work that already exists in @metamask/snaps-utils? https://github.com/MetaMask/snaps/blob/main/packages/snaps-utils/src/namespace.ts
Nice, @hmalik88 ! Didn't know that this was already done. Looks wonderful! I feel like the term "Chain ID" is pretty overloaded now. I don't believe Namespace nor Reference are very clear terms either even though they are the terms used in the CAIP specs. What are your thoughts on this? I think renaming the types to include a |
|
@jiexi You're right, it is an overloaded term. We're releasing the name resolver API soon and I was going to make some type changes. Perhaps you're right that we may want to prefix with |
|
@hmalik88 I'm currently trying to replace our usage of Eth chain id with CAIP chain id. I'm in the process of replacing Do you happen to know if there is an estimate for when these changes will be available? |
|
I've scrapped my types and brought in Caip types from @metamask/snaps-utils. The plan is to make this the new home for these types and to update all usage of @snap-utils to caip types to this new one |
|
Okay. pretty happy with this now |
|
@metamaskbot publish-preview |
mcmire
left a comment
There was a problem hiding this comment.
Just had a couple of questions, but this all seems reasonable to me.
| // Valid caip strings: | ||
|
|
||
| expectAssignable<CaipChainId>('namespace:reference'); | ||
| expectAssignable<CaipChainId>('namespace:'); |
There was a problem hiding this comment.
I'm assuming that this is non-enforceable via TypeScript, is that correct?
There was a problem hiding this comment.
Unfortunately not. We could try to hack it by doing something like this
type Character = "a" | "b" | "c" | "d" | "e" | "f" | "g" | ...
export type CaipChainId = `${Character}${string}:${Character}${string}`;
But I believe this makes a type with n^2 definitions since they get crossmulitplied
| expectAssignable<CaipReference>(`${embeddedString}`); | ||
|
|
||
| expectAssignable<CaipAccountId>('namespace:reference:accountAddress'); | ||
| expectAssignable<CaipAccountId>('namespace:reference:'); |
There was a problem hiding this comment.
Similar as above — I'm assuming this is non-enforceable via TypeScript?
See: MetaMask/core#1489
See: MetaMask/metamask-extension#19991