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

Test with bundled prover instead of local prover. #1017

Merged
merged 3 commits into from
Nov 7, 2023

Conversation

nuttycom
Copy link
Contributor

No description provided.

@nuttycom nuttycom force-pushed the test_with_bundled_prover branch from 6a45221 to 3cdb005 Compare October 11, 2023 20:44
@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Files Coverage Δ
zcash_client_sqlite/src/wallet/sapling.rs 77.55% <ø> (ø)
zcash_extensions/src/transparent/demo.rs 74.46% <ø> (ø)

... and 2 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

str4d
str4d previously approved these changes Oct 31, 2023
Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 3cdb005

- name: Fetch path to Zcash parameters
working-directory: ./zcash_proofs
shell: bash
run: echo "ZCASH_PARAMS=$(cargo run --release --example get-params-path --features directories)" >> $GITHUB_ENV
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can delete the get-params-path example now; it was only added for use in our CI. (However, we should verify that zebrad's CI isn't also using it; I think they might have been at one stage.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git grep shows no usage of get-params-path in the zebrad repo.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, what they actually did is copied those files into their CI. They removed their copies in ZcashFoundation/zebra#7800.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So yes, we can just remove zcash_proofs/examples/ entirely (I don't think we need to offer Sprout parameter downloading examples either).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

- name: Fetch Zcash parameters
if: steps.cache-params.outputs.cache-hit != 'true'
working-directory: ./zcash_proofs
run: cargo run --release --example download-params --features download-params
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can delete the download-params example now; it was only added for use in our CI. (However, we should verify that zebrad's CI isn't also using it; I think they might have been at one stage.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git grep shows no usage of download-params in the zebrad repo.

…default.

`fetch_params.sh` is now deprecated and the bundled proving parameters
from `wagyu-zcash-parameters` are used everywhere, so the tests should
follow suit.

Fixes zcash#1016
Now that the tests use the bundled prover, we no longer need them to
download the zcash parameters files.
Since we now use the bundled prover everywhere, we no longer need
utilities to download Zcash parameters, which was the only purpose that
these examples were serving.
Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK c0a0ef1.

@nuttycom nuttycom merged commit c915c93 into zcash:main Nov 7, 2023
15 of 16 checks passed
@nuttycom nuttycom deleted the test_with_bundled_prover branch November 7, 2023 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

zcash_client_backend: Skip (most) trial decryption with the internal IVK
2 participants