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

add dnslink namespace #268

Merged
merged 2 commits into from
Jun 14, 2022
Merged

add dnslink namespace #268

merged 2 commits into from
Jun 14, 2022

Conversation

lidel
Copy link
Member

@lidel lidel commented May 22, 2022

Rationale:

  • been around since the very early days, seems we just never added it because /ipns/ was good enough for existing use cases
    • we already have dnsaddr, which works similar way, but for multiaddrs (TXT records), adding dnslink removes the uncertainty if someone wants to use codec to distinguish between the two
    • cc @Stebalien in case I am missing any reason we should not have this as a codec
  • we want to support DNSLink chaining without /ipns/ in TXT records, so it is useful for non-IPFS use cases
  • we want a clear way for pinning DNSLink names within Pinning Service API (Proposal: IPNS pinning ipfs/pinning-services-api-spec#85), that is separate from pinning IPFS (cryptographic records)

@rvagg
Copy link
Member

rvagg commented May 23, 2022

sgtm, but could we bump this above 0x7f to save that precious single byte space? or do we anticipate the codec code being used enough to warrant a short code?

@mrodriguez3313
Copy link

I understand the problem, but I don't understand the solution. Would adding this one csv entry, allow go-ipfs to interpret /dnslink? Thus we would re-add (later on) this feature in the dnslink docs to say, 'Hey! If you're using dnslink for your site, use "/dnslink" instead of "/ipns". Because of this...! :

  • we want to support DNSLink chaining without /ipns/ in TXT records, so it is useful for non-IPFS use cases
    dnslink=/dnslink/example.com
  • we want a clear way for pinning DNSLink names within Pinning Service API (Proposal: IPNS pinning ipfs/pinning-services-api-spec#85), that is separate from pinning IPFS (cryptographic records)

lidel added a commit that referenced this pull request May 24, 2022
@lidel
Copy link
Member Author

lidel commented May 24, 2022

@rvagg I don't think it will be as useful as /ip4, more likely usage will be closer to ipfs-ns and ipns-ns ones. Moved to 0xe8 – would this work?

@mrodriguez3313 It is not enough, this PR is only a formal prerequisite that needs to happen before we implement support for mentioned use cases.

@RangerMauve
Copy link

What would CIDs with this new codec look like?

@lidel
Copy link
Member Author

lidel commented Jun 8, 2022

@RangerMauve bit bizarre, but in places where DNSLink has to be passed as a CID, one could inline DNS name (ASCII) using identity multihash, something like:

$ echo -n 'dnslink-site.example.com' | ipfs add --inline --raw-leaves -Q
bafkqagdenzzwy2lonmwxg2lumuxgk6dbnvygyzjomnxw2

(but with dnslink instead of raw codec)

@RangerMauve

This comment was marked as off-topic.

@lidel
Copy link
Member Author

lidel commented Jun 13, 2022

@RangerMauve agree on many of your points, would appreciate your feedback in discussion on "how to pin mutable content paths" happening in ipfs/pinning-services-api-spec#85.

Let's keep discussion in this PR limited to adding dnslink codec (to be in pair with existing dnsaddr)

@vmx @rvagg ok for me to merge this?

@vmx
Copy link
Member

vmx commented Jun 13, 2022

I know too little about all this, hence this one is not blocked on me.

Rationale:
- been around since  the very early days, seems we just never added it?
- we already have dnsaddr, which works similar way, but for multiaddrs  (TXT records)
- we want to support IPNS-agnostic recursive TXT values `dnslink=/dnslink/example.com`
- we need a way for pinning DNSLink names within Pinning Service API
@rvagg rvagg merged commit d702707 into master Jun 14, 2022
@rvagg rvagg deleted the feat/dnslink-codec branch June 14, 2022 03:52
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.

5 participants