signoff!: u32::shift_left and u32::shift_right

BREAKING CHANGE: Remove the implementations of `DeprecatedSnippet` for
`u32::shift_left::ShiftLeft` and `u32::shift_right::ShiftRight`,
implement `BasicSnippet` directly.

Author: jan-ferdinand

tasm-lib/benchmarks/tasmlib_arithmetic_i128_shift_right.json

@@ -2,21 +2,21 @@
   {
     "name": "tasmlib_arithmetic_i128_shift_right",
     "benchmark_result": {
-      "clock_cycle_count": 250,
+      "clock_cycle_count": 245,
       "hash_table_height": 120,
       "u32_table_height": 290,
-      "op_stack_table_height": 189,
+      "op_stack_table_height": 179,
       "ram_table_height": 0
     },
     "case": "CommonCase"
   },
   {
     "name": "tasmlib_arithmetic_i128_shift_right",
     "benchmark_result": {
-      "clock_cycle_count": 236,
+      "clock_cycle_count": 231,
       "hash_table_height": 120,
       "u32_table_height": 310,
-      "op_stack_table_height": 181,
+      "op_stack_table_height": 171,
       "ram_table_height": 0
     },
     "case": "WorstCase"

tasm-lib/benchmarks/tasmlib_arithmetic_u32_shift_right.json

@@ -2,21 +2,21 @@
   {
     "name": "tasmlib_arithmetic_u32_shift_right",
     "benchmark_result": {
-      "clock_cycle_count": 16,
+      "clock_cycle_count": 15,
       "hash_table_height": 18,
       "u32_table_height": 46,
-      "op_stack_table_height": 13,
+      "op_stack_table_height": 11,
       "ram_table_height": 0
     },
     "case": "CommonCase"
   },
   {
     "name": "tasmlib_arithmetic_u32_shift_right",
     "benchmark_result": {
-      "clock_cycle_count": 16,
+      "clock_cycle_count": 15,
       "hash_table_height": 18,
       "u32_table_height": 46,
-      "op_stack_table_height": 13,
+      "op_stack_table_height": 11,
       "ram_table_height": 0
     },
     "case": "WorstCase"

tasm-lib/src/arithmetic/u32/shift_left.rs

@@ -1,177 +1,136 @@
 use std::collections::HashMap;
 
-use rand::prelude::*;
 use triton_vm::prelude::*;
 
-use crate::empty_stack;
+use crate::arithmetic::u32::shift_right::ShiftRight;
 use crate::prelude::*;
-use crate::traits::deprecated_snippet::DeprecatedSnippet;
-use crate::InitVmState;
-
-#[derive(Clone, Debug)]
+use crate::traits::basic_snippet::Reviewer;
+use crate::traits::basic_snippet::SignOffFingerprint;
+
+/// [Shift left][shl] for unsigned 32-bit integers.
+///
+/// # Behavior
+///
+/// ```text
+/// BEFORE: _ [arg: u32] shift_amount
+/// AFTER:  _ [result: u32]
+/// ```
+///
+/// # Preconditions
+///
+/// - input argument `arg` is properly [`BFieldCodec`] encoded
+/// - input argument `shift_amount` is in `0..32`
+///
+/// # Postconditions
+///
+/// - the output is the input argument `arg` bit-shifted to the left by
+///   input argument `shift_amount`
+/// - the output is properly [`BFieldCodec`] encoded
+///
+/// [shl]: core::ops::Shl
+#[derive(Debug, Default, Copy, Clone, Eq, PartialEq, Hash)]
 pub struct ShiftLeft;
 
-impl DeprecatedSnippet for ShiftLeft {
-    fn entrypoint_name(&self) -> String {
-        "tasmlib_arithmetic_u32_shift_left".to_string()
-    }
-
-    fn input_field_names(&self) -> Vec<String> {
-        vec!["value".to_string(), "shift".to_string()]
-    }
-
-    fn input_types(&self) -> Vec<DataType> {
-        vec![DataType::U32, DataType::U32]
-    }
+impl ShiftLeft {
+    pub const SHIFT_AMOUNT_TOO_BIG_ERROR_ID: i128 = 480;
+}
 
-    fn output_field_names(&self) -> Vec<String> {
-        vec!["value << shift".to_string()]
+impl BasicSnippet for ShiftLeft {
+    fn inputs(&self) -> Vec<(DataType, String)> {
+        ShiftRight.inputs()
     }
 
-    fn output_types(&self) -> Vec<DataType> {
-        vec![DataType::U32]
+    fn outputs(&self) -> Vec<(DataType, String)> {
+        ShiftRight.outputs()
     }
 
-    fn stack_diff(&self) -> isize {
-        -1
+    fn entrypoint(&self) -> String {
+        "tasmlib_arithmetic_u32_shift_left".to_string()
     }
 
-    fn function_code(&self, _library: &mut Library) -> String {
-        let entrypoint = self.entrypoint_name();
-
-        // I'm unsure if we should do a bounds check to check if `shift < 32`
-        format!(
-            "
+    fn code(&self, _: &mut Library) -> Vec<LabelledInstruction> {
+        triton_asm!(
             // BEFORE: _ value shift
             // AFTER:  _ (value << shift)
-            {entrypoint}:
-                // Bounds check. May be superfluous but this mimics Rust's behavior.
+            {self.entrypoint()}:
+                /* bounds check mimics Rust's behavior */
                 push 32
                 dup 1
                 lt
-                assert
+                assert error_id {Self::SHIFT_AMOUNT_TOO_BIG_ERROR_ID}
 
                 push 2
                 pow
                 mul
                 split
-                swap 1
+                pick 1
                 pop 1
 
                 return
-                "
         )
     }
 
-    fn crash_conditions(&self) -> Vec<String> {
-        vec![
-            "inputs are not valid u32s".to_string(),
-            "attempting to left shift with a value greater than 31".to_string(),
-        ]
-    }
-
-    fn gen_input_states(&self) -> Vec<InitVmState> {
-        let mut rng = thread_rng();
-        let mut ret: Vec<InitVmState> = vec![];
-        for _ in 0..100 {
-            let value = rng.next_u32();
-            let shift = rng.gen_range(0..32);
-            ret.push(prepare_state(value, shift));
-        }
-
-        ret
-    }
-
-    fn common_case_input_state(&self) -> InitVmState {
-        prepare_state((1 << 16) - 1, 16)
-    }
-
-    fn worst_case_input_state(&self) -> InitVmState {
-        prepare_state(u32::MAX, 31)
-    }
-
-    fn rust_shadowing(
-        &self,
-        stack: &mut Vec<BFieldElement>,
-        _std_in: Vec<BFieldElement>,
-        _secret_in: Vec<BFieldElement>,
-        _memory: &mut HashMap<BFieldElement, BFieldElement>,
-    ) {
-        // Find shift amount
-        let shift_amount: u32 = stack.pop().unwrap().try_into().unwrap();
-
-        // Original value
-        let value: u32 = stack.pop().unwrap().try_into().unwrap();
-
-        let ret = value << shift_amount;
-        stack.push((ret as u64).into());
+    fn sign_offs(&self) -> HashMap<Reviewer, SignOffFingerprint> {
+        let mut sign_offs = HashMap::new();
+        sign_offs.insert(Reviewer("ferdinand"), 0xec49012951c99eb1.into());
+        sign_offs
     }
 }
 
-fn prepare_state(value: u32, shift: u32) -> InitVmState {
-    let mut stack = empty_stack();
-    let value = BFieldElement::new(value as u64);
-    let shift = BFieldElement::new(shift as u64);
-    stack.push(value);
-    stack.push(shift);
-
-    InitVmState::with_stack(stack)
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::test_helpers::test_rust_equivalence_given_input_values_deprecated;
-    use crate::test_helpers::test_rust_equivalence_multiple_deprecated;
+    use crate::test_prelude::*;
 
-    #[test]
-    fn shift_left_test() {
-        test_rust_equivalence_multiple_deprecated(&ShiftLeft, true);
-    }
+    impl Closure for ShiftLeft {
+        type Args = (u32, u32);
 
-    #[test]
-    fn shift_left_max_value_test() {
-        for i in 0..32 {
-            prop_shift_left(u32::MAX, i);
+        fn rust_shadow(&self, stack: &mut Vec<BFieldElement>) {
+            let (arg, shift_amount) = pop_encodable::<Self::Args>(stack);
+            assert!(shift_amount < 32);
+            push_encodable(stack, &(arg << shift_amount));
+        }
+
+        fn pseudorandom_args(
+            &self,
+            seed: [u8; 32],
+            bench_case: Option<BenchmarkCase>,
+        ) -> Self::Args {
+            let mut rng = StdRng::from_seed(seed);
+            match bench_case {
+                Some(BenchmarkCase::CommonCase) => ((1 << 16) - 1, 16),
+                Some(BenchmarkCase::WorstCase) => (u32::MAX, 31),
+                None => (rng.gen(), rng.gen_range(0..32)),
+            }
+        }
+
+        fn corner_case_args(&self) -> Vec<Self::Args> {
+            (0..32).map(|i| (u32::MAX, i)).collect()
         }
     }
 
     #[test]
-    #[should_panic]
-    fn shift_beyond_limit() {
-        let mut init_stack = empty_stack();
-        init_stack.push(BFieldElement::new(u32::MAX as u64));
-        init_stack.push(32u64.into());
-        ShiftLeft.link_and_run_tasm_from_state_for_test(&mut InitVmState::with_stack(init_stack));
+    fn rust_shadow() {
+        ShadowedClosure::new(ShiftLeft).test();
     }
 
-    fn prop_shift_left(value: u32, shift_amount: u32) {
-        let mut init_stack = empty_stack();
-        init_stack.push(BFieldElement::new(value as u64));
-        init_stack.push(BFieldElement::new(shift_amount as u64));
-
-        let expected_u32 = value << shift_amount;
-
-        let mut expected_stack = empty_stack();
-        expected_stack.push((expected_u32 as u64).into());
-
-        test_rust_equivalence_given_input_values_deprecated(
-            &ShiftLeft,
-            &init_stack,
-            &[],
-            HashMap::default(),
-            Some(&expected_stack),
-        );
+    #[proptest]
+    fn too_big_shift_amount_crashes_vm(arg: u32, #[strategy(32_u32..)] shift_amount: u32) {
+        test_assertion_failure(
+            &ShadowedClosure::new(ShiftLeft),
+            InitVmState::with_stack(ShiftLeft.set_up_test_stack((arg, shift_amount))),
+            &[ShiftLeft::SHIFT_AMOUNT_TOO_BIG_ERROR_ID],
+        )
     }
 }
 
 #[cfg(test)]
 mod benches {
     use super::*;
-    use crate::snippet_bencher::bench_and_write;
+    use crate::test_prelude::*;
 
     #[test]
-    fn shift_left_benchmark() {
-        bench_and_write(ShiftLeft);
+    fn benchmark() {
+        ShadowedClosure::new(ShiftLeft).bench();
     }
 }