blockstore: switch from Cid to Multihash
- Dominant language
- Go
- Stars
- 316
- Forks
- 163
- Avg merge
- 4d 12h
- Merged PRs (30d)
- 8
Description
We should change the Blockstore interface once it is updated to store blocks my Multihash and not the full Cid. One way to go is to remove the notation of Cids from a block, however the Cid is very useful in that context and doing so will likely break things. Instead I propose we change the types of two method (1) `AllKeysChan` will return multhashes instead of Cid, this is basically required because we no longer have the necessary information to reconstruct the Cid, (2) `DeleteBlock` will take a multihash, this is not necessary required but it is more honest, because if we delete a block from a Cid we are not just deleting that Cid, but all Cids with that multihash.
~~The proposed interface is thus:~~
**Note: See https://github.com/ipfs/boxo/issues/361 for the updated interface**
```go
type Blockstore interface {
DeleteBlock(mh.Multihash) error
Has(*cid.Cid) (bool, error)
Get(*cid.Cid) (blocks.Block, error)
// Put puts a given block to the underlying datastore
Put(blocks.Block) error
// PutMany puts a slice of blocks at the same time using batching
// capabilities of the underlying datastore whenever possible.
PutMany([]blocks.Block) error
// AllKeysChan returns a channel from which
// the CIDs in the Blockstore can be read. It should respect
// the given context, closing the channel if it becomes Done.
AllKeysChan(ctx context.Context) (<-chan mh.Multihash, error)
// HashOnRead specifies if every read block should be
// rehashed to make sure it matches its CID.
HashOnRead(enabled bool)
}
```
Note that since a Cid is part of a block the `Get` needs the full Cid to reconstruct the block. I have miked feeling about the `Has` method. I was thinking that a "smart" blockstore could use the extra information, but I can't think of a use-case (note that the `idstore` works at the multihash level so it doesn't need to full Cid). One argument is it should take the full Cid so it has the same parameter as the `Get` method.
One question: Why is `DeleteBlock` named that way? Is there a reason it is not just `Delete`? If this is purely for historical reasons, I propose we rename this to just `Delete` since we are changing the API anyway.
As it may sometime be useful to retrieve (or even add) a block at the multihash level, I also propose we introduce a new Codec to mean "unknown" to the Cid standard, for situations when all we have is the multihash but we need a full Cid. Trying to do anything with the data of a block with an "unknown" codec would result in an error. The "raw" codec could be used but it has a slightly different meaning, in particular it means that the data does not have any structure, rather than the structure being unknown. One use for this codec is for the output of `ipfs refs local`. We could just display the multihash, but that can easily be confused with CidV0. If instead we convert them to CidV1 with a "unknown" codec there will be no confusion and this will also allow us to display in different bases.
Thoughts?
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.