Skip to content

Commit

Permalink
chore: cleanup FIXMEs in code (#1292)
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathanpwang authored Jan 26, 2025
1 parent a3d666c commit e855aef
Show file tree
Hide file tree
Showing 10 changed files with 11 additions and 18 deletions.
12 changes: 6 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion crates/sdk/src/keygen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,6 @@ impl AggProvingKey {
let dummy_root_proof = agg_stark_pk
.root_verifier_pk
.generate_dummy_root_proof(dummy_internal_proof);
// FIXME: Halo2VerifierProvingKey is not Send + Sync because Array/Usize use Rc<RefCell>.
let verifier = agg_stark_pk.root_verifier_pk.keygen_static_verifier(
&reader.read_params(halo2_config.verifier_k),
dummy_root_proof,
Expand Down
1 change: 0 additions & 1 deletion crates/sdk/src/keygen/perm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use openvm_circuit::arch::{CONNECTOR_AIR_ID, PROGRAM_AIR_ID, PUBLIC_VALUES_AIR_I

use crate::verifier::common::types::SpecialAirIds;

// FIXME: is there a good crate to replace this?
pub struct AirIdPermutation {
pub perm: Vec<usize>,
}
Expand Down
3 changes: 0 additions & 3 deletions crates/sdk/src/verifier/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ pub fn assert_single_segment_vm_exit_successfully_with_connector_air_id<C: Confi
connector_air_id: usize,
) {
let connector_pvs = get_connector_pvs_impl(builder, proof, connector_air_id);
// FIXME: does single segment VM program always have pc_start = 0?
// Start PC should be 0
builder.assert_felt_eq(connector_pvs.initial_pc, C::F::ZERO);
// Terminate should be 1
Expand All @@ -172,7 +171,6 @@ pub fn assert_required_air_for_agg_vm_present<C: Config>(
builder: &mut Builder<C>,
proof: &StarkProofVariable<C>,
) {
// FIXME: what if PUBLIC_VALUES_AIR_ID(3) >= proof.per_air.len()?
let program_air = builder.get(&proof.per_air, PROGRAM_AIR_ID);
builder.assert_eq::<Usize<_>>(program_air.air_id, RVar::from(PROGRAM_AIR_ID));
let connector_air = builder.get(&proof.per_air, CONNECTOR_AIR_ID);
Expand All @@ -187,7 +185,6 @@ pub fn assert_required_air_for_app_vm_present<C: Config>(
builder: &mut Builder<C>,
proof: &StarkProofVariable<C>,
) {
// FIXME: what if MERKLE_AIR_ID(4) >= proof.per_air.len()?
let program_air = builder.get(&proof.per_air, PROGRAM_AIR_ID);
builder.assert_eq::<Usize<_>>(program_air.air_id, RVar::from(PROGRAM_AIR_ID));
let connector_air = builder.get(&proof.per_air, CONNECTOR_AIR_ID);
Expand Down
1 change: 0 additions & 1 deletion crates/sdk/src/verifier/leaf/vars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ impl Hintable<C> for UserPublicValuesRootProof<F> {
let len = builder.hint_var();
let sibling_hashes = builder.array(len);
builder.range(0, len).for_each(|i_vec, builder| {
// FIXME: add hint support for slices.
let hash = array::from_fn(|_| builder.hint_felt());
builder.set_value(&sibling_hashes, i_vec[0], hash);
});
Expand Down
4 changes: 2 additions & 2 deletions crates/vm/src/metrics/cycle_tracker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ impl CycleTracker {
/// Starts a new cycle tracker span for the given name.
/// If a span already exists for the given name, it ends the existing span and pushes a new one to the vec.
pub fn start(&mut self, mut name: String) {
// FIXME: hack to remove "CT-" prefix
// hack to remove "CT-" prefix
if name.starts_with("CT-") {
name = name.split_off(3);
}
Expand All @@ -22,7 +22,7 @@ impl CycleTracker {
/// Ends the cycle tracker span for the given name.
/// If no span exists for the given name, it panics.
pub fn end(&mut self, mut name: String) {
// FIXME: hack to remove "CT-" prefix
// hack to remove "CT-" prefix
if name.starts_with("CT-") {
name = name.split_off(3);
}
Expand Down
1 change: 0 additions & 1 deletion extensions/ecc/guest/src/msm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use super::Group;

/// Multi-scalar multiplication via Pippenger's algorithm
// Reference: https://github.com/privacy-scaling-explorations/halo2curves/blob/8771fe5a5d54fc03e74dbc8915db5dad3ab46a83/src/msm.rs#L335
// FIXME[jpw]: there are many memcpy in this function
pub fn msm<EcPoint: Group, Scalar: IntMod>(coeffs: &[Scalar], bases: &[EcPoint]) -> EcPoint
where
for<'a> &'a EcPoint: Add<&'a EcPoint, Output = EcPoint>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ pub fn convert_efr<F: PrimeField, EF: ExtensionField<F>>(a: &EF) -> Vec<Fr> {
fn stats_snapshot(ctx: &Context<Fr>, range_chip: Arc<RangeChip<Fr>>) -> Halo2Stats {
Halo2Stats {
total_gate_cell: ctx.advice.len(),
// FIXME: this is inaccurate because of duplicated constants. But it's too slow if we always
// Note[Xinding]: this is inaccurate because of duplicated constants. But it's too slow if we always
// check for duplicates.
total_fixed: ctx.copy_manager.lock().unwrap().constant_equalities.len(),
total_lookup_cell: range_chip.lookup_manager()[0].total_rows(),
Expand Down
2 changes: 1 addition & 1 deletion extensions/native/compiler/tests/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ fn test_fixed_array_var() {
.range(0, fixed_array.len())
.for_each(|i_vec, builder| {
let value: Usize<_> = builder.get(&fixed_array, i_vec[0]);
// `len` instructions to initialize variables. FIXME: this is not optimal.
// `len` instructions to initialize variables.
// `len` instructions of `assert_eq`
builder.assert_eq::<Var<_>>(value, RVar::from(2));
});
Expand Down
2 changes: 1 addition & 1 deletion extensions/native/tests/programs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ std = ["openvm/std"]
openvm-native-compiler = { path = "../../compiler", default-features = false }
openvm-native-transpiler = { path = "../../transpiler", default-features = false }
openvm-instructions = { path = "../../../../crates/toolchain/instructions", default-features = false }
openvm-stark-sdk = { git = "https://github.com/openvm-org/stark-backend.git", rev = "8c9f94", default-features = false } # FIXME
openvm-stark-sdk = { git = "https://github.com/openvm-org/stark-backend.git", rev = "8c9f94", default-features = false }
openvm-stark-backend = { git = "https://github.com/openvm-org/stark-backend.git", rev = "8c9f94", default-features = false }

0 comments on commit e855aef

Please sign in to comment.