chore: turn constants codegen into a formal dependency#24958
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
|
||
| repo_root=$(git rev-parse --show-toplevel) | ||
| node "$repo_root/protocol/constants-codegen/src/cli.ts" --pil pil/vm2/constants_gen.pil \ | ||
| "$repo_root/protocol/constants-codegen/scripts/generate.sh" --pil pil/vm2/constants_gen.pil \ |
There was a problem hiding this comment.
in-repo consumers of the constants codegen use generate.sh to obtain their constants without having to know where they are sourced from
| pub global MAX_TX_BLOB_DATA_SIZE_IN_FIELDS: u32 = 1 // tx start marker | ||
| + 1 // tx hash | ||
| + 1 // transaction fee | ||
| + MAX_NOTE_HASHES_PER_TX | ||
| + MAX_NULLIFIERS_PER_TX | ||
| + MAX_L2_TO_L1_MSGS_PER_TX | ||
| + MAX_TOTAL_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX * 2 // (*2 for leaf slot and value) | ||
| + MAX_PRIVATE_LOGS_PER_TX * (PRIVATE_LOG_SIZE_IN_FIELDS + 1) // (+1 for log length) | ||
| + FLAT_PUBLIC_LOGS_PAYLOAD_LENGTH | ||
| + MAX_CONTRACT_CLASS_LOGS_PER_TX * (CONTRACT_CLASS_LOG_SIZE_IN_FIELDS + 1); // (+1 for contract address) |
There was a problem hiding this comment.
one of the key changes in this PR: by moving MAX_TX_BLOB_DATA_SIZE_IN_FIELDS here, we can dumb down the generator (it no longer needs to weave multiple inputs together) and have exactly one source of truth: constants.nr.
this also makes it easier to encapsulate knowledge on where contants.nr comes from, which is different for in-repo consumers and labs consumers, or anyone else.
the price to pay in return for this is some loss of locality since this calculation lives now in a global file. but if this is a problem, it can be solved in a number of different ways mucho more easily after the repos are split.
| set -euo pipefail | ||
|
|
||
| package_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) | ||
| types_dir="$package_dir/../../noir-projects/noir-protocol-circuits/crates/types/src" |
There was a problem hiding this comment.
embed-inputs.sh lets us encapsulate the source of constants.nr to in-repo constant consumers, properly pack it both for package users and portal users
| allowPositionals: false, | ||
| options: { | ||
| input: { type: 'string' }, | ||
| include: { type: 'string', multiple: true }, |
There was a problem hiding this comment.
we can simplify this out thanks to moving a single constant to constants.nr
|
A comment on the current design for embedding: I'm thinking of what labs and foundation would do after the split. Labs would just remove the call to "embed" and pin a version. But then, when the foundation submodules the repo, I guess they'd have to create a patch to revert both of those changes. I was wondering this alternative: if you call embed as a build step of constants-codegen. Then you can have all generate.sh calls use the internal .nr file. Also you can already remove lab's embed call. If things are set up this way, after the split then: labs only has to pin the version, foundation has to change the version to a portal link, everything else stays the same. I don't feel strongly, but wanted to mention it for consideration. |
Interesting, will explore |
mverzilli
added
the
ci-release-pr
AztecBot
removed
the
ci-release-pr
3 participants
Embeds constants.nr as default input to the constants-codegen package, and adds some bash conveniences to encapsulate references to it, to make it easier to flip the switch when the actual repo split is executed.
Closes F-822, F-823