fix: prevent duplicate trustless-gateway reqs #503
Conversation
|
note the previous failure in CI: https://github.com/ipfs/helia/actions/runs/8745897909/job/24001821684?pr=503#step:5:793 And it should be fixed by |
packages/block-brokers/src/trustless-gateway/trustless-gateway.ts
Outdated
Show resolved
Hide resolved
|
One thing I thought of after stepping away last night: we should probably use finally to remove entry from map |
| if (!res.ok) { | ||
| this.#errors++ | ||
| throw new Error(`unable to fetch raw block for CID ${cid} from gateway ${this.url}`) | ||
| let pendingResponse: Promise<Uint8Array> | undefined = this.#pendingResponses.get(gwUrl.toString()) |
There was a problem hiding this comment.
We should dedupe requests based on a canonical stringification of the multihash of the CID (base64 or something) rather than then requested URL, because you can request two CIDs with different versions/codecs but they'd resolve to the same block.
There was a problem hiding this comment.
I put a codepen together to try and play with how this is done, and had some helpful tips from Adin to get me going
https://codepen.io/SgtPooki/pen/XWQygKB?editors=1111
it would be interesting to see where this falls down.
Also, is it just me that thinks it would be useful to export a blessed "are these CIDs referencing the same bytes" toString() function from multiformats?
There was a problem hiding this comment.
Leaving this PR open to make sure you take a peek at this before I merge
| @@ -190,4 +190,33 @@ describe('trustless-gateway-block-broker', () => { | |||
|
|
|||
| await expect(sessionBlockstore?.retrieve?.(blocks[0].cid)).to.eventually.deep.equal(blocks[0].block) | |||
| }) | |||
|
|
|||
| it('does not trigger new network requests if the same cid request is in-flight', async function () { | |||
There was a problem hiding this comment.
this test was renamed in the last iteration from "[...] same block [...]" to "[...] same cid [...]" because we're not actually testing the uniqueBlock functionality here where we would prevent requests given separate CIDs if they're referencing the same bytes.
|
CI failed but not due to changes here AFAICT: https://github.com/ipfs/helia/actions/runs/8760997666/job/24046923998?pr=503#step:5:240 |
Title
fix: prevent duplicate trustless-gateway reqs
Description
Fixes ipfs/service-worker-gateway#104
This PR fixes issues brought up in service-worker-gateway where sub-resources end up causing multiple requests to a trustless gateway for the root CID.
Notes & open questions
I feel like the blockstore should handle this and ensure requests that depend on a root CID are deduped, but since trustless-gateways may be used outside of the context of blockstores, I believe we still need this improvement.
Change checklist