Skip to content

fix: skip expired ipns records in GetIPNS#154

Open
lidel wants to merge 2 commits into
mainfrom
fix/ipns-record-selection
Open

fix: skip expired ipns records in GetIPNS#154
lidel wants to merge 2 commits into
mainfrom
fix/ipns-record-selection

Conversation

@lidel
Copy link
Copy Markdown
Member

@lidel lidel commented Jun 3, 2026

Note

This is belt-and-suspenders, optional fix in addition to the upstream:

Problem

parallelRouter.GetIPNS races the DHT and the delegated HTTP routers and returned the first record any router produced, without checking its validity. An IPNS record whose EOL has already passed could therefore be returned even when a valid record was available from another router. The record is cryptographically invalid, so a validating consumer rejects it, which surfaced as sporadic failures downstream (e.g. service-worker gateway 500s).

Fix

  • Add isExpiredIPNSRecord, which reports whether an EOL-type record has passed its validity.
  • Treat an expired record as routing.ErrNotFound in both the single-router path and the multi-router race.
  • In the race, keep waiting for another router to return a non-expired record instead of returning the expired one.

GetIPNS never returns a record past its EOL; first-valid-record-wins behavior and latency are otherwise unchanged.

parallelRouter.GetIPNS returned the first record a router produced
without checking its validity, so an expired record could be
served. Treat an EOL-passed record as not found and keep waiting
for a valid one from another router.
@lidel lidel requested review from a team and achingbrain June 3, 2026 15:39
every backend already validates the record it returns, but EOL is
time-varying while signatures are not: a record can expire between a
backend's check and the moment the aggregator returns it. parallelRouter
is first-result-wins with no revalidation, so document why the re-check
guards against expired records winning the race.
Copy link
Copy Markdown
Contributor

@gammazero gammazero left a comment

Choose a reason for hiding this comment

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

LGTM

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.

2 participants