-
Notifications
You must be signed in to change notification settings - Fork 118
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
Cache prover keys #1187
Cache prover keys #1187
Conversation
EDIT: ready now |
let privilegedKey = PrivateKey.fromBase58( | ||
'EKEeoESE2A41YQnSht9f7mjiKpJSeZ4jnfHXYatYi8xJdYSxWBep' | ||
); |
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.
there's no caching if the contract depends on random constants 😬
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.
Huh, why is that? 🤔
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.
Constants that are used in constraints are baked into the circuit digest as well as prover and verifier keys. This example was using PrivateKey.random()
so the circuit was different every time. Since we compare the circuit digest to decide whether or not we can use a cached key, the answer was "no" every time.
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.
Ah, yes, pretty straightforward. That makes sense, thank you!
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.
some comments to understand what is happening here
/** | ||
* Interface for storing and retrieving values, for caching. | ||
* `read()` and `write()` can just throw errors on failure. | ||
*/ | ||
type Cache = { |
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.
this file contains the main new interface -- Cache
! plus a default implementation of Cache
which works in Node.js and uses a common cache directory
src/lib/proof-system/prover-keys.ts
Outdated
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.
this file provides helpers to
- encode and decode all 4 kinds of snark keys to/from bytes
- create a header which is passed to the
Cache
so that it can figure out where and if to read from cache
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.
This file is pretty verbose and could be hard to parse for other readers/maintainers in the future. Mind throwing up a top-level comment that is basically your comment here? It'll be much easier to read once the intent is given in particular :D
let picklesCache: Pickles.Cache = [ | ||
0, | ||
function read_(mlHeader) { | ||
let header = parseHeader(proofSystemTag.name, methodIntfs, mlHeader); | ||
try { | ||
let bytes = cache.read(header); | ||
if (bytes === undefined) return MlResult.unitError(); | ||
return MlResult.ok(decodeProverKey(mlHeader, bytes)); | ||
} catch (e: any) { | ||
return MlResult.unitError(); | ||
} | ||
}, |
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.
this code connects all the dots:
- our helpers to create cache headers and decode snark keys from bytes
- the user-defined
cache
where we canread
andwrite
from - the
Pickles.Cache
which Pickles expects
src/lib/util/fs.ts
Outdated
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.
simple wrapper around a few node-only functions, so that we can provide alternative (dummy) implementations on the web
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.
Great design choice! 👍
@@ -661,7 +662,7 @@ class SmartContract { | |||
* it so that proofs end up in the original finite field). These are fairly expensive operations, so **expect compiling to take at least 20 seconds**, | |||
* up to several minutes if your circuit is large or your hardware is not optimal for these operations. | |||
*/ | |||
static async compile() { | |||
static async compile({ cache = Cache.FileSystemDefault } = {}) { |
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.
SmartContract.compile()
now expects an optional Cache
. same for ZkProgram.compile()
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.
Great work! What are your thoughts on documenting the caching strategy we use? Should we have something on our docs website or/and in-depth doc comments?
I think this information is particularly valuable, and it should probably be easily accessible outside of a PR description:
a step prover key (step-pk) and verification key (step-vk) for every method.
a wrap prover key (wrap-pk) and verification key (wrap-vk) for the entire contract
a .header file for each cache item. this just contains a unique string which is used to determine whether we can use the cached data
src/lib/util/fs.ts
Outdated
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.
Great design choice! 👍
src/lib/proof-system/prover-keys.ts
Outdated
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.
This file is pretty verbose and could be hard to parse for other readers/maintainers in the future. Mind throwing up a top-level comment that is basically your comment here? It'll be much easier to read once the intent is given in particular :D
Addressed in the SRS caching PR! |
closes #87
mina: MinaProtocol/mina#14396 and MinaProtocol/mina#14412
bindings: o1-labs/o1js-bindings#187
This PR enables caching of prover and verifier keys in Node.js. The caching interface is kept flexible and will allow a variety of other caching strategies -- for example, bundling snark keys with your web app, or downloading them from some CDN, etc.
Results for calling
compile()
on thesimple_zkapp.ts
example (my machine)The majority of the remaining 13s is spent in recreating SRS and Lagrange bases (= long lists of elliptic curve points which are the same every time). Caching of those is added in a follow-up PR.
After caching a bunch of
SmartContract
s andZkProgram
s, your cache directory will look like this:Note that per contract, several different keys are cached:
step-pk
) and verification key (step-vk
) for every method.wrap-pk
) and verification key (wrap-vk
) for the entire contract.header
file for each cache item. this just contains a unique string which is used to determine whether we can use the cached dataIf only one of the methods changes, then only the wrap keys and one of the step keys will be regenerated.