feat: Introduce snippet sign-off functionality

tasm-lib/src/test_helpers.rs

@@ -11,6 +11,7 @@ use crate::execute_with_terminal_state;
 use crate::exported_snippets;
 use crate::library::Library;
 use crate::traits::basic_snippet::BasicSnippet;
+use crate::traits::basic_snippet::SignedOffSnippet;
 use crate::traits::deprecated_snippet::DeprecatedSnippet;
 use crate::traits::rust_shadow::RustShadow;
 use crate::InitVmState;
@@ -414,6 +415,11 @@ pub fn test_rust_equivalence_given_complete_state<T: RustShadow>(
     sponge: &Option<VmHasher>,
     expected_final_stack: Option<&[BFieldElement]>,
 ) -> VMState {
+    shadowed_snippet
+        .inner()
+        .borrow()
+        .assert_all_sign_offs_are_up_to_date();
+
     let init_stack = stack.to_vec();
 
     let rust = rust_final_state(shadowed_snippet, stack, stdin, nondeterminism, sponge);

tasm-lib/src/traits/basic_snippet.rs

@@ -1,10 +1,22 @@
+use std::collections::HashMap;
+use std::fmt::Display;
+use std::fmt::Formatter;
+use std::hash::Hash;
+use std::hash::Hasher;
+
 use num_traits::Zero;
+use triton_vm::isa::instruction::AnInstruction;
 use triton_vm::isa::op_stack::NUM_OP_STACK_REGISTERS;
 use triton_vm::prelude::*;
 
 use crate::data_type::DataType;
 use crate::library::Library;
 
+/// ### Dyn-Compatibility
+///
+/// This trait is [dyn-compatible] (previously known as “object safe”).
+///
+/// [dyn-compatible]: https://doc.rust-lang.org/reference/items/traits.html#object-safety
 pub trait BasicSnippet {
     fn inputs(&self) -> Vec<(DataType, String)>;
     fn outputs(&self) -> Vec<(DataType, String)>;
@@ -122,35 +134,199 @@ pub trait BasicSnippet {
         }
         diff
     }
+
+    /// Contains an entry for every sign off.
+    ///
+    /// Many of the snippets defined in this TASM library are critical for the
+    /// consensus logic of the blockchain [Neptune Cash](https://neptune.cash).
+    /// Therefore, it is paramount that the snippets are free of errors. In order
+    /// to catch as many errors as possible, the snippets are reviewed by as many
+    /// developers as possible. The requirements of such a review are listed here.
+    ///
+    /// A reviewer can (and should) sign off on any snippet they have reviewed and
+    /// for which they found no defects. This is done by adding that snippet's
+    /// [fingerprint] (at the time) to the overriding implementation of this method
+    /// on that snippet.
+    ///
+    /// Together with the tools [`git blame`][blame] and cryptographic
+    /// [signing] of commits, this makes sign-offs traceable. It also guarantees
+    /// that changes to snippets that have already been signed-off are easy to
+    /// detect.
+    ///
+    /// # For Reviewers
+    ///
+    /// ## Modifying snippets
+    ///
+    /// While the primary intention of the review process is to _review_ a snippet,
+    /// there are circumstances under which modifying it is acceptable.
+    ///
+    /// Modifying a snippet to simplify reviewing that snippet is fair game. A
+    /// common example of this case is replacing a `swap`-juggle chain with a few
+    /// `pick`s & `place`s.
+    ///
+    /// Modifying a snippet in order to improve performance should only happen if
+    /// the performance impact is meaningful. The currently agreed-upon threshold
+    /// is 0.5% of at least one consensus program.
+    ///
+    /// It is acceptable, and can be desired, to modify a snippet by including
+    /// assumption checks. For example, if the snippet's pre-conditions require
+    /// some input to fall within a certain range, it is fine to add a corresponding
+    /// range check to the snippet.
+    /// Removing existing checks of such nature is considered bad practice.
+    ///
+    /// In either case, modifying a snippet that has already been reviewed and
+    /// signed off by someone else in a way that alters its [fingerprint] requires
+    /// their consent.
+    ///
+    /// ## Checklist
+    ///
+    /// Use the following checklist to guide your review. Signing off on a snippet
+    /// means that in your eyes, all points on this checklist are true.
+    ///
+    /// - the snippet's documentation lists pre- and post-conditions
+    /// - the snippet makes no assumptions outside the stated pre-conditions
+    /// - given all pre-conditions, all post-conditions are met
+    /// - whenever this snippet calls another snippet, all of that other snippet's
+    ///   pre-conditions are met
+    /// - all dynamic memory offsets are range-checked before they are used
+    /// - each field accessor is used at most once per struct instance, or
+    ///   range-checked before each use
+    /// - reading from non-deterministically initialized memory only happens from
+    ///   the region specified in the [memory convention]
+    /// - memory-writes only happen outside of page 0 (see [memory convention])
+    ///
+    /// ## Non-Unit Structs
+    ///
+    /// Most, but not all types implementing [BasicSnippet] are unit structs.
+    /// [Fingerprinting][fingerprint] gets more difficult for non-unit structs.
+    /// In such cases, a default instantiation should be selected and signed off.
+    ///
+    /// ## Overriding this Method
+    ///
+    /// This default implementation _is_ intended to be overridden for any snippet
+    /// that has been signed off, but _should not_ call the [fingerprint] method.
+    ///
+    /// [fingerprint]: SignedOffSnippet::fingerprint
+    /// [memory convention]: crate::memory
+    /// [blame]: https://git-scm.com/docs/git-blame
+    /// [signing]: https://git-scm.com/book/en/v2/Git-Tools-Signing-Your-Work
+    fn sign_offs(&self) -> HashMap<Reviewer, SignOffFingerprint> {
+        HashMap::default()
+    }
+}
+
+/// Extension trait for [`BasicSnippet`] related to
+/// [signing off](BasicSnippet::sign_offs). Contains methods that are callable,
+/// but for which the provided default implementation cannot be overridden.
+///
+/// ### Dyn-Compatibility
+///
+/// This trait is [dyn-compatible] (previously known as “object safe”).
+///
+/// [dyn-compatible]: https://doc.rust-lang.org/reference/items/traits.html#object-safety
+//
+// Because `$[final]` trait methods are in pre-RFC phase [0], and trait
+// sealing [1] would be clumsy, use this workaround.
+//
+// [0]: https://internals.rust-lang.org/t/pre-rfc-final-trait-methods/18407
+// [1]: https://predr.ag/blog/definitive-guide-to-sealed-traits-in-rust
+pub trait SignedOffSnippet: BasicSnippet {
+    /// The unique fingerprint as used for [signing off][BasicSnippet::sign_offs] on
+    /// this snippet.
+    fn fingerprint(&self) -> SignOffFingerprint {
+        let mut hasher = std::hash::DefaultHasher::new();
+        triton_vm::proof::CURRENT_VERSION.hash(&mut hasher);
+
+        for instruction in self.code(&mut Library::new()) {
+            let LabelledInstruction::Instruction(instruction) = instruction else {
+                continue;
+            };
+
+            if let AnInstruction::Call(_) = instruction {
+                AnInstruction::Call("").opcode().hash(&mut hasher);
+            } else {
+                instruction.hash(&mut hasher)
+            }
+        }
+
+        SignOffFingerprint(hasher.finish())
+    }
+
+    /// Panics if any [sign-offs](BasicSnippet::sign_offs) disagree with the actual
+    /// [fingerprint](Self::fingerprint).
+    fn assert_all_sign_offs_are_up_to_date(&self) {
+        let fingerprint = self.fingerprint();
+        let mut out_of_date_sign_offs = self
+            .sign_offs()
+            .into_iter()
+            .filter(|(_, fp)| fp != &fingerprint)
+            .peekable();
+
+        if out_of_date_sign_offs.peek().is_none() {
+            return;
+        }
+
+        let name = self.entrypoint();
+        for (reviewer, fp) in out_of_date_sign_offs {
+            eprintln!("reviewer {reviewer} of snippet “{name}” has signed off on fingerprint {fp}")
+        }
+        panic!("A sign-off is out of date. Current fingerprint of “{name}”: {fingerprint}");
+    }
+}
+
+// Blanket implementation conflicts with any other implementation, making the
+// provided defaults final.
+impl<T: BasicSnippet + ?Sized> SignedOffSnippet for T {}
+
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
+pub struct Reviewer(pub &'static str);
+
+impl Display for Reviewer {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.0)
+    }
+}
+
+/// A fingerprint as used for [signing off][BasicSnippet::sign_offs] snippets.
+///
+/// While this fingerprint can be used to distinguish [`BasicSnippets`], it is
+/// not cryptographically secure.
+#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
+pub struct SignOffFingerprint(pub(crate) u64);
+
+impl Display for SignOffFingerprint {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{:x}", self.0)
+    }
+}
+
+impl From<u64> for SignOffFingerprint {
+    fn from(value: u64) -> Self {
+        Self(value)
+    }
 }
 
 #[cfg(test)]
 mod tests {
-    use triton_vm::prelude::triton_asm;
-    use triton_vm::prelude::Program;
-
-    use crate::traits::basic_snippet::BasicSnippet;
+    use super::*;
 
     #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)]
     struct DummySnippet;
 
     impl BasicSnippet for DummySnippet {
-        fn inputs(&self) -> Vec<(crate::data_type::DataType, String)> {
+        fn inputs(&self) -> Vec<(DataType, String)> {
             vec![]
         }
 
-        fn outputs(&self) -> Vec<(crate::data_type::DataType, String)> {
+        fn outputs(&self) -> Vec<(DataType, String)> {
             vec![]
         }
 
         fn entrypoint(&self) -> String {
             "____dummy_snippet_test".to_string()
         }
 
-        fn code(
-            &self,
-            _library: &mut crate::library::Library,
-        ) -> Vec<triton_vm::prelude::LabelledInstruction> {
+        fn code(&self, _library: &mut Library) -> Vec<LabelledInstruction> {
             triton_asm!(
                 {self.entrypoint()}:
                     push 14
@@ -168,9 +344,56 @@ mod tests {
         let calculated_init_stack = DummySnippet.init_stack_for_isolated_run();
         let program = DummySnippet.link_for_isolated_run();
         let program = Program::new(&program);
-        let init_vm_state =
-            triton_vm::vm::VMState::new(program, Default::default(), Default::default());
+        let init_vm_state = VMState::new(program, Default::default(), Default::default());
 
         assert_eq!(init_vm_state.op_stack.stack, calculated_init_stack);
     }
+
+    #[test]
+    fn defined_traits_are_dyn_compatible() {
+        fn basic_snippet_is_dyn_compatible(snippet: Box<dyn BasicSnippet>) {
+            snippet.fingerprint();
+        }
+
+        fn signed_off_snippet_is_dyn_compatible(snippet: Box<dyn SignedOffSnippet>) {
+            snippet.fingerprint();
+        }
+
+        basic_snippet_is_dyn_compatible(Box::new(DummySnippet));
+        signed_off_snippet_is_dyn_compatible(Box::new(DummySnippet));
+    }
+
+    macro_rules! dummy_snippet {
+        ($name:ident: $($instr:tt)+) => {
+            struct $name;
+
+            impl BasicSnippet for $name {
+                fn inputs(&self) -> Vec<(DataType, String)> { vec![] }
+                fn outputs(&self) -> Vec<(DataType, String)> { vec![] }
+                fn entrypoint(&self) -> String {
+                    stringify!($name).to_ascii_lowercase()
+                }
+
+                fn code(&self, _: &mut Library) -> Vec<LabelledInstruction> {
+                    triton_asm!($($instr)+)
+                }
+            }
+        };
+    }
+
+    #[test]
+    fn snippets_differing_only_in_call_targets_have_identical_fingerprints() {
+        dummy_snippet!(SomeLabel: call some_label);
+        dummy_snippet!(OtherLabel: call other_label);
+
+        assert_eq!(SomeLabel.fingerprint(), OtherLabel.fingerprint());
+    }
+
+    #[test]
+    fn snippets_differing_only_in_instruction_args_have_different_fingerprints() {
+        dummy_snippet!(Push20: push 20);
+        dummy_snippet!(Push42: push 42);
+
+        assert_ne!(Push20.fingerprint(), Push42.fingerprint());
+    }
 }