feat: support identity hash in block.get + dag.get #3616
Conversation
|
It looks like the type errors may be related to the changes to ipfs-repo since they mention BlockStores - could you please investigate? |
Add core interface tests for block.get and dag.get when fetching identity hashes
make sure that UInt8Arrays are converted to Buffer so Hapi properly sends them as binary
hannahhoward
force-pushed
the
feat/upgrade-ipfs-repo
branch
from
035a2c5
to
1a96c39
Compare
| @@ -17,6 +17,7 @@ const BLOCK_RM_CONCURRENCY = 256 | |||
| * @typedef {import('ipfs-core-types/src/refs').API} RefsAPI | |||
| * @typedef {import('ipfs-repo')} IPFSRepo | |||
| * @typedef {import('interface-datastore').Key} Key | |||
| * @typedef {import('ipfs-repo/src/types').Block} Block | |||
There was a problem hiding this comment.
Should this not be import('ipld-block')?
There was a problem hiding this comment.
I had to make that reduced type in the interface over in js-ipfs-repo to resolve the other type error with Bitswap
There was a problem hiding this comment.
@achingbrain I'm actually gonna need a bit of assistance from you to verify the proper solution here.
The reason this is no ipld-block is I pushed a new commit to my branch on js-ipfs-repo to narrow the Block used for the Blockstore interface - hannahhoward/js-ipfs-repo@dd812a6
The reason I did this is cause of a type error that was happening in ipfs-core/src/components/network.js --- and here the reason is that IPFSBitswap expects a Blockstore that is defined in ipfs-core-types, which in turn uses a much more limited block interface.
Here's where I get lost -- ipfs-core-types is part of this repo, but looks like it was massively rewritten just recently? However, ipfs-bitswap seems to just reference the last release, which has this limited Blockstore interface -- it appears to be gone from the source code that's in this repo.
I feel like I lack the context here to understand what the proper typing solution is. What I have compiles and typechecks, but I think perhaps getting to ipld-block properly is something that should be handled by someone who knows what's going on with ipfs-core-types
There was a problem hiding this comment.
It's a bit in flux at the moment. The initial ipfs-core-types was published in an earlier attempt at typing the API and contained a mishmash of shared interfaces, internal and external, as well as partial typings of other modules which should really live in those modules. ipfs-bitswap used types from this module which on balance was a mistake as it creates a dependency loop that is hard to break.
Since then the various low-level modules have had types added, ipfs-core-types has been rewritten to only contain the public API and all the partial typings have been removed, so internals like ipfs-bitswap should not depend on it, instead getting types directly from the modules they use, unless they consume the public API, which they shouldn't as we're back to dependency loops again.
It's possible that bitswap and the repo (which were typed early in this exercise) need a pass to remove their own partial typings of other modules and other extraneous cruft.
| @@ -110,12 +111,16 @@ async function * deleteUnmarkedBlocks ({ repo }, markedSet, blockKeys) { | |||
| let removedBlocksCount = 0 | |||
|
|
|||
| /** | |||
| * @param {CID} cid | |||
| * @param {Block|CID} cid | |||
There was a problem hiding this comment.
Why did this get added? Seems a bit weird to loosen the input type to accept a Block or a CID, then ignore anything that isn't a CID.
There was a problem hiding this comment.
that is the current return type to blocks.query -- which is accurate cause that's what you get if you don't specify keys-only
There was a problem hiding this comment.
there is a TODO in the blockstore to seperate the keys only method, probably cause that's such an arbirtrarily wide return type
There was a problem hiding this comment.
I see - I think we can do something clever with input arguments to disambiguate the return type - it-pushable takes this approach though changing return types always seems a bit wrong to me.
There was a problem hiding this comment.
oh right function overloading in TS -- capturing the keysOnly properly may be difficult but yes that could work!
Co-authored-by: Alex Potsides <alex@achingbrain.net>
achingbrain
changed the title
achingbrain
changed the title
Goals
Verifies ipfs/js-ipfs-repo#297 and closes #3289
Implementation
feat/idstorebranch referenced in above PR to get tests to pass.