Skip to content

Commit

Permalink
fix: Add EIP4844 to fri_prover_group_config (#1309)
Browse files Browse the repository at this point in the history 
255 corresponds to EIP4844. This is added to the group of expected
circuits (base circuit as of 4844). Additionally, fixed panicing config
by turning it into an Result.

This is needed for 4844 release and prevents outages caused by the part
of the config that was touched.
  • Loading branch information
@EmilLuta
EmilLuta authored Mar 1, 2024
1 parent 54f8d8c commit edf9397
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 65 deletions.
92 changes: 29 additions & 63 deletions core/lib/config/src/configs/fri_prover_group.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,11 @@ impl FriProverGroupConfig {
}
/// check all_circuit ids present exactly once
/// and For each aggregation round, check that the circuit ids are in the correct range.
/// For example, in aggregation round 0, the circuit ids should be 1 to 13.
/// For example, in aggregation round 0, the circuit ids should be 1 to 13 + 255 (EIP4844).
/// In aggregation round 1, the circuit ids should be 3 to 15.
/// In aggregation round 2, the circuit ids should be 2.
/// In aggregation round 3, the circuit ids should be 1.
pub fn validate(&self) {
pub fn validate(&self) -> anyhow::Result<()> {
let mut rounds: Vec<Vec<CircuitIdRoundTuple>> = vec![Vec::new(); 4];
let groups = [
&self.group_0,
Expand Down Expand Up @@ -113,111 +113,77 @@ impl FriProverGroupConfig {
.copied()
.collect();

match round {
let (missing_ids, not_in_range, expected_circuits_description) = match round {
0 => {
let expected_range: Vec<_> = (1..=13).collect();
let mut expected_range: Vec<_> = (1..=13).collect();
// temporary EIP4844 1.4.2 circuits
// will be removed with a proper circuit id in 1.5.0
expected_range.push(255);
let missing_ids: Vec<_> = expected_range
.iter()
.copied()
.filter(|id| !circuit_ids.contains(id))
.collect();
assert!(
missing_ids.is_empty(),
"Circuit IDs for round {} are missing: {:?}",
round,
missing_ids
);
assert_eq!(
circuit_ids.len(),
unique_circuit_ids.len(),
"Circuit IDs: {:?} should be unique for round {}.",
duplicates,
round
);

let not_in_range: Vec<_> = circuit_ids
.iter()
.filter(|&id| !expected_range.contains(id))
.collect();
assert!(not_in_range.is_empty(), "Aggregation round 0 should only contain circuit IDs 1 to 13. Ids out of range: {:?}", not_in_range);
(missing_ids, not_in_range, "circuit IDs 1 to 13 and 255")
}
1 => {
let expected_range: Vec<_> = (3..=15).collect();
let missing_ids: Vec<_> = expected_range
.iter()
.copied()
.filter(|id| !circuit_ids.contains(id))
.collect();
assert!(
missing_ids.is_empty(),
"Circuit IDs for round {} are missing: {:?}",
round,
missing_ids
);
assert_eq!(
circuit_ids.len(),
unique_circuit_ids.len(),
"Circuit IDs: {:?} should be unique for round {}.",
duplicates,
round
);
let not_in_range: Vec<_> = circuit_ids
.iter()
.filter(|&id| !expected_range.contains(id))
.collect();
assert!(not_in_range.is_empty(), "Aggregation round 1 should only contain circuit IDs 3 to 15. Ids out of range: {:?}", not_in_range);
(missing_ids, not_in_range, "circuit IDs 3 to 15")
}
2 => {
let expected_range = [2];
let expected_range: Vec<_> = vec![2];
let missing_ids: Vec<_> = expected_range
.iter()
.copied()
.filter(|id| !circuit_ids.contains(id))
.collect();
assert!(
missing_ids.is_empty(),
"Circuit IDs for round {} are missing: {:?}",
round,
missing_ids
);
assert_eq!(
circuit_ids.len(),
unique_circuit_ids.len(),
"Circuit IDs: {:?} should be unique for round {}.",
duplicates,
round
);
let not_in_range: Vec<_> = circuit_ids
.iter()
.filter(|&id| !expected_range.contains(id))
.collect();
assert!(not_in_range.is_empty(), "Aggregation round 2 should only contain circuit ID 2. Ids out of range: {:?}", not_in_range);
(missing_ids, not_in_range, "circuit ID 2")
}
3 => {
let expected_range = [1];
let expected_range: Vec<_> = vec![1];
let missing_ids: Vec<_> = expected_range
.iter()
.copied()
.filter(|id| !circuit_ids.contains(id))
.collect();
assert!(
missing_ids.is_empty(),
"Circuit IDs for round {} are missing: {:?}",
round,
missing_ids
);
assert_eq!(
circuit_ids.len(),
unique_circuit_ids.len(),
"Circuit IDs: {:?} should be unique for round {}.",
duplicates,
round
);
let not_in_range: Vec<_> = circuit_ids
.iter()
.filter(|&id| !expected_range.contains(id))
.collect();
assert!(not_in_range.is_empty(), "Aggregation round 3 should only contain circuit ID 1. Ids out of range: {:?}", not_in_range);
(missing_ids, not_in_range, "circuit ID 1")
}
_ => {
panic!("Unknown round {}", round)
anyhow::bail!("Unknown round {}", round);
}
};
if !missing_ids.is_empty() {
anyhow::bail!("Circuit IDs for round {round} are missing: {missing_ids:?}");
}
if circuit_ids.len() != unique_circuit_ids.len() {
anyhow::bail!("Circuit IDs: {duplicates:?} should be unique for round {round}.",);
}
if !not_in_range.is_empty() {
anyhow::bail!("Aggregation round {round} should only contain {expected_circuits_description}. Ids out of range: {not_in_range:?}");
}
}
Ok(())
}
}
17 changes: 16 additions & 1 deletion core/lib/env_config/src/fri_prover_group.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl FromEnv for FriProverGroupConfig {
group_11: groups.remove("group_11").unwrap_or_default(),
group_12: groups.remove("group_12").unwrap_or_default(),
};
config.validate();
config.validate()?;
Ok(config)
}
}
Expand Down Expand Up @@ -105,6 +105,7 @@ mod tests {
CircuitIdRoundTuple::new(11, 0),
CircuitIdRoundTuple::new(12, 0),
CircuitIdRoundTuple::new(13, 0),
CircuitIdRoundTuple::new(255, 0),
]
.into_iter()
.collect::<HashSet<_>>(),
Expand Down Expand Up @@ -173,6 +174,10 @@ mod tests {
"FRI_PROVER_GROUP_GROUP_4_2",
CircuitIdRoundTuple::new(13, 0),
),
(
"FRI_PROVER_GROUP_GROUP_4_3",
CircuitIdRoundTuple::new(255, 0),
),
("FRI_PROVER_GROUP_GROUP_5_0", CircuitIdRoundTuple::new(5, 0)),
("FRI_PROVER_GROUP_GROUP_6_0", CircuitIdRoundTuple::new(3, 1)),
("FRI_PROVER_GROUP_GROUP_7_0", CircuitIdRoundTuple::new(7, 0)),
Expand Down Expand Up @@ -286,6 +291,16 @@ mod tests {
fri_prover_group_config.get_group_id_for_circuit_id_and_aggregation_round(12, 0)
);

assert_eq!(
Some(4),
fri_prover_group_config.get_group_id_for_circuit_id_and_aggregation_round(13, 0)
);

assert_eq!(
Some(4),
fri_prover_group_config.get_group_id_for_circuit_id_and_aggregation_round(255, 0)
);

assert_eq!(
Some(5),
fri_prover_group_config.get_group_id_for_circuit_id_and_aggregation_round(5, 0)
Expand Down
2 changes: 1 addition & 1 deletion etc/env/base/fri_prover_group.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ group_0 = [{"circuit_id"=1,"aggregation_round"=3},{"circuit_id"=2,"aggregation_r
group_1 = [{"circuit_id"=1,"aggregation_round"=0}]
group_2 = [{"circuit_id"=2,"aggregation_round"=0},{"circuit_id"=4,"aggregation_round"=0},{"circuit_id"=6,"aggregation_round"=0},{"circuit_id"=9,"aggregation_round"=0}]
group_3 = [{"circuit_id"=3,"aggregation_round"=0}]
group_4 = [{"circuit_id"=11,"aggregation_round"=0},{"circuit_id"=12,"aggregation_round"=0},{"circuit_id"=13,"aggregation_round"=0}]
group_4 = [{"circuit_id"=11,"aggregation_round"=0},{"circuit_id"=12,"aggregation_round"=0},{"circuit_id"=13,"aggregation_round"=0},{"circuit_id"=255,"aggregation_round"=0}]
group_5 = [{"circuit_id"=5,"aggregation_round"=0}]
group_6 = [{"circuit_id"=3,"aggregation_round"=1}]
group_7 = [{"circuit_id"=7,"aggregation_round"=0}]
Expand Down

0 comments on commit edf9397

Please sign in to comment.