service/block: add basic encoding validation in BlockService

[renaynay]

This PR relies on #87 being merged first.

Resolves #92.

[liamsi]

Looks good! 👏🏼

[Wondertan]

It is not absolutely necessary to do this validation though, as we trust the Core. For the future case with p2p Fetcher, the protocol doing the fetch would do this check on the lower level as well. Also, if we do this check why don't we check other fields in the block validation process? It just does not make sense to me not to trust the Core only for DataHash, but trust with other things.

cc @liamsi

[liamsi]

It just does not make sense to me not to trust the Core only for DataHash, but trust with other things.

Yeah, I think we should ideally do basic validation on the block. But as we get header and block separentley, we should make sure that they match up (each field).

as we trust the Core.

Yeah, you are absolutely right about trusting core. I think we should still make sure that the stuff we get from core actually checks out. The check here is not wrong though. I think we should open an issue for the checks but most likely we can just use whatever core does as a library with the difference that we download block and header separately here (but maybe there are functions we can still use in that case).

[renaynay]

think we should open an issue for the checks but most likely we can just use whatever core does as a library with the difference that we download block and header separately here (but maybe there are functions we can still use in that case).

@liamsi can you pls open this issue on our repo? 🙏🏻

[Wondertan]

Codewise looks good. Conceptually, my concern is still the same, we should either have validation for everything or for nothing, but we can see this as a first step towards validation for everything