Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option to validate CID matches block #123

Open
rvagg opened this issue Jan 27, 2023 · 4 comments
Open

Option to validate CID matches block #123

rvagg opened this issue Jan 27, 2023 · 4 comments
Assignees

Comments

@rvagg
Copy link
Member

rvagg commented Jan 27, 2023

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

Ref: #121 (comment)

@rvagg
Copy link
Member Author

rvagg commented Jan 30, 2023

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

@hugomrdias
Copy link
Member

hugomrdias commented Jan 30, 2023

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

@rvagg
Copy link
Member Author

rvagg commented Jan 30, 2023

🤷 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

@Gozala
Copy link
Contributor

Gozala commented Apr 18, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🥞 Todo
Development

No branches or pull requests

3 participants