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

CIP-0142? | Web-Wallet Bridge - Network Determination #972

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

nathanbogale
Copy link

@nathanbogale nathanbogale commented Jan 23, 2025

This PR introduces a new CIP that extends CIP-0030 to allow dApps to determine the specific network magic number of the connected Cardano network. While the existing getNetworkId() function only distinguishes between mainnet (1) and testnet (0), this extension enables dApps to identify specific test networks (preview, preprod, etc.) through their magic numbers.

Key features:

Adds getNetworkMagic() function under cip-0142 namespace
Maintains full backwards compatibility with CIP-0030
Reuses existing error handling patterns
Simple, focused solution for network identification
This extension will help dApps better configure themselves based on the specific test network they're connected to, improving the development and testing experience in the Cardano ecosystem.

Resolves: #489


(rendered proposal)

nathanbogale and others added 6 commits January 9, 2025 19:47
updated the CIP number in readme

Co-authored-by: Robert Phair <[email protected]>
added description to the motivation section

Co-authored-by: Robert Phair <[email protected]>
added description to the rationale section

Co-authored-by: Robert Phair <[email protected]>
@rphair
Copy link
Collaborator

rphair commented Jan 23, 2025

@nathanbogale - before we continue with this one - why did you pull the plug on #960? We need to follow a proper review process in every case: and the other PR was already discussed at meetings, obtained online discussion and review, etc. This community interaction makes these documents quite different than just the posted material.

@Ryun1 @Crypto2099 @perturbing I would recommend that this PR be closed and #960 be reopened to preserve that continuity: unless some practical argument is made against that here. Of course we will need the author's cooperation to do that and hope consensus is achieved about that before any action is taken in this PR.

@Ryun1
Copy link
Collaborator

Ryun1 commented Jan 23, 2025

@Ryun1 @Crypto2099 @perturbing I would recommend that this PR be closed and #960 be reopened to preserve that continuity

I second this!
we dont want to spread the discussion across two pull requests

@rphair
Copy link
Collaborator

rphair commented Jan 23, 2025

OK @nathanbogale - with @Ryun1's seconding the motion, I am bouncing this & reopening the original PR. If there is some critical flaw of the original PR please post exactly what that is & we will revise accordingly.

@rphair rphair closed this Jan 23, 2025
@rphair
Copy link
Collaborator

rphair commented Jan 23, 2025

I see @nathanbogale that you have destroyed PR #960 by deleting its branch. I sincerely hope you never do anything like that again: especially after editors have devoted their time to reviewing at a meeting, assigning a PR number, and conducted active and committed reviews. Since this has effectively become an unreviewed PR I'll remove the CIP number assignment and tag it for Triage again.

@rphair rphair reopened this Jan 23, 2025
@rphair rphair added State: Triage Applied to new PR afer editor cleanup on GitHub, pending CIP meeting introduction. Category: Wallets Proposals belonging to the 'Wallets' category. labels Jan 23, 2025
@nathanbogale
Copy link
Author

@rphair and @Ryun1
I understand the inconveniences created.
It was simply to have more aligned and less confusing structures and naming for reviewers and community interaction overall that i've changed the branch name.
Will be following the process more strictly moving onward.

@rphair rphair changed the title Cip 0142: Web-Wallet Bridge - Network Determination CIP-XXXX: Web-Wallet Bridge - Network Determination Jan 23, 2025
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

Due to #960 (comment) we will have to leave both PRs in the Discussions: list:

CIP-0142/README.md Show resolved Hide resolved
@rphair
Copy link
Collaborator

rphair commented Jan 23, 2025

It was simply to have more aligned and less confusing structures and naming for reviewers and community interaction overall that i've changed the branch name.

@nathanbogale a recommendation in CIP-0001 is to use the purpose of the CIP rather than a void number: e.g. CIP-network-magic (which you could also use for the initial directory name)

@rphair
Copy link
Collaborator

rphair commented Jan 23, 2025

@nathanbogale regarding #972 (comment) I meant that assigning a generic name in place of a CIP number is simply a good way of providing a convenient branch name that doesn't have to change. I didn't realise that you were going to be backing the number 142 out of the entire document & imposing the name all through the document itself.

You can reinstate the number 142 since it was going to be reserved for this CIP anyway... so please reverse daae1b8 except for the Discussions: link. And please rename the directory to CIP-0142 (again).

@Ryun1 @Crypto2099 @perturbing I've looked through the new branch & it's a practically identical copy of the deleted one, so effectively it's been Triaged already and so I'm reinstating the Confirmed status as per this week's review. We've confirmed it wasn't the intent of the author to divert review into a different thread & this was all basically a fluke of generally undocumented GitHub behaviour 😎 (though still common sense not to rename a branch after submitting a PR)

@rphair rphair changed the title CIP-XXXX: Web-Wallet Bridge - Network Determination CIP-0142? | Web-Wallet Bridge - Network Determination Jan 23, 2025
@rphair rphair added State: Confirmed Candiate with CIP number (new PR) or update under review. and removed State: Triage Applied to new PR afer editor cleanup on GitHub, pending CIP meeting introduction. labels Jan 23, 2025
@nathanbogale
Copy link
Author

@rphair this is handled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Wallets Proposals belonging to the 'Wallets' category. State: Confirmed Candiate with CIP number (new PR) or update under review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CIP-30: Add getNetworkName() Function to API
3 participants