-
Notifications
You must be signed in to change notification settings - Fork 315
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
Default cache paths to use FIL_PROOFS_CACHE_DIR if set. #1207
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears to perform what I would like it to
Just to clarify (and in preparation for change log) I believe this is how it works now: If The behaviour of the parameter cache is unchanged. Setting |
@travisperson I think this looks right except for one thing:
Did you mean setting |
Yes |
Approved, although no longer related to TMPDIR. Will that part cause confusion? |
@cryptonemo Fixed reference to TMPDIR. If you have bandwidth to propagate changes everywhere after #1211 merges (README, config sample, etc.) that would be helpful. If so, I consider this can merge and be incorporated in the next release when ready. (I will update the description to reflect this decision.) |
@cryptonemo I pushed one more commit documenting usage in a comment. Please review before propagating, since it clarifies a wart: specifically, the new env var cannot be set by config file — just env var. |
Closes #1206.
NOTE: Do not merge until a decision is made. This PR was a proposal for @lanzafame's approval, but merging it may cause as much or more trouble as the originally reported problem.UPDATE: We have settled on solution: separate actual cache usage from canonical parameter location.
In particular, we must ensure the location of the Groth parameter cache does not change for any user. (cc: @travisperson)
This is untested, but I wanted to push something quickly for @lanzafame to review, since this is high priority for him.
@lanzafame Please approve if this seems to satisfy your requirements. If so,
we'llwe might get this into the upcoming release after making sure it's solid.TODO: