Skip to content

Commit

Permalink
refactor: aligning some naming in BaseOrMergeRollupPublicInputs (#4510
Browse files Browse the repository at this point in the history
)

Partially fixes #3849

**Note**: Not fully tackling the issue here because the yellow paper
needs to be updated first.
  • Loading branch information
benesjan committed Feb 9, 2024
1 parent 79bf445 commit 47d66f9
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ struct BaseOrMergeRollupPublicInputs {
rollup_type : u32,
// subtree height is always 0 for base.
// so that we always pass-in two base/merge circuits of the same height into the next level of recursion
// TODO(benesjan): rename to height_in_block_tree
rollup_subtree_height : Field,
height_in_block_tree : Field,
aggregation_object : AggregationObject,
constants : ConstantRollupData,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ impl BaseRollupInputs {

BaseOrMergeRollupPublicInputs {
rollup_type : BASE_ROLLUP_TYPE,
rollup_subtree_height : 0,
height_in_block_tree : 0,
aggregation_object : aggregation_object,
constants : self.constants,
start: self.start,
Expand Down Expand Up @@ -1300,7 +1300,7 @@ mod tests {
unconstrained fn subtree_height_is_0() {
let outputs = BaseRollupInputsBuilder::new().execute();

assert_eq(outputs.rollup_subtree_height, 0);
assert_eq(outputs.height_in_block_tree, 0);
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ pub fn assert_both_input_proofs_of_same_height_and_return(
right: BaseOrMergeRollupPublicInputs
) -> Field {
assert(
left.rollup_subtree_height == right.rollup_subtree_height, "input proofs are of different rollup heights"
left.height_in_block_tree == right.height_in_block_tree, "input proofs are of different rollup heights"
);
left.rollup_subtree_height
left.height_in_block_tree
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ impl MergeRollupInputs {

let public_inputs = BaseOrMergeRollupPublicInputs {
rollup_type : MERGE_ROLLUP_TYPE,
rollup_subtree_height : current_height + 1,
height_in_block_tree : current_height + 1,
aggregation_object : aggregation_object,
constants : left.constants,
start : left.start,
Expand Down Expand Up @@ -60,8 +60,8 @@ mod tests {
#[test(should_fail_with="input proofs are of different rollup heights")]
fn different_height_fails() {
let mut inputs = default_merge_rollup_inputs();
inputs.previous_rollup_data[0].base_or_merge_rollup_public_inputs.rollup_subtree_height = 0;
inputs.previous_rollup_data[1].base_or_merge_rollup_public_inputs.rollup_subtree_height = 1;
inputs.previous_rollup_data[0].base_or_merge_rollup_public_inputs.height_in_block_tree = 0;
inputs.previous_rollup_data[1].base_or_merge_rollup_public_inputs.height_in_block_tree = 1;
let _output = inputs.merge_rollup_circuit();
}

Expand Down Expand Up @@ -111,19 +111,19 @@ mod tests {
let mut outputs = inputs.merge_rollup_circuit();
assert_eq(outputs.rollup_type, 1);
assert_eq(
outputs.rollup_subtree_height, inputs.previous_rollup_data[0].base_or_merge_rollup_public_inputs.rollup_subtree_height + 1
outputs.height_in_block_tree, inputs.previous_rollup_data[0].base_or_merge_rollup_public_inputs.height_in_block_tree + 1
);

// set inputs to have a merge rollup type and set the rollup height and test again.
inputs.previous_rollup_data[0].base_or_merge_rollup_public_inputs.rollup_type = 1;
inputs.previous_rollup_data[0].base_or_merge_rollup_public_inputs.rollup_subtree_height = 1;
inputs.previous_rollup_data[0].base_or_merge_rollup_public_inputs.height_in_block_tree = 1;

inputs.previous_rollup_data[1].base_or_merge_rollup_public_inputs.rollup_type = 1;
inputs.previous_rollup_data[1].base_or_merge_rollup_public_inputs.rollup_subtree_height = 1;
inputs.previous_rollup_data[1].base_or_merge_rollup_public_inputs.height_in_block_tree = 1;

outputs = inputs.merge_rollup_circuit();
assert_eq(outputs.rollup_type, 1);
assert_eq(outputs.rollup_subtree_height, 2);
assert_eq(outputs.height_in_block_tree, 2);
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ pub fn default_previous_rollup_data() -> [PreviousRollupData; 2] {
previous_rollup_data[0].base_or_merge_rollup_public_inputs.rollup_type = BASE_ROLLUP_TYPE;
previous_rollup_data[1].base_or_merge_rollup_public_inputs.rollup_type = BASE_ROLLUP_TYPE;

previous_rollup_data[0].base_or_merge_rollup_public_inputs.rollup_subtree_height = 1;
previous_rollup_data[1].base_or_merge_rollup_public_inputs.rollup_subtree_height = 1;
previous_rollup_data[0].base_or_merge_rollup_public_inputs.height_in_block_tree = 1;
previous_rollup_data[1].base_or_merge_rollup_public_inputs.height_in_block_tree = 1;

previous_rollup_data[0].base_or_merge_rollup_public_inputs.calldata_hash = [0, 1];
previous_rollup_data[1].base_or_merge_rollup_public_inputs.calldata_hash = [2, 3];
Expand Down
4 changes: 2 additions & 2 deletions yarn-project/noir-protocol-circuits/src/type_conversion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1223,7 +1223,7 @@ export function mapBaseOrMergeRollupPublicInputsToNoir(
): BaseOrMergeRollupPublicInputsNoir {
return {
rollup_type: mapFieldToNoir(new Fr(baseOrMergeRollupPublicInputs.rollupType)),
rollup_subtree_height: mapFieldToNoir(new Fr(baseOrMergeRollupPublicInputs.rollupSubtreeHeight)),
height_in_block_tree: mapFieldToNoir(new Fr(baseOrMergeRollupPublicInputs.rollupSubtreeHeight)),
aggregation_object: {},
constants: mapConstantRollupDataToNoir(baseOrMergeRollupPublicInputs.constants),
start: mapPartialStateReferenceToNoir(baseOrMergeRollupPublicInputs.start),
Expand Down Expand Up @@ -1270,7 +1270,7 @@ export function mapBaseOrMergeRollupPublicInputsFromNoir(
): BaseOrMergeRollupPublicInputs {
return new BaseOrMergeRollupPublicInputs(
mapNumberFromNoir(baseOrMergeRollupPublicInputs.rollup_type),
mapFieldFromNoir(baseOrMergeRollupPublicInputs.rollup_subtree_height),
mapFieldFromNoir(baseOrMergeRollupPublicInputs.height_in_block_tree),
AggregationObject.makeFake(),
mapConstantRollupDataFromNoir(baseOrMergeRollupPublicInputs.constants),
mapPartialStateReferenceFromNoir(baseOrMergeRollupPublicInputs.start),
Expand Down

0 comments on commit 47d66f9

Please sign in to comment.