Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(ipns): reading records with raw []byte Value #830

Merged
merged 3 commits into from
Feb 4, 2025

Conversation

lidel
Copy link
Member

@lidel lidel commented Feb 3, 2025

This PR aims to improve / maximize interop with legacy/alternative IPNS clients by restoring support for []byte CID in Value field and gracefully handling empty Value in scenarios where IPNS is used for its Data field, and not IPFS paths.

  • record.Value() will convert a binary CID to a valid path.Path.
  • Empty Value() will produce NoopPath (/ipfs/bafkqaaa) to avoid breaking existing code that expects a valid record to always produce a valid content path.
  • tests to ensure we test both + error if Value() can't produce a meaningful path

Thank you @ianopolous and @aschmahmann for catching this.

If / once this lands, I'll update phrasing in https://specs.ipfs.tech/ipns/ipns-record/ to make it clear binary CID and empty Value are both allowed.

Aiming to include this in Kubo 0.34.

- []byte CID works again
- empty value is allowed and produces NoopPath
@lidel lidel mentioned this pull request Feb 3, 2025
31 tasks
Copy link

codecov bot commented Feb 3, 2025

Codecov Report

Attention: Patch coverage is 84.21053% with 3 lines in your changes missing coverage. Please review.

Project coverage is 60.51%. Comparing base (1ff84ce) to head (c520ac2).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
ipns/record.go 84.21% 2 Missing and 1 partial ⚠️

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #830      +/-   ##
==========================================
+ Coverage   60.42%   60.51%   +0.08%     
==========================================
  Files         244      244              
  Lines       31120    31136      +16     
==========================================
+ Hits        18804    18841      +37     
+ Misses      10633    10615      -18     
+ Partials     1683     1680       -3     
Files with missing lines Coverage Δ
ipns/record.go 69.81% <84.21%> (+1.59%) ⬆️

... and 12 files with indirect coverage changes

@lidel lidel marked this pull request as ready for review February 3, 2025 22:20
@lidel lidel requested a review from a team as a code owner February 3, 2025 22:20
@gammazero gammazero merged commit 8d689c8 into main Feb 4, 2025
15 checks passed
@gammazero gammazero deleted the fix/ipns-allow-binary-values branch February 4, 2025 16:54
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants