Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@Maddiaa0
Maddiaa0 committed Aug 1, 2023
1 parent d06a919 commit dc20631
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 75 deletions.
6 changes: 0 additions & 6 deletions circuits/cpp/src/aztec3/circuits/abis/rollup/root/root_rollup_public_inputs.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ template <typename NCT> struct RootRollupPublicInputs {

GlobalVariables<NCT> globalVariables;

fr block_hash;

AppendOnlyTreeSnapshot<NCT> start_private_data_tree_snapshot;
AppendOnlyTreeSnapshot<NCT> end_private_data_tree_snapshot;

Expand Down Expand Up @@ -61,7 +59,6 @@ template <typename NCT> struct RootRollupPublicInputs {
std::vector<uint8_t> buf;

write(&buf, globalVariables);
write(&buf, block_hash);
write(buf, start_private_data_tree_snapshot);
write(buf, start_nullifier_tree_snapshot);
write(buf, start_contract_tree_snapshot);
Expand Down Expand Up @@ -113,7 +110,6 @@ template <typename NCT> void read(uint8_t const*& it, RootRollupPublicInputs<NCT

read(it, obj.end_aggregation_object);
read(it, obj.globalVariables);
read(it, obj.block_hash);
read(it, obj.start_private_data_tree_snapshot);
read(it, obj.end_private_data_tree_snapshot);
read(it, obj.start_nullifier_tree_snapshot);
Expand Down Expand Up @@ -142,7 +138,6 @@ template <typename NCT> void write(std::vector<uint8_t>& buf, RootRollupPublicIn

write(buf, obj.end_aggregation_object);
write(buf, obj.globalVariables);
write(buf, obj.block_hash);
write(buf, obj.start_private_data_tree_snapshot);
write(buf, obj.end_private_data_tree_snapshot);
write(buf, obj.start_nullifier_tree_snapshot);
Expand All @@ -169,7 +164,6 @@ template <typename NCT> std::ostream& operator<<(std::ostream& os, RootRollupPub
{
return os << "end_aggregation_object: " << obj.end_aggregation_object << "\n"
<< "global_variables: " << obj.globalVariables << "\n"
<< "block_hash: " << obj.block_hash << "\n"
<< "start_private_data_tree_snapshot: " << obj.start_private_data_tree_snapshot << "\n"
<< "end_private_data_tree_snapshot: " << obj.end_private_data_tree_snapshot << "\n"
<< "start_nullifier_tree_snapshot: " << obj.start_nullifier_tree_snapshot << "\n"
Expand Down
1 change: 0 additions & 1 deletion circuits/cpp/src/aztec3/circuits/rollup/root/native_root_rollup_circuit.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ RootRollupPublicInputs root_rollup_circuit(DummyBuilder& builder, RootRollupInpu
RootRollupPublicInputs public_inputs = {
.end_aggregation_object = aggregation_object,
.globalVariables = left.constants.global_variables,
.block_hash = block_hash,
.start_private_data_tree_snapshot = left.start_private_data_tree_snapshot,
.end_private_data_tree_snapshot = right.end_private_data_tree_snapshot,
.start_nullifier_tree_snapshot = left.start_nullifier_tree_snapshot,
Expand Down
5 changes: 0 additions & 5 deletions circuits/cpp/src/aztec3/circuits/rollup/test_utils/utils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -383,18 +383,13 @@ RootRollupInputs get_root_rollup_inputs(utils::DummyBuilder& builder,

// Blocks tree
auto blocks_tree_sibling_path = get_sibling_path<HISTORIC_BLOCKS_TREE_HEIGHT>(historic_blocks_tree, 0, 0);
info("problematic sibling path");
info(blocks_tree_sibling_path);

// Blocks tree snapshots
AppendOnlyTreeSnapshot const start_historic_blocks_tree_snapshot = {
.root = historic_blocks_tree.root(),
.next_available_leaf_index = 0,
};

info("snapshot");
info(start_historic_blocks_tree_snapshot);

RootRollupInputs rootRollupInputs = {
.previous_rollup_data = get_previous_rollup_data(builder, std::move(kernel_data)),
.new_historic_private_data_tree_root_sibling_path = historic_data_sibling_path,
Expand Down
9 changes: 0 additions & 9 deletions yarn-project/circuits.js/src/structs/rollup/root_rollup.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,6 @@ export class RootRollupPublicInputs {
* Global variables of the L2 block.
*/
public globalVariables: GlobalVariables,
// constants: ConstantRollupData // TODO maybe don't include this

/**
* The hash of the block being included into the historic block hashes tree.
*/
public blockHash: Fr,

/**
* Snapshot of the private data tree at the start of the rollup.
*/
Expand Down Expand Up @@ -215,7 +208,6 @@ export class RootRollupPublicInputs {
return [
fields.endAggregationObject,
fields.globalVariables,
fields.blockHash,
fields.startPrivateDataTreeSnapshot,
fields.endPrivateDataTreeSnapshot,
fields.startNullifierTreeSnapshot,
Expand Down Expand Up @@ -274,7 +266,6 @@ export class RootRollupPublicInputs {
return new RootRollupPublicInputs(
reader.readObject(AggregationObject),
reader.readObject(GlobalVariables),
reader.readFr(),
reader.readObject(AppendOnlyTreeSnapshot),
reader.readObject(AppendOnlyTreeSnapshot),
reader.readObject(AppendOnlyTreeSnapshot),
Expand Down
10 changes: 6 additions & 4 deletions yarn-project/circuits.js/src/tests/factories.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,7 @@ export function makeGlobalVariables(seed = 1, blockNumber: number | undefined =
export function makeConstantBaseRollupData(
seed = 1,
blockNumber: number | undefined = undefined,
globalVariables: GlobalVariables | undefined = undefined,
): ConstantBaseRollupData {
return ConstantBaseRollupData.from({
startTreeOfHistoricPrivateDataTreeRootsSnapshot: makeAppendOnlyTreeSnapshot(seed),
Expand All @@ -709,7 +710,7 @@ export function makeConstantBaseRollupData(
publicKernelVkTreeRoot: fr(seed + 0x402),
baseRollupVkHash: fr(seed + 0x403),
mergeRollupVkHash: fr(seed + 0x404),
globalVariables: makeGlobalVariables(seed + 0x405, blockNumber),
globalVariables: globalVariables ?? makeGlobalVariables(seed + 0x405, blockNumber),
});
}

Expand Down Expand Up @@ -768,12 +769,13 @@ export function makeSchnorrSignature(seed = 1): SchnorrSignature {
export function makeBaseOrMergeRollupPublicInputs(
seed = 0,
blockNumber: number | undefined = undefined,
globalVariables: GlobalVariables | undefined = undefined,
): BaseOrMergeRollupPublicInputs {
return new BaseOrMergeRollupPublicInputs(
RollupTypes.Base,
new Fr(0n),
makeAggregationObject(seed + 0x100),
makeConstantBaseRollupData(seed + 0x200, blockNumber),
makeConstantBaseRollupData(seed + 0x200, blockNumber, globalVariables),
makeAppendOnlyTreeSnapshot(seed + 0x300),
makeAppendOnlyTreeSnapshot(seed + 0x400),
makeAppendOnlyTreeSnapshot(seed + 0x500),
Expand Down Expand Up @@ -833,11 +835,11 @@ export function makeRootRollupInputs(seed = 0, blockNumber: number | undefined =
export function makeRootRollupPublicInputs(
seed = 0,
blockNumber: number | undefined = undefined,
globalVariables: GlobalVariables | undefined = undefined,
): RootRollupPublicInputs {
return RootRollupPublicInputs.from({
endAggregationObject: makeAggregationObject(seed),
globalVariables: makeGlobalVariables((seed += 0x100), blockNumber),
blockHash: fr((seed += 0x100)),
globalVariables: globalVariables ?? makeGlobalVariables((seed += 0x100), blockNumber),
startPrivateDataTreeSnapshot: makeAppendOnlyTreeSnapshot((seed += 0x100)),
endPrivateDataTreeSnapshot: makeAppendOnlyTreeSnapshot((seed += 0x100)),
startNullifierTreeSnapshot: makeAppendOnlyTreeSnapshot((seed += 0x100)),
Expand Down
28 changes: 14 additions & 14 deletions yarn-project/sequencer-client/src/block_builder/solo_block_builder.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@ describe('sequencer/solo_block_builder', () => {
mockL1ToL2Messages = new Array(NUMBER_OF_L1_L2_MESSAGES_PER_ROLLUP).fill(new Fr(0n));

// Create mock outputs for simulator
baseRollupOutputLeft = makeBaseOrMergeRollupPublicInputs();
baseRollupOutputRight = makeBaseOrMergeRollupPublicInputs();
rootRollupOutput = makeRootRollupPublicInputs();
baseRollupOutputLeft = makeBaseOrMergeRollupPublicInputs(0, blockNumber, globalVariables);
baseRollupOutputRight = makeBaseOrMergeRollupPublicInputs(0, blockNumber, globalVariables);
rootRollupOutput = makeRootRollupPublicInputs(0, blockNumber, globalVariables);

// Set up mocks
prover.getBaseRollupProof.mockResolvedValue(emptyProof);
Expand Down Expand Up @@ -149,7 +149,16 @@ describe('sequencer/solo_block_builder', () => {
await expectsDb.appendLeaves(MerkleTreeId.L1_TO_L2_MESSAGES_TREE, asBuffer);
};

const updateHistoricBlocksTree = async (blockHash: Fr) => {
const updateHistoricBlocksTree = async () => {
const blockHash = computeBlockHash(
wasm,
globalVariables,
rootRollupOutput.endPrivateDataTreeSnapshot.root,
rootRollupOutput.endNullifierTreeSnapshot.root,
rootRollupOutput.endContractTreeSnapshot.root,
rootRollupOutput.endL1ToL2MessageTreeSnapshot.root,
rootRollupOutput.endPublicDataTreeRoot,
);
await expectsDb.appendLeaves(MerkleTreeId.BLOCKS_TREE, [blockHash.toBuffer()]);
};

Expand Down Expand Up @@ -212,16 +221,7 @@ describe('sequencer/solo_block_builder', () => {

// Calculate block hash
rootRollupOutput.globalVariables = globalVariables;
rootRollupOutput.blockHash = computeBlockHash(
wasm,
globalVariables,
rootRollupOutput.endPrivateDataTreeSnapshot.root,
rootRollupOutput.endNullifierTreeSnapshot.root,
rootRollupOutput.endContractTreeSnapshot.root,
rootRollupOutput.endL1ToL2MessageTreeSnapshot.root,
rootRollupOutput.endPublicDataTreeRoot,
);
await updateHistoricBlocksTree(rootRollupOutput.blockHash);
await updateHistoricBlocksTree();
rootRollupOutput.endHistoricBlocksTreeSnapshot = await getTreeSnapshot(MerkleTreeId.BLOCKS_TREE);

const txs = [...txsLeft, ...txsRight];
Expand Down
64 changes: 28 additions & 36 deletions yarn-project/sequencer-client/src/block_builder/solo_block_builder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ export class SoloBlockBuilder implements BlockBuilder {

// Run the root rollup with the last two merge rollups (or base, if no merge layers)
const [mergeOutputLeft, mergeOutputRight] = mergeRollupInputs;
return this.rootRollupCircuit(mergeOutputLeft, mergeOutputRight, newL1ToL2Messages, globalVariables);
return this.rootRollupCircuit(mergeOutputLeft, mergeOutputRight, newL1ToL2Messages);
}

protected async baseRollupCircuit(
Expand Down Expand Up @@ -295,7 +295,6 @@ export class SoloBlockBuilder implements BlockBuilder {
left: [BaseOrMergeRollupPublicInputs, Proof],
right: [BaseOrMergeRollupPublicInputs, Proof],
newL1ToL2Messages: Fr[],
globals: GlobalVariables,
): Promise<[RootRollupPublicInputs, Proof]> {
this.debug(`Running root rollup circuit`);
const rootInput = await this.getRootRollupInput(...left, ...right, newL1ToL2Messages);
Expand All @@ -315,41 +314,39 @@ export class SoloBlockBuilder implements BlockBuilder {
// and validate them against the output of the root circuit simulation
this.debug(`Updating and validating root trees`);
await this.db.updateHistoricRootsTrees();
await this.updateHistoricBlocksTree(left[0].constants.globalVariables);

// Calculate the block hash and add it to the historic block hashes tree
const blockHash = await this.calculateBlockHash(globals);
await this.db.appendLeaves(MerkleTreeId.BLOCKS_TREE, [blockHash.toBuffer()]);

await this.validateRootOutput(blockHash, rootOutput);
await this.validateRootOutput(rootOutput);

return [rootOutput, rootProof];
}

protected async calculateBlockHash(globals: GlobalVariables) {
const [privateDataTreeRoot, nullifierTreeRoot, contractTreeRoot, publicDataTreeRoot, l1ToL2MessageTreeRoot] = (
await Promise.all(
[
MerkleTreeId.PRIVATE_DATA_TREE,
MerkleTreeId.NULLIFIER_TREE,
MerkleTreeId.CONTRACT_TREE,
MerkleTreeId.PUBLIC_DATA_TREE,
MerkleTreeId.L1_TO_L2_MESSAGES_TREE,
].map(tree => this.getTreeSnapshot(tree)),
)
).map(r => r.root);
async updateHistoricBlocksTree(globalVariables: GlobalVariables) {
// Calculate the block hash and add it to the historic block hashes tree
const blockHash = await this.calculateBlockHash(globalVariables);
await this.db.appendLeaves(MerkleTreeId.BLOCKS_TREE, [blockHash.toBuffer()]);
}

const wasm = await CircuitsWasm.get();
const blockHash = computeBlockHash(
wasm,
globals,
protected async calculateBlockHash(globals: GlobalVariables) {
const [
privateDataTreeRoot,
nullifierTreeRoot,
contractTreeRoot,
l1ToL2MessageTreeRoot,
publicDataTreeRoot,
);
l1ToL2MessageTreeRoot,
] = (await Promise.all(
[
MerkleTreeId.PRIVATE_DATA_TREE,
MerkleTreeId.NULLIFIER_TREE,
MerkleTreeId.CONTRACT_TREE,
MerkleTreeId.PUBLIC_DATA_TREE,
MerkleTreeId.L1_TO_L2_MESSAGES_TREE,
].map(tree => this.getTreeSnapshot(tree)))).map(r => r.root);

const wasm = await CircuitsWasm.get();
const blockHash = computeBlockHash(wasm, globals, privateDataTreeRoot, nullifierTreeRoot, contractTreeRoot, l1ToL2MessageTreeRoot, publicDataTreeRoot);
return blockHash;
}
}

// Validate that the new roots we calculated from manual insertions match the outputs of the simulation
protected async validateTrees(rollupOutput: BaseOrMergeRollupPublicInputs | RootRollupPublicInputs) {
Expand All @@ -362,14 +359,7 @@ export class SoloBlockBuilder implements BlockBuilder {
}

// Validate that the roots of all local trees match the output of the root circuit simulation
protected async validateRootOutput(blockHash: Fr, rootOutput: RootRollupPublicInputs) {
// Check the calculated block hash is the same as is calculated within the circuits.
if (!blockHash.equals(rootOutput.blockHash)) {
throw new Error(
`Block hash calculated from the circuit output ${rootOutput.blockHash} does not match the block hash calculated from the data ${blockHash}`,
);
}

protected async validateRootOutput(rootOutput: RootRollupPublicInputs) {
await Promise.all([
this.validateTrees(rootOutput),
this.validateRootTree(rootOutput, MerkleTreeId.CONTRACT_TREE_ROOTS_TREE, 'Contract'),
Expand Down Expand Up @@ -484,7 +474,9 @@ export class SoloBlockBuilder implements BlockBuilder {

// Get historic block tree roots
const startHistoricBlocksTreeSnapshot = await this.getTreeSnapshot(MerkleTreeId.BLOCKS_TREE);
const newHistoricBlocksTreeSiblingPath = await getRootTreeSiblingPath(MerkleTreeId.BLOCKS_TREE);
const newHistoricBlocksTreeSiblingPath = await getRootTreeSiblingPath(
MerkleTreeId.BLOCKS_TREE,
);

return RootRollupInputs.from({
previousRollupData,
Expand All @@ -496,7 +488,7 @@ export class SoloBlockBuilder implements BlockBuilder {
startL1ToL2MessageTreeSnapshot,
startHistoricTreeL1ToL2MessageTreeRootsSnapshot,
startHistoricBlocksTreeSnapshot,
newHistoricBlocksTreeSiblingPath,
newHistoricBlocksTreeSiblingPath
});
}

Expand Down

0 comments on commit dc20631

Please sign in to comment.