Fix two TODOs

Author: c410-f3r

ceno_cli/src/commands/common_args/ceno.rs

@@ -307,15 +307,15 @@ impl CenoOptions {
                     self,
                     compilation_options,
                     elf_path,
-                    Checkpoint::PrepVerify, // FIXME: when whir and babybear is ready
+                    Checkpoint::PrepVerify, // TODO: when whir and babybear is ready
                 )
             }
             (PcsKind::Whir, FieldType::BabyBear) => {
                 prove_inner::<BabyBearExt4, Whir<BabyBearExt4, WhirDefaultSpec>, P>(
                     self,
                     compilation_options,
                     elf_path,
-                    Checkpoint::PrepVerify, // FIXME: when whir and babybear is ready
+                    Checkpoint::PrepVerify, // TODO: when whir and babybear is ready
                 )
             }
         }

ceno_emul/src/syscalls/sha256.rs

@@ -38,7 +38,6 @@ pub fn sha_extend(w: &mut [u32]) {
             .wrapping_add(s0)
             .wrapping_add(w[i - 7])
             .wrapping_add(s1);
-        // TODO: why doesn't sp1 use wrapping_add?
     }
 }
 

ceno_rt/src/lib.rs

@@ -6,7 +6,8 @@ use getrandom::{Error, register_custom_getrandom};
 use core::arch::{asm, global_asm};
 use std::{
     alloc::{Layout, alloc_zeroed},
-    ptr::null,
+    ptr,
+    sync::atomic::{AtomicU32, Ordering},
 };
 
 #[cfg(target_arch = "riscv32")]
@@ -37,7 +38,7 @@ pub extern "C" fn sys_alloc_words(nwords: usize) -> *mut u32 {
 #[unsafe(no_mangle)]
 #[linkage = "weak"]
 pub extern "C" fn sys_getenv(_name: *const u8) -> *const u8 {
-    null()
+    ptr::null()
 }
 
 /// Generates random bytes.
@@ -49,26 +50,37 @@ pub extern "C" fn sys_getenv(_name: *const u8) -> *const u8 {
 #[unsafe(no_mangle)]
 #[linkage = "weak"]
 pub unsafe extern "C" fn sys_rand(recv_buf: *mut u8, words: usize) {
-    unsafe fn step() -> u32 {
-        static mut X: u32 = 0xae569764;
-        // We are stealing Borland Delphi's random number generator.
-        // The random numbers here are only good enough to make eg
-        // HashMap work.
+    // We are stealing Borland Delphi's random number generator.
+    // The random numbers here are only good enough to make eg
+    // HashMap work.
+    fn step() -> u32 {
+        static X: AtomicU32 = AtomicU32::new(0xae569764);
+        let rslt = X.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |x| {
+            Some(x.wrapping_mul(134775813).wrapping_add(1))
+        });
+        match rslt {
+            Ok(el) => el,
+            Err(err) => err,
+        }
+    }
+    let mut idx = 0;
+    let steps = words / 4;
+    let rest = words % 4;
+    for _ in 0..steps {
+        let bytes = step().to_le_bytes();
+        // SAFETY: Up to the caller
         unsafe {
-            X = X.wrapping_mul(134775813) + 1;
-            X
+            ptr::copy_nonoverlapping(bytes.as_ptr(), recv_buf.add(idx), 4);
         }
+        idx = idx.wrapping_add(4);
     }
-    // TODO(Matthias): this is a bit inefficient,
-    // we could fill whole u32 words at a time.
-    // But it's just for testing.
-    for i in 0..words {
+    let [a, b, c, _] = step().to_le_bytes();
+    for (_, el) in (0..rest).zip([a, b, c]) {
+        // SAFETY: Up to the caller
         unsafe {
-            let element = recv_buf.add(i);
-            // The lower bits ain't really random, so might as well take
-            // the higher order ones, if we are only using 8 bits.
-            *element = step().to_le_bytes()[3];
+            *recv_buf.add(idx) = el;
         }
+        idx = idx.wrapping_add(1);
     }
 }
 
@@ -153,3 +165,17 @@ unsafe extern "C" {
     // The address of this variable is the start of the stack (growing downwards).
     static _stack_start: u8;
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::sys_rand;
+
+    #[test]
+    fn fills_with_random_bytes() {
+        let mut buf = [0u8; 65];
+        unsafe {
+            sys_rand(buf.as_mut_ptr(), buf.len());
+        }
+        assert_ne!(buf, [0u8; 65]);
+    }
+}

ceno_zkvm/src/bin/e2e.rs

@@ -284,7 +284,7 @@ fn main() {
                 max_steps,
                 args.proof_file,
                 args.vk_file,
-                Checkpoint::PrepVerify, // FIXME: when whir and babybear is ready
+                Checkpoint::PrepVerify, // TODO: when whir and babybear is ready
             )
         }
         (PcsKind::Whir, FieldType::BabyBear) => {
@@ -299,7 +299,7 @@ fn main() {
                 max_steps,
                 args.proof_file,
                 args.vk_file,
-                Checkpoint::PrepVerify, // FIXME: when whir and babybear is ready
+                Checkpoint::PrepVerify, // TODO: when whir and babybear is ready
             )
         }
     };

ceno_zkvm/src/instructions.rs

@@ -106,7 +106,7 @@ pub trait Instruction<E: ExtensionField> {
         num_structural_witin: usize,
         steps: Vec<StepRecord>,
     ) -> Result<(RMMCollections<E::BaseField>, Multiplicity<u64>), ZKVMError> {
-        // FIXME selector is the only structural witness
+        // TODO: selector is the only structural witness
         // this is workaround, as call `construct_circuit` will not initialized selector
         // we can remove this one all opcode unittest migrate to call `build_gkr_iop_circuit`
         assert!(num_structural_witin == 0 || num_structural_witin == 1);

ceno_zkvm/src/scheme/prover.rs

@@ -254,7 +254,7 @@ impl<
                     evaluations.push(opcode_proof.wits_in_evals.clone());
                     chip_proofs.insert(index, opcode_proof);
                 } else {
-                    // FIXME: PROGRAM table circuit is not guaranteed to have 2^n instances
+                    // TODO: PROGRAM table circuit is not guaranteed to have 2^n instances
                     input.num_instances = 1 << input.log2_num_instances();
                     let (mut table_proof, pi_in_evals, input_opening_point) = self
                         .create_chip_proof(circuit_name, pk, input, &mut transcript, &challenges)?;
@@ -266,7 +266,7 @@ impl<
                         ]
                         .concat(),
                     );
-                    // FIXME: PROGRAM table circuit is not guaranteed to have 2^n instances
+                    // TODO: PROGRAM table circuit is not guaranteed to have 2^n instances
                     table_proof.num_instances = num_instances;
                     chip_proofs.insert(index, table_proof);
                     for (idx, eval) in pi_in_evals {

gkr_iop/src/gkr/layer/cpu/mod.rs

@@ -302,7 +302,7 @@ pub(crate) fn prove_rotation<E: ExtensionField, PCS: PolynomialCommitmentScheme<
     )
     .collect_vec();
     exit_span!(span);
-    // TODO FIXME: we pick a random point from output point, does it sound?
+    // TODO: we pick a random point from output point, does it sound?
     let builder = VirtualPolynomialsBuilder::new_with_mles(
         num_threads,
         max_num_variables,

gkr_iop/src/gkr/layer_constraint_system.rs

@@ -1,4 +1,4 @@
-/// TODO FIXME LayerConstrainSystem is deprecated
+/// TODO: LayerConstrainSystem is deprecated
 use std::{cmp::Ordering, collections::BTreeMap};
 
 use crate::{
@@ -134,7 +134,7 @@ impl<E: ExtensionField> LayerConstraintSystem<E> {
         assert!(size <= 16);
         let rlc_record = rlc_chip_record(
             vec![
-                // TODO FIXME layer constrain system is deprecated
+                // TODO: layer constrain system is deprecated
                 E::BaseField::from_canonical_u64(LookupTable::Dynamic as u64).expr(),
                 value.clone(),
             ],