-
Notifications
You must be signed in to change notification settings - Fork 254
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
feat: verify block proofs on chain #6568
Conversation
31f6d51
to
bc98f18
Compare
724675f
to
8dca4d6
Compare
function extractAggregationObject(proof: Proof, numPublicInputs: number): Fr[] { | ||
const buffer = proof.buffer.subarray( | ||
Fr.SIZE_IN_BYTES * (numPublicInputs - AGGREGATION_OBJECT_LEN), | ||
Fr.SIZE_IN_BYTES * numPublicInputs, | ||
); | ||
return BufferReader.asReader(buffer).readArray(AGGREGATION_OBJECT_LEN, Fr); | ||
} |
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 is based on this snippet from barretenberg
aztec-packages/barretenberg/acir_tests/sol-test/src/index.js
Lines 175 to 183 in 432cec0
const [numPublicInputs, publicInputs] = readPublicInputs( | |
JSON.parse(proofAsFields.toString()) | |
); | |
const proofPath = getEnvVar("PROOF"); | |
const proof = readFileSync(proofPath); | |
// Cut the number of public inputs off of the proof string | |
const proofStr = `0x${proof.toString("hex").substring(64 * numPublicInputs)}`; |
The public inputs are extracted from the proof_as_fields object but then the rest of the proof is extracted by removing the equivalent bytes from the binary proof. Looks to me like the public inputs are already encoded as field elements in the binary proof so I can just read what I need from them.
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.
If this ends up changing a lot (which I suspect it will) we might want to disable the integration test since regenerating this fixture is pretty expensive.
df994f8
to
d78a23c
Compare
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.
Nice! So cool to see the pieces coming together!
@@ -43,7 +44,7 @@ contract Rollup is IRollup { | |||
uint256 public lastWarpedBlockTs; | |||
|
|||
constructor(IRegistry _registry, IAvailabilityOracle _availabilityOracle, IERC20 _gasToken) { | |||
VERIFIER = new MockVerifier(); | |||
verifier = new MockVerifier(); |
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.
Why break the SCREAMING_SNAKE tradition?
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.
😆 I thought I'd do this because it's not immutable
anymore.
import { type Anvil } from '@viem/anvil'; | ||
import { readFile } from 'fs/promises'; | ||
import { join } from 'path'; | ||
// @ts-expect-error solc-js doesn't publish its types https://github.com/ethereum/solc-js/issues/689 |
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.
Poor guy has even had a PR posted for over a year
let acvmTeardown: () => Promise<void>; | ||
let verifierContract: GetContractReturnType<any, typeof walletClient>; | ||
|
||
beforeAll(async () => { |
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.
Is it possible to reuse the e2e_prover test harness here?
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.
I wanted something a lighter than deploying the whole L2 for this test. I'm considering disabling it since keeping that fixture in sync with the rest of the system would be a pain.
yarn-project/bb-prover/src/bb/cli.ts
Outdated
) | ||
.requiredOption('-b, --bb-path <string>', 'The path to the BB binary', BB_BINARY_PATH) | ||
.requiredOption('-c, --circuit <string>', 'The name of a protocol circuit') | ||
.requiredOption('-cn --contract-name <string>', 'The name of the contract to generate', 'contract.sol') |
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.
Super nit: two character short flags are weird to me. Maybe just -n
for contract name?
22755c7
to
bea42bf
Compare
bea42bf
to
cf8bca3
Compare
This PR adds commands to generate the verification Solidity contract for the RootRollup so that block proofs can be validated onchain
Stacked on top of #6425