Skip to content
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

refactor: aligning some naming in BaseOrMergeRollupPublicInputs #4510

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -110,7 +110,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 @@ -1259,7 +1259,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
Loading