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

Decompression Prover input padding and error handling #187

Merged
merged 5 commits into from
Oct 16, 2024

Conversation

Tabaie
Copy link
Contributor

@Tabaie Tabaie commented Oct 15, 2024

During integration testing some proving requests were failing with an error claiming the blob checksum didn't match the one provided by the request.

This error was incorrectly reported because in fact the assign function in the prover was failing. However before checking for a returned error, the backend first compared the returned "snark hash" (which was empty) with the given, correct one.

The cause of the actual error was that the data fed into the prover was not the right length (128KB). The fix is for the prover to accept shorter blobs than the full size and pad them with zeros as needed.

@Tabaie Tabaie added bug Something isn't working Prover Tag to use for all work impacting the prover labels Oct 15, 2024
@Tabaie Tabaie requested a deployment to docker-build-and-e2e October 15, 2024 18:54 — with GitHub Actions Waiting
@codecov-commenter
Copy link

codecov-commenter commented Oct 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.36%. Comparing base (b385746) to head (76e9844).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #187   +/-   ##
=========================================
  Coverage     70.36%   70.36%           
  Complexity     1040     1040           
=========================================
  Files           282      282           
  Lines         11309    11309           
  Branches       1080     1028   -52     
=========================================
  Hits           7958     7958           
  Misses         2880     2880           
  Partials        471      471           
Flag Coverage Δ
hardhat 98.69% <ø> (ø)
kotlin 67.86% <ø> (ø)

@gusiri
Copy link
Contributor

gusiri commented Oct 15, 2024

I tested it again with this fix, and it works now. Thank you so much for the quick fix!

@gusiri
Copy link
Contributor

gusiri commented Oct 16, 2024

@AlexandreBelling Could you please review this?

@gusiri gusiri requested a deployment to docker-build-and-e2e October 16, 2024 12:36 — with GitHub Actions Waiting
@gusiri gusiri merged commit 48e1001 into main Oct 16, 2024
5 checks passed
@gusiri gusiri deleted the fix/blob-proof-error branch October 16, 2024 12:59
@gusiri gusiri requested a deployment to docker-build-and-e2e October 16, 2024 13:00 — with GitHub Actions Waiting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Prover Tag to use for all work impacting the prover
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants