chore: rename key validation generators#20520
Conversation
|
OMG THE SIZE OF THE CHANGE JUST FROM RENAMING ONE CONCEPT HELP Edit: mostly Prover.tomls |
| echo_stderr "Hash preimage: $NOIR_HASH-$program_hash-$circuits_hash" | ||
| local hash=$(hash_str "$NOIR_HASH-$program_hash-$circuits_hash" $(cache_content_hash "^noir-projects/noir-protocol-circuits/bootstrap.sh")) |
There was a problem hiding this comment.
Claude says this:
This adds $circuits_hash (which is already computed on line 26 as a hash of all crate sources + Noir
hash + bootstrap.sh) to each circuit's cache key. Now any change to any .nr file in the crates will
bust the cache for all circuits, ensuring ABI-only changes like renames don't get stale cached
artifacts.
The tradeoff is that this is conservative — changing one circuit busts the cache for all circuits.
But it's correct and safe. A more targeted approach would hash per-circuit source files, but that's
more complex and nargo check already runs per-package anyway (so the overall build time is dominated
by compilation, not hashing).
✻ Churned for 5m 32s
❯ EXplain like I'm less smart
● The Noir build cache works like this:
- Before compiling a circuit, it computes a hash of the compiled bytecode (the math/constraints)
- If that hash matches something in the remote cache, it downloads the cached artifact instead of
recompiling
Your rename changed names (Generator → Separator) but not the math. The constraints are identical —
same fields, same types, same layout. So the bytecode hash is the same, the cache says "I already
have this!", and downloads the old artifact that still says KeyValidationRequestAndGenerator
everywhere in its type metadata.
The TypeScript codegen then reads those old artifacts and generates types/index.ts with the old
names, but your hand-edited client.ts expects the new names. Boom — type errors.
The fix adds the source file content hash to the cache key, so renaming things (which changes the
source but not the bytecode) forces a recompile.
There was a problem hiding this comment.
This means changing the comments/tests will also force a rebuild. Not sure if this is what we want? When I just renamed something I changed a comment (fix a typo, add/remove a space etc) in this bootstrap.sh file because it would also trigger a rebuild. Maybe we can add a comment in this file dedicating to this kind of scenario when a rebuild needs to be forced?
a893917 to
33a1113
Compare
| echo_stderr "Hash preimage: $NOIR_HASH-$program_hash-$circuits_hash" | ||
| local hash=$(hash_str "$NOIR_HASH-$program_hash-$circuits_hash" $(cache_content_hash "^noir-projects/noir-protocol-circuits/bootstrap.sh")) |
There was a problem hiding this comment.
This means changing the comments/tests will also force a rebuild. Not sure if this is what we want? When I just renamed something I changed a comment (fix a typo, add/remove a space etc) in this bootstrap.sh file because it would also trigger a rebuild. Maybe we can add a comment in this file dedicating to this kind of scenario when a rebuild needs to be forced?
f7e8d6c to
67bdccf
Compare
"Generator" isn't the correct term, really. The things are literally the domain separators for deriving public keys, which are also being adopted for deriving the different types of app-siloed secret keys. Let's see if claude did a good job
67bdccf to
fb0e6c4
Compare
"Generator" isn't the correct term, really. The things are literally the domain separators for deriving public keys, which are also being adopted for deriving the different types of app-siloed secret keys.
Let's see if claude did a good job