Skip to content
This repository was archived by the owner on Feb 12, 2024. It is now read-only.

feat: support identity hash in block.get + dag.get #3616

Merged
merged 8 commits into from
Apr 28, 2021
Merged

feat: support identity hash in block.get + dag.get #3616

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4705cb0
test(coreapi): block.get + dag.get for identity hash
hannahhoward Apr 8, 2021
e2da92a
fix(http-server): properly stream UInt8Array
hannahhoward Apr 8, 2021
db4d9d1
fix(deps): update ipfs-repo to fix types
hannahhoward Apr 8, 2021
1a96c39
fix(ipfs-core): fix repo gc typing
hannahhoward Apr 8, 2021
4376618
Update packages/ipfs-http-server/src/api/resources/block.js
hannahhoward Apr 8, 2021
94df79c
Merge remote-tracking branch 'origin/master' into feat/upgrade-ipfs-repo
achingbrain Apr 28, 2021
178318c
chore: update types after switch to queryKeys method
achingbrain Apr 28, 2021
7bdf168
Merge remote-tracking branch 'origin/master' into feat/upgrade-ipfs-repo
achingbrain Apr 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions packages/interface-ipfs-core/src/block/get.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
'use strict'

const uint8ArrayFromString = require('uint8arrays/from-string')
const multihash = require('multihashing-async').multihash
const multihashing = require('multihashing-async')
const CID = require('cids')
const { getDescribe, getIt, expect } = require('../utils/mocha')
const testTimeout = require('../utils/test-timeout')
Expand Down Expand Up @@ -43,7 +43,7 @@ module.exports = (common, options) => {
})

it('should get by CID in string', async () => {
const block = await ipfs.block.get(multihash.toB58String(hash))
const block = await ipfs.block.get(multihashing.multihash.toB58String(hash))

expect(block.data).to.eql(uint8ArrayFromString('blorb'))
expect(block.cid.multihash).to.eql(hash)
Expand Down Expand Up @@ -89,6 +89,14 @@ module.exports = (common, options) => {
expect(block.data).to.eql(input)
})

it('should get a block with an identity CID, without putting first', async () => {
const identityData = uint8ArrayFromString('A16461736466190144', 'base16upper')
const identityHash = await multihashing(identityData, 'identity')
const identityCID = new CID(1, 'dag-cbor', identityHash)
const block = await ipfs.block.get(identityCID)
expect(block.data).to.eql(identityData)
})

it('should return an error for an invalid CID', () => {
return expect(ipfs.block.get('Non-base58 character')).to.eventually.be.rejected
.and.be.an.instanceOf(Error)
Expand Down
9 changes: 9 additions & 0 deletions packages/interface-ipfs-core/src/dag/get.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const all = require('it-all')
const CID = require('cids')
const { getDescribe, getIt, expect } = require('../utils/mocha')
const testTimeout = require('../utils/test-timeout')
const multihashing = require('multihashing-async')

/** @typedef { import("ipfsd-ctl/src/factory") } Factory */
/**
Expand Down Expand Up @@ -240,6 +241,14 @@ module.exports = (common, options) => {
expect(result.value).to.deep.equal(buf)
})

it('should be able to get a dag-cbor node with the identity hash', async () => {
const identityData = uint8ArrayFromString('A16461736466190144', 'base16upper')
const identityHash = await multihashing(identityData, 'identity')
const identityCID = new CID(1, 'dag-cbor', identityHash)
const result = await ipfs.dag.get(identityCID)
expect(result.value).to.deep.equal({ asdf: 324 })
})

it('should throw error for invalid string CID input', () => {
return expect(ipfs.dag.get('INVALID CID'))
.to.eventually.be.rejected()
Expand Down
2 changes: 1 addition & 1 deletion packages/ipfs-cli/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
"ipfs-core-utils": "^0.7.2",
"ipfs-daemon": "^0.5.4",
"ipfs-http-client": "^49.0.4",
"ipfs-repo": "^9.0.0",
"ipfs-repo": "github:hannahhoward/js-ipfs-repo#dd812a6",
"ipfs-utils": "^6.0.4",
"ipld-dag-cbor": "^0.18.0",
"ipld-dag-pb": "^0.22.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/ipfs-core/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
"ipfs-block-service": "^0.19.0",
"ipfs-core-types": "^0.3.1",
"ipfs-core-utils": "^0.7.2",
"ipfs-repo": "^9.0.0",
"ipfs-repo": "github:hannahhoward/js-ipfs-repo#dd812a6",
"ipfs-unixfs": "^4.0.1",
"ipfs-unixfs-exporter": "^5.0.1",
"ipfs-unixfs-importer": "^7.0.1",
Expand Down
9 changes: 7 additions & 2 deletions packages/ipfs-core/src/components/repo/gc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

Should this not be import('ipld-block')?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to make that reduced type in the interface over in js-ipfs-repo to resolve the other type error with Bitswap

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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

Copy link
Member

@achingbrain achingbrain Apr 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

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.

*/

/**
Expand Down Expand Up @@ -101,7 +102,7 @@ async function createMarkedSet ({ pin, refs, repo }) {
* @param {object} arg
* @param {IPFSRepo} arg.repo
* @param {Set<string>} markedSet
* @param {AsyncIterable<CID>} blockKeys
* @param {AsyncIterable<CID|Block>} blockKeys
*/
async function * deleteUnmarkedBlocks ({ repo }, markedSet, blockKeys) {
// Iterate through all blocks and find those that are not in the marked set
Expand All @@ -110,12 +111,16 @@ async function * deleteUnmarkedBlocks ({ repo }, markedSet, blockKeys) {
let removedBlocksCount = 0

/**
* @param {CID} cid
* @param {Block|CID} cid
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is a TODO in the blockstore to seperate the keys only method, probably cause that's such an arbirtrarily wide return type

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh right function overloading in TS -- capturing the keysOnly properly may be difficult but yes that could work!

*/
const removeBlock = async (cid) => {
blocksCount++

try {
if (!CID.isCID(cid)) {
return null
}

const b32 = multibase.encode('base32', cid.multihash).toString()

if (markedSet.has(b32)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/ipfs-http-server/src/api/resources/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ exports.get = {
throw Boom.notFound('Block was unwanted before it could be remotely retrieved')
}

return h.response(block.data).header('X-Stream-Output', '1')
return h.response(Buffer.from(block.data.buffer)).header('X-Stream-Output', '1')
}
}
exports.put = {
Expand Down