Skip to content

feat(ipns): helper ValidateWithPeerID and UnmarshalIpnsEntry#294

Merged
hacdias merged 1 commit intomainfrom
feat/reusable-ipns-validation
May 10, 2023
Merged

feat(ipns): helper ValidateWithPeerID and UnmarshalIpnsEntry#294
hacdias merged 1 commit intomainfrom
feat/reusable-ipns-validation

Conversation

@hacdias
Copy link
Copy Markdown
Member

@hacdias hacdias commented May 10, 2023

After my overly excited merge and revert of #292, I looked at the code and saw we were already duplicating existing code. E.g. the key extraction logic already existed and was being duplicated. This PR adds two helper functions ValidateWithPeerID and UnmarshalIpnsEntry. They already use existing functionality. I also added tests.

I did not include IpnsInspectEntry for the reasons here: #292 (comment) - it doesn't make sense for us to be adding it here since it is tightly related to the type we use in ipfs name inspect. We only created it in first place to simplify the type used by the RPC client in Kubo.

I also enabled a test that has been disabled for 5 years in 0320605. It seems to pass now, and reading the commit description, I don't think it is a problem due to the way Peer IDs are encoded nowadays.

Tested in Kubo here: ipfs/kubo#9867

@hacdias hacdias requested review from a team and lidel as code owners May 10, 2023 09:29
@hacdias hacdias requested review from Jorropo and laurentsenta May 10, 2023 09:29
@hacdias hacdias changed the title feat: reusable ipns verify (#292) feat(ipns): helper ValidateWithPeerID and UnmarshalIpnsEntry May 10, 2023
@codecov
Copy link
Copy Markdown

codecov bot commented May 10, 2023

Codecov Report

Merging #294 (f9b6003) into main (dc731ca) will decrease coverage by 0.02%.
The diff coverage is 21.42%.

❗ Current head f9b6003 differs from pull request most recent head 50d2bc2. Consider uploading reports for the commit 50d2bc2 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #294      +/-   ##
==========================================
- Coverage   48.02%   48.00%   -0.02%     
==========================================
  Files         279      279              
  Lines       33439    33451      +12     
==========================================
  Hits        16059    16059              
- Misses      15694    15703       +9     
- Partials     1686     1689       +3     
Impacted Files Coverage Δ
gateway/handler_ipns_record.go 0.00% <0.00%> (ø)
ipns/ipns.go 55.83% <23.07%> (-1.58%) ⬇️

... and 8 files with indirect coverage changes

@hacdias hacdias force-pushed the feat/reusable-ipns-validation branch 3 times, most recently from 2e10687 to 88ba464 Compare May 10, 2023 10:39
Copy link
Copy Markdown
Contributor

@Jorropo Jorropo left a comment

Choose a reason for hiding this comment

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

LGTM once thoses minors things are fixed.

@hacdias hacdias force-pushed the feat/reusable-ipns-validation branch from 0b41ecc to 50d2bc2 Compare May 10, 2023 11:17
@hacdias hacdias requested a review from Jorropo May 10, 2023 11:17
@hacdias hacdias enabled auto-merge (squash) May 10, 2023 11:21
@hacdias hacdias disabled auto-merge May 10, 2023 11:40
@hacdias hacdias merged commit 33e3f0c into main May 10, 2023
@hacdias hacdias deleted the feat/reusable-ipns-validation branch May 10, 2023 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants