-
Notifications
You must be signed in to change notification settings - Fork 295
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
chore: refactor Solidity Transcript and improve error handling in sol_honk flow #11158
Conversation
@@ -254,6 +254,18 @@ library Honk { | |||
G1Point lagrangeLast; | |||
} |
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 pattern of having to copy updates from Solidity to here is really not great (although same thing was done for Plonk). So we should come up with an alternative script
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.
LG - one comment that maybe doesn't apply anyway
y_0: uint256(bytes32(proof[0x1a0:0x1c0])), | ||
y_1: uint256(bytes32(proof[0x1c0:0x1e0])) | ||
}); | ||
p.w1 = bytesToG1ProofPoint(proof[0x60:0xe0]); |
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 seems like a nice improvement - a thought about how to make it even less error prone: A simple class that gets instantiated with a proof and has methods like read_G1, read_Fr etc. that automatically read the right amount of data and increment a cursor to prepare for the next read. Would avoid having to explicitly write out these ranges in hex which seems painful. Just something to consider
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.
Oh, maybe this logic has to match the solidity so you can't do things like what I suggested?
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.
yeah definitely can be on the list of TODOs
@@ -782,26 +735,26 @@ library RelationsLib { | |||
|
|||
function accumulatePermutationRelation( | |||
Fr[NUMBER_OF_ENTITIES] memory p, | |||
Transcript memory tp, | |||
Honk.RelationParameters memory rp, |
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 two letter name is better than the single letter above it but cant it at least be params or something haha
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.
will address in followup :)
* master: (329 commits) fix(avm): mac build (#11195) fix: docs rebuild patterns (#11191) chore: refactor Solidity Transcript and improve error handling in sol_honk flow (#11158) chore: move witness computation into class plus some other cleanup (#11140) fix: get_next_power_exponent off by 1 (#11169) chore(avm): vm2 followup cleanup (#11186) fix: underconstrained bug (#11174) refactor: VariableMerkleTree readability improvements (#11165) chore: removing noir bug workaround (#10535) chore(docs): Remove node pages (#11161) git subrepo push --branch=master noir-projects/aztec-nr git_subrepo.sh: Fix parent in .gitrepo file. [skip ci] chore: replace relative paths to noir-protocol-circuits git subrepo push --branch=master barretenberg feat(avm2): avm redesign init (#10906) feat: Sync from noir (#11138) feat: simulator split (#11144) chore: rpc server cleanup & misc fixes (#11145) git subrepo push --branch=master noir-projects/aztec-nr git_subrepo.sh: Fix parent in .gitrepo file. [skip ci] ...
A cleanup PR in preparation for the ZK contract
RelationsLib
don't need to take the Transcript as function argument ( this will be useful to reuse this library for both the zk and non-zk contract)loadProof
less manual and more robust by creating some utility functionssol_honk
flow actually displays contract compilation errors and clear errors (if possible) when deploying to aid debugging