Skip to content
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

Aleo: Add CAIP-2 and CAIP-10 specs #90

Merged
merged 21 commits into from
Jan 18, 2024
Merged

Conversation

jonandgon
Copy link
Contributor

This is the start of adding CAIP specs for the Aleo Network. Aleo is a privacy-focused L1 that uses zero-knowledge proofs.

@JonathanConn
Copy link

Hello 👋
My name is Jon, I am with WalletConnect
We need this PR merged to support their chain @bumblefudge

cc @jonandgon

@jonandgon
Copy link
Contributor Author

@bumblefudge @JonathanConn what work needs to be done to get this merged? I can spend some time improving the PR, but I would like some feedback if you guys have any.

Thanks!

@bumblefudge
Copy link
Collaborator

bumblefudge commented Oct 26, 2023

hey @jonandgon there are minor/formatting change requests above as well as content questions I'd love answered (may not need changes depending on the answers, or i may be able to add whatever's missing otherwise), nothing blocking other than that

@jonandgon
Copy link
Contributor Author

jonandgon commented Oct 30, 2023

Hey @bumblefudge I'm not seeing the change requests / questions, can you send a link to it? Or I may not have view access to it.

thank you!!

@bumblefudge
Copy link
Collaborator

#90 ? i am guessing you're seeing these messages via email instead of on github dot com?

@jonandgon
Copy link
Contributor Author

yeah, I'm getting email notifications - but I respond directly on github.com.

If you could just resend the comments/questions somehow (screenshots work) I can get the fixes in

aleo/README.md Outdated Show resolved Hide resolved
aleo/caip10.md Outdated Show resolved Hide resolved
aleo/README.md Outdated Show resolved Hide resolved
aleo/caip10.md Outdated Show resolved Hide resolved
aleo/caip2.md Outdated Show resolved Hide resolved
aleo/caip2.md Outdated Show resolved Hide resolved
aleo/caip2.md Outdated

## Syntax

The Aleo chain ID is the name of the chain. e.g. `testnet3` or `mainnet` when mainnet is released.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mmmm, that's more semantics than syntax. What are valid chain IDs? Are they case-sensitive? Canonically lowercase? Max length? etc etc. A reg-exp never hurts, if it's already specified somewhere.

aleo/caip2.md Outdated Show resolved Hide resolved
aleo/caip2.md Outdated Show resolved Hide resolved
@bumblefudge
Copy link
Collaborator

If you could just resend the comments/questions somehow (screenshots work) I can get the fixes in

Terribly sorry, I didn't realize my suggestions were all "pending" because I hadn't "submitted" my review! Mea culpa

jonandgon and others added 9 commits November 9, 2023 10:43
Co-authored-by: Bumblefudge <caballerojuan@pm.me>
Co-authored-by: Bumblefudge <caballerojuan@pm.me>
Co-authored-by: Bumblefudge <caballerojuan@pm.me>
Co-authored-by: Bumblefudge <caballerojuan@pm.me>
Co-authored-by: Bumblefudge <caballerojuan@pm.me>
Co-authored-by: Bumblefudge <caballerojuan@pm.me>
Co-authored-by: Bumblefudge <caballerojuan@pm.me>
@jonandgon
Copy link
Contributor Author

@bumblefudge - just addressed all your comments, let me know if there's anything else!

@bumblefudge
Copy link
Collaborator

Hey @jonandgon sorry for the back-and-forth. I'm still a little baffled on the chain identifiers-- how do you know you're on testnet3 and not some private network? On most blockchain networks, there is an RPC command to ask the node what network/consensus-community/chainID it's on-- on L1s where chains are identified by genesis hash, the node would return that for the its network, and on L1s where network identifiers are hardcoded into the protocol, a human-readable name or integer/ASCII-character usually comes back. Are testnet3 and mainnet values encoded anywhere, or is network switching not codified yet? Looking at the docs it seems like network identifiers are hardcoded into the RESTful endpoint paths, which would imply short URL-friendly human-readable slugs are, by design, the primary identifier rather than machine-readable ones or hashes. But are there constraints on possible future network identifiers?

The regexp here is a little unnecessary because if you're just specifying the two valid values, this spec will be out of date when a new public network is launched. More importantly, it does not provide any guidance for validating a private network, if those can be spun up-- usually we ask for a regexp of all possible valid network identifiers, not an ENUM expressed as a regex :D

@jonandgon
Copy link
Contributor Author

jonandgon commented Dec 14, 2023

ok so I got an update from the Aleo team regarding this.

in addition to network identifiers hardcoded into the REST endpoint, the network ID is output from the block data itself. For example, if you call https://api.explorer.aleo.org/v1/testnet3/latest/block, you will see that the network is 3 for testnet3. Mainnet will have a network ID of 0 The network is a u16 number.

Unfortunately, since there is no official support for local devnets, it's hard to say what the network ID for that would be, but since there is 65535 possible ids, they could easily add one.

I will update the spec to reflect this new info. Thanks for the patience!

@jonandgon
Copy link
Contributor Author

annnnddd @bumblefudge the specs have been updated. thank you for the help, and please lmk what else needs to be done!

@bumblefudge
Copy link
Collaborator

Cool, I just added a "contents may shift after mainnet launch" disclaimer to the CAIP-2 profile and it's GTG as far as i'm concerned. one more thumbs up from another CASA reviewer and it's shipped

@jonandgon
Copy link
Contributor Author

@bumblefudge any update from the 2nd reviewer? thanks

@obstropolos obstropolos self-requested a review January 18, 2024 18:11
Copy link
Contributor

@obstropolos obstropolos left a comment

Choose a reason for hiding this comment

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

All looks good to me - looking forward to the mainnet launch noted in the CAIP!

@bumblefudge bumblefudge merged commit f95019b into ChainAgnostic:main Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants