Finschia / ostracon

Ostracon, a consensus algorithm, is forked from Tendermint Core. We have added VRF to Tendermint BFT. It adds randomness to PoS Validator elections and improves security.
Apache License 2.0
70 stars 28 forks source link

fix: add proof size check when the VRF proof is empty #765

Closed 170210 closed 3 months ago

170210 commented 3 months ago

Description

This PR added an error handle in proof size check when the VRF proof is empty.

Previously, we allowed the proof hash to be 0 in Validateproof because there was no proof required when verifying Genesis block's information. However, since there is no Genesis block, the proof hash should not be zero in general verification.

I tested it with finschia-sdk, and there is no problem when this handling was added.

related #510

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 66.53%. Comparing base (fc54d22) to head (4a2f67b). Report is 3 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #765 +/- ## ========================================== - Coverage 66.58% 66.53% -0.05% ========================================== Files 285 285 Lines 37919 37917 -2 ========================================== - Hits 25248 25228 -20 - Misses 10871 10882 +11 - Partials 1800 1807 +7 ``` | [Files](https://app.codecov.io/gh/Finschia/ostracon/pull/765?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia) | Coverage Δ | | |---|---|---| | [crypto/ed25519/migration.go](https://app.codecov.io/gh/Finschia/ostracon/pull/765?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia#diff-Y3J5cHRvL2VkMjU1MTkvbWlncmF0aW9uLmdv) | `100.00% <100.00%> (+16.66%)` | :arrow_up: | ... and [17 files with indirect coverage changes](https://app.codecov.io/gh/Finschia/ostracon/pull/765/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia)
ulbqb commented 3 months ago

https://github.com/Finschia/ostracon/blob/898b48c4c66a9e9a7e148f1b0e61669f3ba197f7/crypto/ed25519/migration.go#L41-L43

It looks like empty proofs are admitted intentionally. Would you research this?

ulbqb commented 3 months ago

Please add following contents to description:

ulbqb commented 3 months ago

https://github.com/Finschia/ostracon/blob/ae67c39c8a79827fc0aeeddf86f64bdbf193cbf4/spec/core/data_structures.md?plain=1#L56 And update spec please.