celestiaorg / rsmt2d

Go implementation of two dimensional Reed-Solomon merkle tree data availability scheme.
Apache License 2.0
162 stars 80 forks source link

fix: verify encoding of shares in case when axis is fully reconstructed from orthogonal #313

Closed walldiss closed 7 months ago

walldiss commented 7 months ago

If axis is fully recomputed from orthogonal axises, we need to verify correct encoding on top of verification of roots. Test coverage should happen by https://github.com/celestiaorg/rsmt2d/pull/312

codecov[bot] commented 7 months ago

Codecov Report

Attention: Patch coverage is 50.00000% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 81.15%. Comparing base (d6c118c) to head (ff65800). Report is 16 commits behind head on main.

Files Patch % Lines
extendeddatacrossword.go 50.00% 6 Missing and 3 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #313 +/- ## ========================================== + Coverage 80.89% 81.15% +0.25% ========================================== Files 8 7 -1 Lines 869 626 -243 ========================================== - Hits 703 508 -195 + Misses 119 72 -47 + Partials 47 46 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Wondertan commented 7 months ago

Do you know why codecov failed? 🤔

Wondertan commented 7 months ago

This seems like an important case we missed before and needs rsmt2d OGs to review it сс @adlerjohn and @musalbas

musalbas commented 7 months ago

I think this makes sense, nice catch.

@adlerjohn should also have a look.

rootulp commented 7 months ago

Do you know why codecov failed? 🤔

codecov/patch is failing because not enough lines are covered. IMO we can ignore that CI check if this fix is verified via the tests in https://github.com/celestiaorg/rsmt2d/pull/312