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

CPS-0008? | Domain Name Resolution #605

Merged
merged 12 commits into from
Aug 20, 2024
Merged

Conversation

HinsonSIDAN
Copy link
Contributor

@HinsonSIDAN HinsonSIDAN commented Oct 14, 2023

As the ecosystem emerges, more and more domain service projects entering Cardano. Currently noticeably there are 3 domain projects:

  1. ADA Handle
  2. adadomains
  3. Cardano Name Service

Both adadomains and Cardano Name Service has chosen .ada as their domain suffix. When proceeding with wallet integration on address resolving, one common issue faced is the ambiguity in resolving approach, which could lead to potentially serious loss of fund in sending fund to undesired recipients.

The CIP is proposed accordingly to address the community feedback and suggestion.


(rendered proposal in branch)

@HinsonSIDAN HinsonSIDAN changed the title CIP???? | Domain Address Resolving Standard CIP-???? | Domain Address Resolving Standard Oct 14, 2023
@rphair rphair added the Category: Wallets Proposals belonging to the 'Wallets' category. label Oct 14, 2023
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.

I don't know if this CIP is specific enough to be usable by wallets (I'm not saying it's not). At first reading it seems to set a common-sense behavioural guideline & therefore any wallet (all those mentioned in Path to Active) would probably "agree with the approach" as you say there... but we'd never know for sure, and neither would they, unless your approach were detailed more exactly.

A more specific example of how exactly a wallet is expected to behave under these circumstances... something that's not being done already... would help show how necessary this CIP might be. (Note I don't work directly with these implementations & we have a couple other editors @Ryun1 @Crypto2099 who work extensively with wallets & their standards, and so they might be able to see this from what you have written.)

CIP-?/README.md Outdated Show resolved Hide resolved
CIP-?/README.md Outdated Show resolved Hide resolved
CIP-?/README.md Outdated Show resolved Hide resolved
CIP-?/README.md Outdated Show resolved Hide resolved
CIP-?/README.md Outdated Show resolved Hide resolved
CIP-?/README.md Outdated
- Every participating Cardano domain service provider provides either a desired prefix or suffix.
- Wallet providers to execute and integrate with resolving address, domain service project to provide assistance.

#### Participating Cardano Domain Service
Copy link
Collaborator

@rphair rphair Oct 14, 2023

Choose a reason for hiding this comment

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

One way or another this item should be removed, since it's not a standard part of Path to Active:

  • Based on the branch name I am assuming this is your homegrown solution: if so you can cite it as an implementation & it would address my original reservation (about not being specific enough) if you could talk in your Specification about how your own system works.
  • If you think your service is beyond the scope of your CIP, you might put it in Rationale to talk about how & why the CIP is designed to accommodate implementations like your own.
  • Regardless of how much detail you go into, it should instead be an item in your Implementation Plan since you appear to have a working model (or plan to have one).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I just need to remove the line #### Participating Cardano Domain Service right? I think the content of the session is necessary to stay for community to take reference (and also its part of the implementation).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just please make sure that whatever content remains fits into the section where it appears to be. If it's in either of the two Path to Active sections, it should fit that section as described here: https://github.com/cardano-foundation/CIPs/tree/master/CIP-0001#path-to-active

CIP-?/README.md Outdated Show resolved Hide resolved
CIP-?/README.md Outdated Show resolved Hide resolved
@HinsonSIDAN
Copy link
Contributor Author

@rphair
I don't know if this CIP is specific enough to be usable by wallets (I'm not saying it's not). At first reading it seems to set a general behavioural guideline which seems like common sense & therefore any wallet (all those mentioned in Path to Active) would probably "agree with the approach" as you say there... but we'd never know for sure, and neither would they, unless your approach were detailed more exactly.

A more specific example of how exactly a wallet is expected to behave under these circumstances... something that's not being done already... would help show how necessary this CIP might be. (Note I don't work directly with these implementations & we have a couple other editors @Ryun1 @Crypto2099 who work extensively with wallets & their standards, and so they might be able to see this from what you have written.)

Just added some specific examples on the expected behaviour. Does it address your concern, or is it better for me to try to pull wallets include discussion here as well?

@Ryun1
Copy link
Collaborator

Ryun1 commented Oct 15, 2023

I agree with @rphair, such wallet behaviours are not normally under the remit of CIPs. I believe this proposal to be more best practices rather than a technical problem and a solution. But I would be keen to hear from Wallet providers on this, perhaps this is something that they would be interested in pursuing.

@rphair
Copy link
Collaborator

rphair commented Oct 15, 2023

OK then @SIDANWhatever @Ryun1 - we can talk about both issues (CIP proper scope & how to get wallet devs into the discussion) on Tuesday; I've added it to the agenda: https://hackmd.io/@cip-editors/75

@perturbing
Copy link
Collaborator

I'm on board with the idea of standardizing "DNS" tokens, much like what the Ada Handle team aimed for in this old PR. That one got closed since it was too specific to Ada Handle's "DNS" design, unlike this proposal which seems more general. I see value here but fitting it into the bigger picture needs some more thought. Looking forward to seeing how this evolves and blends with other initiatives in the ecosystem.

@HinsonSIDAN HinsonSIDAN changed the title CIP-???? | Domain Address Resolving Standard CPS-???? | Domain Address Resolving Standard Oct 17, 2023
@HinsonSIDAN
Copy link
Contributor Author

As per discussion in bi-weekly call, amended the proposal with format of CPS. Appreciate any suggestion on how could I move contents into suitable session (especially here I have a proposed solution or standard to this particular CPS).

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

rphair commented Oct 17, 2023

thanks @SIDANWhatever - this definitely resolves the difficulty of the criteria "3 of wallets listed below agree with the approach" since that should occur in discussion of the CPS and any other CIPs submitted for it.

re: #605 (comment) - All the content about your own reference implementation can be posted into a separate CIP document with a separate pull request... since these 2 documents will be considered & reviewed independently.

HinsonSIDAN and others added 2 commits October 18, 2023 08:28
Co-authored-by: Ryan Williams <44342099+Ryun1@users.noreply.github.com>
@HinsonSIDAN HinsonSIDAN changed the title CPS-???? | Domain Address Resolving Standard CPS-???? | Domain Information Resolving Standard Oct 26, 2023
@HinsonSIDAN
Copy link
Contributor Author

@rphair Redrafting it into a CPS, I would create another PR for my suggested solution as CIP particularly to the address resolving (since mis-alignment on it might cause loss of fund, it comes with a higher priority).

One thing I would like to hear editors' advice is that, I amended this CPS to address a higher level problem - information resolving as a whole but not address resolving. In this sense, I am not sure which category should I put on this CPS.

I have invited collaboration on the forked repo. Please feel free to edit and comment. Thanks!

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.

@SIDANWhatever yes a CIP for the specific (perhaps more urgent) solution sounds like it would be welcome.

CPS-?/README.md Outdated Show resolved Hide resolved
CPS-?/README.md Outdated Show resolved Hide resolved
@rphair rphair added Category: Tools Proposals belonging to the 'Tools' category. and removed Category: Wallets Proposals belonging to the 'Wallets' category. labels Oct 26, 2023
@rphair rphair requested a review from Ryun1 October 26, 2023 18:25
@rphair rphair changed the title CPS-???? | Domain Information Resolving Standard CPS-???? | Domain Name Resolution Oct 26, 2023
@rphair
Copy link
Collaborator

rphair commented Oct 26, 2023

Putting this on next CIP meeting agenda for Review since this proposal has changed shape a lot since it was presented in Triage the last time: https://hackmd.io/@cip-editors/76

@SamDelaney
Copy link
Contributor

Another relevant use case: Anastasia Labs is working on "Smart Beacons" as of Fund 10. https://www.lidonation.com/en/proposals/anastasia-labs-smart-beacons-router-nfts-f10.

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.

Promoted to CPS candidate at today's meeting; thanks @SIDANWhatever for your attendance & progressive work... please change this directory name to CPS-0008 and update the "rendered proposal" link at the top 🤩

@colll78 in light of #605 (comment) if you are behind https://github.com/Anastasia-Labs/smart-handles then please grace us this PR with a review and/or feedback whenever you see fit 🤓

CPS-?/README.md Outdated Show resolved Hide resolved
@rphair rphair changed the title CPS-???? | Domain Name Resolution CPS-0008? | Domain Name Resolution Nov 15, 2023
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.

@HinsonSIDAN I can't see that this problem (as stated in your CPS) has gone away, and in fact I've referred to your exposition of the problem a few times when discussing the new work on Cardano Payment URLs which I believe will require some means of settling domain name conflicts (cc @Crypto2099).

So although it's been somewhat "abandoned" (currently we're finding many such proposals in the CIP repo PR queue) I'm going to vote that this should be merged and therefore tag Last Check which which will trigger its appearance on a CIP meeting soon: maybe the one tomorrow.

As far as I'm concerned this passed review a long time ago (and somehow slipped through cracks in editors' attention) & no further action is required on your part, but if there is anything you need to add please update it ASAP.

@rphair rphair added the State: Last Check Review favourable with disputes resolved; staged for merging. label Aug 20, 2024
@rphair rphair requested a review from Crypto2099 August 20, 2024 02:45
Copy link
Collaborator

@Ryun1 Ryun1 left a comment

Choose a reason for hiding this comment

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

This one slipped through the cracks
Happy to see it merged

@rphair rphair merged commit 21002c8 into cardano-foundation:master Aug 20, 2024
@rphair rphair removed the State: Last Check Review favourable with disputes resolved; staged for merging. label Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Tools Proposals belonging to the 'Tools' category.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants