-
Notifications
You must be signed in to change notification settings - Fork 72
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
Mina: Add namespace / CAIP-2 #91
Conversation
hi @bumblefudge is there anything else I need to do / add etc for this to be reviewed? |
mina/caip2.md
Outdated
- `devnet` | ||
- `berkeley` | ||
|
||
The human readable identifiers above may be composed to add description using `-` as a separator. |
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.
this is a little opaque-- are testnet-berkeley
and testnet
two different networks, or is the former a subset of the nodes of the latter that are running the berkeley nightly/feature-branch of the testnet codebase? i guess i'm confused what "compose" means when discussing network identifiers!
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.
hi, in this case maybe concatenated is better, will update ;)
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.
yes two different networks - at the moment Berkeley is a test network on which zkApps (smart contracts) are supported, (testnet-berkeley would be a testnet for the Berkeley test network - this doesn't exist but could if it was required) - to be merged to mainnet, testnet is the test network for the mainnet. Idea was to define an "all" encompassing naming convention that would also support any future networks.
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, ok. but is this convention already defined somewhere in the code or in the community? seeing that nodes return a networkID
value implies a convention has already been set somewhere by whomever decides what a valid networkID
is... is there a normative reference maybe? an MIP or even just a permanent URL in the docs?
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.
hi, by "this convention already defined" - do you mean berkeley/devnet/testnet etc names? Or do you mean concatenating testnet-berkeley - if we spun up a test network for the berkeley network? The names are embedded in the code ... the ability to concatenate is intended to "future proof" the convention in case we have more networks, it's new though we can add a definition of this to the docs and link it if it helps ...
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.
heh, I guess what I mean by "convention definition" is that nothing internal (or native) to the namespace should be defined for the first time here at CASA, or canonically in a CASA spec-- the CASA spec is downstream of mina docs, the only thing being defined here is the export format for cross-chain purposes. so I guess what I mean is that if there are rules about what future network names will be valid (and instructions on how to embed those in code when you generate a new network/testnet/etc), linking to that from the references would be great! I highly recommend adding those kinds of definitions to the official docs sooner than later, because you never know who's tinkering a private repo with the expectation of being able to publicly add their private network to the list of known/public chainIds down the road...
mina/caip2.md
Outdated
|
||
## Syntax | ||
|
||
The blockchain namespace and blockchain identifiers will be in lowercase. |
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.
maximum length? whitespace allowed? other characters, diacritics, etc? a regex never hurts :D
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'll add a regex ;D
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.
hi @bumblefudge - added a regex and, hopefully ;), clarified what the possible names can be ... does it look ok now?
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.
If it is, I'll post a link to think PR in the mina discord so the mina community can comment
Apologies, the queue has been a little backed up with travel and conferences! This looks fine except the nits mentioned above, can I get an independent review from a tooling company or experience Mina dev just in case I'm missing something out of ignorance? Also, I noticed the Rosetta link in the references-- this is a cool open-source standards-support feature to highlight for cross-chain dapps and exchanges. One way of doing this would be to add that line to the references from the |
@bumblefudge - thanks for the review! - will make some updates and come back to you ;D |
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.
LGTM. if, down the road, the chainID naming convention gets formalized a bit more, remember to update this to point to whatever registry/IP-process/etc governs what names are valid and what names to be suspicious of!
hi @bumblefudge - can this be merged? Or do I need to add more? |
hey sorry to do this to you but i dug into the docs again and noticed that the RPCs actually use a machine-readable hash to identify networks-- which is what i was asking about before, why invent a human-readable mapping that isn't canonically published anywhere? |
in other words, if "mainnet" is actually |
hi @bumblefudge, sorry missed this ;) This was discussed internally and the product team were keen to have a human readable name so the graphQL was extended to support this. Do you think we need to reappraise this or can we push forward with the proposal? |
interesting-- if human-readable is how the chainId system is actually working in the community's code "in the wild", then sure, the CASA scheme should reflect that. But my main concern is keeping CASA's docs maximally "downstream" of more canonical ones with lots of mina-community eyes on them and thus maximally low-maintenance. do you have a link to somewhere in the maintained docs about this mapping? When I google around, all I see are references to |
@halsaphi The next CASA call is on 14th of December. I guess, it makes sense for you to join there, so that we could have a chance to go through that change in a synchronous fashion, if @bumblefudge does not mind. Disclaimer: I like what Mina is doing. |
hey im not blocking the design I just want the upstream docs updated so I can link to them and know theyll be updated there when future mappings get added! attending an editorial meeting isn't a requirement but always welcome |
"I just want the upstream docs updated so I can link to them" - yeah, I understand, and fully support the concern. Mina should have some sort of "Improvement Proposals" or RFC standardisation process on their own to specify available network names. |
if there isn't a page in the Mina docs and getting one added is out of your hands, just put in a comment like, "at time of writing the current recognized values are the ones listed above and no canonical human-readable or machine-readable mapping has yet been published. In future, the Mina official docs will link to such a mapping once one is published." |
hi @bumblefudge - sorry been busy at some events in Taipei - I'll add your comment to the spec and work on getting the mina docs updated ... once the docs contain the canonical mappings I'll update with a link to the docs. |
There is a update to the docs coming in Jan/Feb - will have the canonical mappings in that update and will then update the files here with links. |
@bumblefudge @ukstv - I have added a link to some updated documentation showing the network identifiers and how to resolve the network identifier. Can this be merged now? Is there anything I need to do for this? Thanks |
@bumblefudge and @ukstv - please note there is a planned upgrade to the Mina Networks (adding zkApps) - and that post this upgrade the documentation linked to above will move to https://docs.minaprotocol.com/ - I will update the link to reflect this post upgrade. |
@bumblefudge This looks okay by me. What do you think? |
hi @bumblefudge and @ukstv - how do I get the second approving review? |
@halsaphi I guess, the next step is to wait till the next CASA call. Usually, we merge PRs like this during our meeting. AFAIK, it is on April the 4th. |
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.
It's an honour to me!
Upd: Mea culpa. Removed "sirs" at the end, as it is gender exclusive.
hi @bumblefudge and @ukstv thanks for all your help and patience with this ! :) |
This is the initial namespace specification / CAIP-2 for the Mina Protocol. The Mina Protocol is a L1 built from the ground up to use zero-knowledge proofs.