fix: ttl and caching for ipns urls (#34)

* fix: calculate ipns ttl correctly * fix: set the cache ttl as seconds consistently also add trace logging * chore: suffix argument with unit to avoid confusion * add tests to ensure the ttl is correctly computed * chore: disable failing test due to bug in js-ipns * Update packages/verified-fetch/test/utils/parse-url-string.spec.ts Co-authored-by: Russell Dempsey <[email protected]> --------- Co-authored-by: Daniel N <[email protected]> Co-authored-by: Russell Dempsey <[email protected]>
ipfs · Mar 28, 2024 · 44ac5a1 · 44ac5a1
1 parent 140bab3
commit 44ac5a1
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 7 deletions.
diff --git a/packages/verified-fetch/src/utils/parse-url-string.ts b/packages/verified-fetch/src/utils/parse-url-string.ts
@@ -85,15 +85,15 @@ function matchURLString (urlString: string): MatchUrlGroups {
  *
  * @see https://github.com/ipfs/js-ipns/blob/16e0e10682fa9a663e0bb493a44d3e99a5200944/src/index.ts#L200
  * @see https://github.com/ipfs/js-ipns/pull/308
+ * @returns the ttl in seconds
  */
 function calculateTtl (resolveResult?: IPNSResolveResult | DNSLinkResolveResult): number | undefined {
   if (resolveResult == null) {
     return undefined
   }
   const dnsLinkTtl = (resolveResult as DNSLinkResolveResult).answer?.TTL
   const ipnsTtlNs = (resolveResult as IPNSResolveResult).record?.ttl
-  // For some reason, ipns "nanoseconds" are 1e-8 of a second, instead of 1e-9.
-  const ipnsTtl = ipnsTtlNs != null ? Number(ipnsTtlNs / BigInt(1e8)) : undefined
+  const ipnsTtl = ipnsTtlNs != null ? Number(ipnsTtlNs / BigInt(1e9)) : undefined
   return dnsLinkTtl ?? ipnsTtl
 }
 
@@ -214,11 +214,14 @@ export async function parseUrlString ({ urlString, ipns, logger }: ParseUrlStrin
     throw new AggregateError(errors, `Invalid resource. Cannot determine CID from URL "${urlString}"`)
   }
 
-  const ttl = calculateTtl(resolveResult)
+  let ttl = calculateTtl(resolveResult)
 
   if (resolveResult != null) {
     // use the ttl for the resolved resouce for the cache, but fallback to 2 minutes if not available
-    ipnsCache.set(cidOrPeerIdOrDnsLink, resolveResult, ttl ?? 60 * 1000 * 2)
+    ttl = ttl ?? 60 * 2
+    log.trace('caching %s resolved to %s with TTL: %s', cidOrPeerIdOrDnsLink, cid, ttl)
+    // convert ttl from seconds to ms for the cache
+    ipnsCache.set(cidOrPeerIdOrDnsLink, resolveResult, ttl * 1000)
   }
 
   // parse query string

diff --git a/packages/verified-fetch/src/utils/tlru.ts b/packages/verified-fetch/src/utils/tlru.ts
@@ -28,8 +28,8 @@ export class TLRU<T> {
     return undefined
   }
 
-  set (key: string, value: T, ttl: number): void {
-    this.lru.set(key, { value, expire: Date.now() + ttl })
+  set (key: string, value: T, ttlMs: number): void {
+    this.lru.set(key, { value, expire: Date.now() + ttlMs })
   }
 
   has (key: string): boolean {

diff --git a/packages/verified-fetch/test/cache-control-header.spec.ts b/packages/verified-fetch/test/cache-control-header.spec.ts
@@ -71,7 +71,10 @@ describe('cache-control header', () => {
     expect(resp.headers.get('Cache-Control')).to.not.containIgnoreCase('immutable')
   })
 
-  it('should return the correct max-age in the cache-control header for an IPNS name', async () => {
+  // Skipping until https://github.com/ipfs/js-ipns/issues/310 is resolved
+  // Note that the source of the error is from the `name.publish` call rather than the max-age value
+  // in the cache control header.
+  it.skip('should return the correct max-age in the cache-control header for an IPNS name', async () => {
     const obj = {
       hello: 'world'
     }

diff --git a/packages/verified-fetch/test/utils/parse-url-string.spec.ts b/packages/verified-fetch/test/utils/parse-url-string.spec.ts
@@ -257,6 +257,40 @@ describe('parseUrlString', () => {
     })
   })
 
+  describe('TTL', () => {
+    const oneHourInSeconds = 3600
+    const oneHourInNanoseconds = BigInt(3600 * 1e9)
+
+    it('should return the correct TTL from the DNS Answer ', async () => {
+      ipns.resolveDNSLink.withArgs('newdomain.com').resolves({
+        cid: CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr'),
+        path: '',
+        answer: {
+          TTL: oneHourInSeconds,
+          type: 16,
+          name: 'n/a',
+          data: 'n/a'
+        }
+      })
+
+      const result = await parseUrlString({ urlString: 'ipns://newdomain.com/', ipns, logger })
+      expect(result.ttl).to.equal(oneHourInSeconds)
+    })
+
+    it('should return the correct TTL from the IPNS answer', async () => {
+      const testPeerId = await createEd25519PeerId()
+
+      ipns.resolve.withArgs(matchPeerId(testPeerId)).resolves({
+        cid: CID.parse('QmQJ8fxavY54CUsxMSx9aE9Rdcmvhx8awJK2jzJp4iAqCr'),
+        path: '',
+        record: ipnsRecordStub({ peerId: testPeerId, ttl: oneHourInNanoseconds })
+      })
+
+      const result = await parseUrlString({ urlString: `ipns://${testPeerId}`, ipns, logger })
+      expect(result.ttl).to.equal(oneHourInSeconds)
+    })
+  })
+
   describe('/ipfs/<CID> URLs', () => {
     it('should parse an IPFS Path with a CID only', async () => {
       await assertMatchUrl(