js-car icon indicating copy to clipboard operation
js-car copied to clipboard

Option to validate CID matches block

Open rvagg opened this issue 3 years ago • 4 comments

Of course this requires having the right hashers available to do the validation, which might be a problem.

Ref: https://github.com/ipld/js-car/pull/121#discussion_r1074000910

rvagg avatar Jan 27 '23 08:01 rvagg

Thinking about this more, what would be really nice is a wrapper, that could either live in js-multiformats or in its own package, that did this verification regardless of underlying source of blocks. Then we'd be forced to standardise on a basic blockstore interface.

You'd load up all your supported hashers into this wrapper, then plug in the backing blockstore, and then have all your blocks validated on the way out. You could even use it to come up with alternative strategies for various edge cases - like what to do if you don't have a hasher for a given CID, perhaps you don't error but shunt it onto an alternative call that collects them for sending a warning to the user.

I'm hesitant to try and shove this into js-car because of the hasher complexities, unless we can come up with a neat and somewhat generic solution we can reuse anywhere there's a source of blocks. The approach we've taken in Go is to always validate blocks but you can turn on a Trusted flag to skip validation. That all assumes you have the hashers available, but mostly we do and where they are missing folks go and add them to go-multihash so they become available. With js-multiformats (and js-block before it), we decided to avoid the kitchen-sink approach for JS, so we need to think of it differently, yet we still need standardisation for things like this.

/cc @alanshaw @hugomrdias

rvagg avatar Jan 30 '23 02:01 rvagg

I like it 😀, but it seems more like a plugin no? Car reader implements that interface and optionally runs plugins available while streaming blocks. Or are you thinking more wrapper takes implementations of interface and runs logic?

After reading this again looks more like the second. Either way basic blockstore interface is a win for everyone.

/Cc @achingbrain

hugomrdias avatar Jan 30 '23 07:01 hugomrdias

:shrug: we don't have consistent blockstore interfaces across the ipld libraries .. I prefer the wrapper style but it may not make sense for the various pieces we'd want to use it on

rvagg avatar Jan 30 '23 08:01 rvagg

I have wrote up proposal to introduce MultihashVerifier interface which could be used to verify hashes. I would suggest to make passing an instance in the reader mandatory and in cases where you want to opt out from verification you could pass trust based MultihashVerifier something like trust: (config) => MultihashVerifier.

That way it would use safe defaults as opposed to requiring extra steps to ensure safety.

Gozala avatar Apr 18 '23 04:04 Gozala