celestiaorg / go-header

Go library with all the services needed to request, sync and store blockchain headers.
Apache License 2.0
17 stars 16 forks source link

feat: require non-zero headers for Verify #184

Closed cristaloleg closed 1 month ago

cristaloleg commented 1 month ago

Overview

This was shortly discussed with @renaynay. We agreed on documenting that zero trusted header is allowed but untrusted header cannot be zero. Theoretically we can enforce non-zero headers for both but this requires a bit wider discussion inside the team (which we can make in this PR).

Wondertan commented 1 month ago

I can't recall why we allow zero headers and I don't see a reason to allow this. I guess this is an oversight? I would prefer to disallow that rather than documenting its allowed.

cristaloleg commented 1 month ago

Yep, for me it also sounds like an oversight. During discussion with Rene was mentioned that it doesn't bite us (yet!) 'cause we do validation on a higher level.

However, this lib can and is used in the wild. Which may bring bugs to the users. Gonna add validation to the trusted header. Thanks.

cristaloleg commented 1 month ago

@renaynay I renamed the PR.