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

Murisi/mock masp prover #2695

Merged
merged 5 commits into from
Feb 27, 2024
Merged

Murisi/mock masp prover #2695

merged 5 commits into from
Feb 27, 2024

Conversation

murisi
Copy link
Contributor

@murisi murisi commented Feb 22, 2024

Describe your changes

Replaced the test fixture saving system with a mock MASP prover and verifier system. More specifically:

  • Repurposed the mock transaction prover (used to generate arbitrary transactions fast) for the MASP client
    • This prover is identical to the real one except that it generates fake proofs in order to save time
  • Wrote a mock MASP verification context that is identical to the real one except that it assumes correct proofs
    • This is necessary because the real verification context would always reject transactions from the mock prover
  • When the testing flag is enabled, the mock prover and verifier are now activated instead of the real ones
  • Removed the test fixture directory and the variations of the test-integration command

The advantages/drawbacks of the above system are as follows:

  • Generation of new (never before saved) MASP transactions is much faster (a few seconds)
    • This means that it is much quicker to test changes to the MASP client or integration tests
  • Test fixtures no longer have to be saved in the repository
  • Less make targets are required for testing the MASP client
  • Drawback is that proofs are no longer verified in testing, hence proof verification must be tested elsewhere

Indicate on which release or other PRs this topic is based on

Namada 0.31.5
anoma/masp#72

Checklist before merging to draft

  • I have added a changelog
  • Git history is in acceptable state

@murisi murisi marked this pull request as ready for review February 22, 2024 13:01
@murisi murisi force-pushed the murisi/mock-masp-prover branch 2 times, most recently from 6253bb2 to 28fc780 Compare February 22, 2024 13:06
Copy link

codecov bot commented Feb 22, 2024

Codecov Report

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

Project coverage is 53.34%. Comparing base (c733be2) to head (93b92c7).
Report is 79 commits behind head on main.

Files Patch % Lines
crates/sdk/src/masp.rs 0.00% 159 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2695      +/-   ##
==========================================
- Coverage   53.38%   53.34%   -0.04%     
==========================================
  Files         302      302              
  Lines      103403   103469      +66     
==========================================
- Hits        55198    55196       -2     
- Misses      48205    48273      +68     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@murisi murisi mentioned this pull request Feb 22, 2024
2 tasks
tzemanovic added a commit that referenced this pull request Feb 26, 2024
* murisi/mock-masp-prover:
  masp: move the mock stuff under testing sub-mod
  Added changelog entry.
  Do not allow MASP_TEST_SEED to be used outside of testing.
  Implemented a MockSaplingVerificationContext and enable it for testing flag.
  Modified the transaction generator to use mock prover when in testing mode.
@tzemanovic tzemanovic merged commit 53affea into main Feb 27, 2024
14 of 17 checks passed
@tzemanovic tzemanovic deleted the murisi/mock-masp-prover branch February 27, 2024 21:40
brentstone added a commit that referenced this pull request Mar 4, 2024
* murisi/mock-masp-prover:
  masp: move the mock stuff under testing sub-mod
  Added changelog entry.
  Do not allow MASP_TEST_SEED to be used outside of testing.
  Implemented a MockSaplingVerificationContext and enable it for testing flag.
  Modified the transaction generator to use mock prover when in testing mode.
@brentstone brentstone mentioned this pull request Mar 4, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants