fix!: replace dag walkers with generic CID extraction from blocks #447
Conversation
Replace the codec-specific `.dagWalkers` property with a generic dag walker internally that uses the `Block` interface from the `multicodecs` module. - Removes the `.dagWalkers` property from the Helia interface - Adds `getCodec` and `getHasher` to retrieve codecs and hashers by code - Adds `loadCodec` and `loadHasher` options to allow sync or async loading of extra codecs/hashes in addition to staticlly configured ones in the `codecs`/`hashers` keys BREAKING CHANGE: the `.dagWalkers` property has been removed
|
I think the pinning benchmarks should be re-run with this PR before merging. The This could potentially be more expensive than deserializing a block, collecting |
|
|
||
| await withBlock(cid, block) | ||
| const block = createUnsafe({ bytes, cid, codec }) |
There was a problem hiding this comment.
what's the implication of using createUnsafe ? I'm not sure I understand how this change affects things. And https://github.com/multiformats/js-multiformats/blob/5e2159a5126f15f2e16032b29a8bb5e31e619160/src/block.ts#L197 doesn't have much information
There was a problem hiding this comment.
We need a Block instance to call .links() on it.
createUnsafe doesn't verify that the block hash matches the block data, which is a (potentially) expensive operation, it just creates the block.
There was a problem hiding this comment.
For the sake of verified-fetch, where do we verify the block hash matches the data after the changes in this PR?
|
|
||
| await addChildren(subChild, name, level + 1, index + i, depth - 1, children, dag, codec, blocks) | ||
| links.push( | ||
| await createAndPutBlock(dagCbor.code, dagCbor.encode(child), blocks) |
There was a problem hiding this comment.
I know this is just a test fixture, but do we want to remove the ability to use any codec?
There was a problem hiding this comment.
The problem is creating a layered DAG depends on the codec - different codecs will implement this in different ways.
Previously we were only ever using one codec so it was a bit misleading.
achingbrain
force-pushed
the
fix/remove-dag-walkers
branch
from
9bb948d
to
c31f5e1
Compare
Co-authored-by: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com>
Replace the codec-specific
.dagWalkersproperty with a generic dag walker internally that uses theBlockinterface from themulticodecsmodule..dagWalkersproperty from the Helia interfacegetCodecandgetHasherto retrieve codecs and hashers by codeloadCodecandloadHasheroptions to allow sync or async loading of extra codecs/hashes in addition to staticlly configured ones in thecodecs/hasherskeysBREAKING CHANGE: the
.dagWalkersproperty has been removedChange checklist