Skip to content

Commit

Permalink
fix: code review
Browse files Browse the repository at this point in the history
  • Loading branch information
alexghr committed May 22, 2024
1 parent c4be958 commit cf8bca3
Show file tree
Hide file tree
Showing 14 changed files with 54 additions and 48 deletions.
17 changes: 10 additions & 7 deletions l1-contracts/src/core/Rollup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,9 @@ contract Rollup is IRollup {
revert Errors.Rollup__UnavailableTxs(header.contentCommitment.txsEffectsHash);
}

bytes32[] memory publicInputs = new bytes32[](
2 +
Constants.HEADER_LENGTH +
16
);
bytes32[] memory publicInputs =
new bytes32[](2 + Constants.HEADER_LENGTH + Constants.AGGREGATION_OBJECT_LENGTH);
// the archive tree root
publicInputs[0] = _archive;
// this is the _next_ available leaf in the archive tree
// normally this should be equal to the block number (since leaves are 0-indexed and blocks 1-indexed)
Expand All @@ -98,8 +96,13 @@ contract Rollup is IRollup {
// the block proof is recursive, which means it comes with an aggregation object
// this snippet copies it into the public inputs needed for verification
// it also guards against empty _aggregationObject used with mocked proofs
for (uint256 i = 0; i < 16 && i * 32 < _aggregationObject.length; i++) {
publicInputs[i + 2 + headerFields.length] = bytes32(_aggregationObject[i * 32:(i + 1) * 32]);
uint256 aggregationLength = _aggregationObject.length / 32;
for (uint256 i = 0; i < Constants.AGGREGATION_OBJECT_LENGTH && i < aggregationLength; i++) {
bytes32 part;
assembly {
part := calldataload(add(_aggregationObject.offset, mul(i, 32)))
}
publicInputs[i + 2 + Constants.HEADER_LENGTH] = part;
}

if (!verifier.verify(_proof, publicInputs)) {
Expand Down
1 change: 1 addition & 0 deletions l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ library Constants {
AZTEC_ADDRESS_LENGTH + FUNCTION_DATA_LENGTH + PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH;
uint256 internal constant PUBLIC_CONTEXT_INPUTS_LENGTH =
CALL_CONTEXT_LENGTH + HEADER_LENGTH + GLOBAL_VARIABLES_LENGTH + GAS_LENGTH + 2;
uint256 internal constant AGGREGATION_OBJECT_LENGTH = 16;
uint256 internal constant SCOPED_READ_REQUEST_LEN = READ_REQUEST_LENGTH + 1;
uint256 internal constant PUBLIC_DATA_READ_LENGTH = 2;
uint256 internal constant VALIDATION_REQUESTS_LENGTH = ROLLUP_VALIDATION_REQUESTS_LENGTH
Expand Down
12 changes: 9 additions & 3 deletions l1-contracts/src/core/libraries/HeaderLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,17 @@ library HeaderLib {
fields[6] = _header.stateReference.l1ToL2MessageTree.root;
fields[7] = bytes32(uint256(_header.stateReference.l1ToL2MessageTree.nextAvailableLeafIndex));
fields[8] = _header.stateReference.partialStateReference.noteHashTree.root;
fields[9] = bytes32(uint256(_header.stateReference.partialStateReference.noteHashTree.nextAvailableLeafIndex));
fields[9] = bytes32(
uint256(_header.stateReference.partialStateReference.noteHashTree.nextAvailableLeafIndex)
);
fields[10] = _header.stateReference.partialStateReference.nullifierTree.root;
fields[11] = bytes32(uint256(_header.stateReference.partialStateReference.nullifierTree.nextAvailableLeafIndex));
fields[11] = bytes32(
uint256(_header.stateReference.partialStateReference.nullifierTree.nextAvailableLeafIndex)
);
fields[12] = _header.stateReference.partialStateReference.publicDataTree.root;
fields[13] = bytes32(uint256(_header.stateReference.partialStateReference.publicDataTree.nextAvailableLeafIndex));
fields[13] = bytes32(
uint256(_header.stateReference.partialStateReference.publicDataTree.nextAvailableLeafIndex)
);
fields[14] = bytes32(_header.globalVariables.chainId);
fields[15] = bytes32(_header.globalVariables.version);
fields[16] = bytes32(_header.globalVariables.blockNumber);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ global PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH: u64 = CALL_CONTEXT_LENGTH + 2 + (REA
global PRIVATE_CALL_STACK_ITEM_LENGTH: u64 = AZTEC_ADDRESS_LENGTH + FUNCTION_DATA_LENGTH + PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH;
global PUBLIC_CONTEXT_INPUTS_LENGTH: u64 = CALL_CONTEXT_LENGTH + HEADER_LENGTH + GLOBAL_VARIABLES_LENGTH + GAS_LENGTH + 2;

global AGGREGATION_OBJECT_LENGTH: u64 = 16;

global SCOPED_READ_REQUEST_LEN = READ_REQUEST_LENGTH + 1;
global PUBLIC_DATA_READ_LENGTH = 2;
global VALIDATION_REQUESTS_LENGTH = ROLLUP_VALIDATION_REQUESTS_LENGTH + (SCOPED_READ_REQUEST_LEN * MAX_NOTE_HASH_READ_REQUESTS_PER_TX) + (SCOPED_READ_REQUEST_LEN * MAX_NULLIFIER_READ_REQUESTS_PER_TX) + (SCOPED_READ_REQUEST_LEN * MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_TX) + (SCOPED_KEY_VALIDATION_REQUEST_LENGTH * MAX_KEY_VALIDATION_REQUESTS_PER_TX) + (PUBLIC_DATA_READ_LENGTH * MAX_PUBLIC_DATA_READS_PER_TX);
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/bb-prover/src/bb/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export function getProgram(log: LogFn): Command {
)
.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')
.requiredOption('-n --contract-name <string>', 'The name of the contract to generate', 'contract.sol')
.action(async options => {
const compiledCircuit = ProtocolCircuitArtifacts[options.circuit as ProtocolArtifact];
if (!compiledCircuit) {
Expand Down
11 changes: 8 additions & 3 deletions yarn-project/bb-prover/src/prover/bb_native_proof_creator.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { type AppCircuitProofOutput, type KernelProofOutput, type ProofCreator } from '@aztec/circuit-types';
import { type CircuitProvingStats, type CircuitWitnessGenerationStats } from '@aztec/circuit-types/stats';
import {
AGGREGATION_OBJECT_LENGTH,
Fr,
NESTED_RECURSIVE_PROOF_LENGTH,
type PrivateCircuitPublicInputs,
Expand Down Expand Up @@ -51,7 +52,7 @@ import {
verifyProof,
} from '../bb/execute.js';
import { mapProtocolArtifactNameToCircuitName } from '../stats.js';
import { AGGREGATION_OBJECT_SIZE, extractVkData } from '../verification_key/verification_key_data.js';
import { extractVkData } from '../verification_key/verification_key_data.js';

/**
* This proof creator implementation uses the native bb binary.
Expand Down Expand Up @@ -394,12 +395,16 @@ export class BBNativeProofCreator implements ProofCreator {
const json = JSON.parse(proofString);
const fields = json.map(Fr.fromString);
const numPublicInputs =
circuitType === 'App' ? vkData.numPublicInputs : vkData.numPublicInputs - AGGREGATION_OBJECT_SIZE;
circuitType === 'App' ? vkData.numPublicInputs : vkData.numPublicInputs - AGGREGATION_OBJECT_LENGTH;
const fieldsWithoutPublicInputs = fields.slice(numPublicInputs);
this.log.debug(
`Circuit type: ${circuitType}, complete proof length: ${fields.length}, without public inputs: ${fieldsWithoutPublicInputs.length}, num public inputs: ${numPublicInputs}, circuit size: ${vkData.circuitSize}, is recursive: ${vkData.isRecursive}, raw length: ${binaryProof.length}`,
);
const proof = new RecursiveProof<PROOF_LENGTH>(fieldsWithoutPublicInputs, new Proof(binaryProof), true);
const proof = new RecursiveProof<PROOF_LENGTH>(
fieldsWithoutPublicInputs,
new Proof(binaryProof, vkData.numPublicInputs),
true,
);
return proof;
}
}
15 changes: 10 additions & 5 deletions yarn-project/bb-prover/src/prover/bb_prover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
} from '@aztec/circuit-types';
import { type CircuitProvingStats, type CircuitWitnessGenerationStats } from '@aztec/circuit-types/stats';
import {
AGGREGATION_OBJECT_LENGTH,
type BaseOrMergeRollupPublicInputs,
type BaseParityInputs,
type BaseRollupInputs,
Expand Down Expand Up @@ -64,7 +65,7 @@ import {
} from '../bb/execute.js';
import { PublicKernelArtifactMapping } from '../mappings/mappings.js';
import { mapProtocolArtifactNameToCircuitName } from '../stats.js';
import { AGGREGATION_OBJECT_SIZE, extractVkData } from '../verification_key/verification_key_data.js';
import { extractVkData } from '../verification_key/verification_key_data.js';

const logger = createDebugLogger('aztec:bb-prover');

Expand Down Expand Up @@ -510,7 +511,7 @@ export class BBNativeRollupProver implements ServerCircuitProver {
}

const operation = async (bbWorkingDirectory: string) => {
const numPublicInputs = vk.numPublicInputs - AGGREGATION_OBJECT_SIZE;
const numPublicInputs = vk.numPublicInputs - AGGREGATION_OBJECT_LENGTH;
const proofFullFilename = `${bbWorkingDirectory}/${PROOF_FILENAME}`;
const vkFullFilename = `${bbWorkingDirectory}/vk`;

Expand Down Expand Up @@ -543,7 +544,7 @@ export class BBNativeRollupProver implements ServerCircuitProver {
const fields = json.slice(numPublicInputs).map(Fr.fromString);
return new RecursiveProof<typeof NESTED_RECURSIVE_PROOF_LENGTH>(
fields,
new Proof(proof.binaryProof.buffer),
new Proof(proof.binaryProof.buffer, vk.numPublicInputs),
true,
);
};
Expand Down Expand Up @@ -616,12 +617,16 @@ export class BBNativeRollupProver implements ServerCircuitProver {
}
const numPublicInputs = CIRCUITS_WITHOUT_AGGREGATION.has(circuitType)
? vkData.numPublicInputs
: vkData.numPublicInputs - AGGREGATION_OBJECT_SIZE;
: vkData.numPublicInputs - AGGREGATION_OBJECT_LENGTH;
const fieldsWithoutPublicInputs = json.slice(numPublicInputs).map(Fr.fromString);
logger.debug(
`Circuit type: ${circuitType}, complete proof length: ${json.length}, without public inputs: ${fieldsWithoutPublicInputs.length}, num public inputs: ${numPublicInputs}, circuit size: ${vkData.circuitSize}, is recursive: ${vkData.isRecursive}, raw length: ${binaryProof.length}`,
);
const proof = new RecursiveProof<PROOF_LENGTH>(fieldsWithoutPublicInputs, new Proof(binaryProof), true);
const proof = new RecursiveProof<PROOF_LENGTH>(
fieldsWithoutPublicInputs,
new Proof(binaryProof, numPublicInputs),
true,
);
if (proof.proof.length !== proofLength) {
throw new Error("Proof length doesn't match expected length");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import * as fs from 'fs/promises';

import { VK_FIELDS_FILENAME, VK_FILENAME } from '../bb/execute.js';

export const AGGREGATION_OBJECT_SIZE = 16;

/**
* Reads the verification key data stored at the specified location and parses into a VerificationKeyData
* @param filePath - The directory containing the verification key data files
Expand Down
1 change: 1 addition & 0 deletions yarn-project/circuits.js/src/constants.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ export const PRIVATE_CALL_STACK_ITEM_LENGTH =
AZTEC_ADDRESS_LENGTH + FUNCTION_DATA_LENGTH + PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH;
export const PUBLIC_CONTEXT_INPUTS_LENGTH =
CALL_CONTEXT_LENGTH + HEADER_LENGTH + GLOBAL_VARIABLES_LENGTH + GAS_LENGTH + 2;
export const AGGREGATION_OBJECT_LENGTH = 16;
export const SCOPED_READ_REQUEST_LEN = READ_REQUEST_LENGTH + 1;
export const PUBLIC_DATA_READ_LENGTH = 2;
export const VALIDATION_REQUESTS_LENGTH =
Expand Down
6 changes: 3 additions & 3 deletions yarn-project/circuits.js/src/structs/proof.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export class Proof {
*/
public buffer: Buffer,

public numPublicInputs = 0,
public numPublicInputs: number,
) {}

/**
Expand All @@ -30,9 +30,9 @@ export class Proof {
*/
static fromBuffer(buffer: Buffer | BufferReader): Proof {
const reader = BufferReader.asReader(buffer);
const numPublicInputs = reader.readNumber();
const size = reader.readNumber();
const buf = reader.readBytes(size);
const numPublicInputs = reader.readNumber();
return new Proof(buf, numPublicInputs);
}

Expand All @@ -43,7 +43,7 @@ export class Proof {
* @returns A Buffer containing the serialized proof data in custom format.
*/
public toBuffer() {
return serializeToBuffer(this.numPublicInputs, this.buffer.length, this.buffer);
return serializeToBuffer(this.buffer.length, this.buffer, this.numPublicInputs);
}

/**
Expand Down
12 changes: 1 addition & 11 deletions yarn-project/circuits.js/src/tests/factories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -496,16 +496,6 @@ export function makePublicCallRequest(seed = 1): PublicCallRequest {
);
}

/**
* Creates a uint8 vector of a given size filled with a given value.
* @param size - The size of the vector.
* @param fill - The value to fill the vector with.
* @returns A uint8 vector.
*/
export function makeDynamicSizeBuffer(size: number, fill: number) {
return new Proof(Buffer.alloc(size, fill));
}

/**
* Creates arbitrary/mocked membership witness where the sibling paths is an array of fields in an ascending order starting from `start`.
* @param size - The size of the membership witness.
Expand Down Expand Up @@ -588,7 +578,7 @@ export function makeRollupKernelData(seed = 1, kernelPublicInputs?: KernelCircui
* @returns A proof.
*/
export function makeProof(seed = 1) {
return makeDynamicSizeBuffer(16, seed);
return new Proof(Buffer.alloc(16, seed), 0);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { L2Block, deployL1Contract, fileURLToPath } from '@aztec/aztec.js';
import { BBCircuitVerifier } from '@aztec/bb-prover';
import { Fr, HEADER_LENGTH, Proof } from '@aztec/circuits.js';
import { AGGREGATION_OBJECT_LENGTH, Fr, HEADER_LENGTH, Proof } from '@aztec/circuits.js';
import { type L1ContractAddresses } from '@aztec/ethereum';
import { type Logger } from '@aztec/foundation/log';
import { BufferReader, serializeToBuffer } from '@aztec/foundation/serialize';
Expand All @@ -22,19 +22,15 @@ import {
} from 'viem';
import { mnemonicToAccount } from 'viem/accounts';

import { AGGREGATION_OBJECT_SIZE } from '../../../bb-prover/src/verification_key/verification_key_data.js';
import { MNEMONIC } from '../fixtures/fixtures.js';
import { getACVMConfig } from '../fixtures/get_acvm_config.js';
import { getBBConfig } from '../fixtures/get_bb_config.js';
import { getLogger, setupL1Contracts, startAnvil } from '../fixtures/utils.js';

/**
* README
*
* If this test starts failing, regenerate its fixture with
* Regenerate this test's fixture with
* AZTEC_GENERATE_TEST_DATA=1 yarn workspace @aztec/end-to-end test e2e_prover
*/

describe('proof_verification', () => {
let proof: Proof;
let block: L2Block;
Expand Down Expand Up @@ -141,12 +137,12 @@ describe('proof_verification', () => {
// +2 fields for archive
const archive = reader.readArray(2, Fr);
const header = reader.readArray(HEADER_LENGTH, Fr);
const aggObject = reader.readArray(AGGREGATION_OBJECT_SIZE, Fr);
const aggObject = reader.readArray(AGGREGATION_OBJECT_LENGTH, Fr);

const publicInputs = [...archive, ...header, ...aggObject].map(x => x.toString());

const proofStr = `0x${proof.buffer
.subarray((HEADER_LENGTH + 2 + AGGREGATION_OBJECT_SIZE) * Fr.SIZE_IN_BYTES)
.subarray((HEADER_LENGTH + 2 + AGGREGATION_OBJECT_LENGTH) * Fr.SIZE_IN_BYTES)
.toString('hex')}` as const;

await expect(verifierContract.read.verify([proofStr, publicInputs])).resolves.toBeTruthy();
Expand Down
Loading

0 comments on commit cf8bca3

Please sign in to comment.