feat!(acir): Add RAM and ROM opcodes (#146)

* block id is public

* add ram and rom opcodes

* integration with backend

* Code review

acir/src/circuit/opcodes.rs

@@ -4,27 +4,106 @@ use super::directives::{Directive, LogInfo};
 use crate::native_types::{Expression, Witness};
 use crate::serialization::{read_n, read_u16, read_u32, write_bytes, write_u16, write_u32};
 use crate::BlackBoxFunc;
+use acir_field::FieldElement;
 use serde::{Deserialize, Serialize};
 
 #[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Hash, Copy, Default)]
 pub struct BlockId(pub u32);
 
 /// Operation on a block
 /// We can either write or read at a block index
-#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize, Debug)]
 pub struct MemOp {
+    /// Can be 0 (read) or 1 (write)
     pub operation: Expression,
     pub index: Expression,
     pub value: Expression,
 }
 
+/// Represents operations on a block of length len of data
+#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub struct MemoryBlock {
+    /// Id of the block
+    pub id: BlockId,
+    /// Length of the memory block
+    pub len: u32,
+    /// Trace of memory operations
+    pub trace: Vec<MemOp>,
+}
+
+impl MemoryBlock {
+    pub fn read<R: Read>(mut reader: R) -> std::io::Result<Self> {
+        let id = read_u32(&mut reader)?;
+        let len = read_u32(&mut reader)?;
+        let trace_len = read_u32(&mut reader)?;
+        let mut trace = Vec::with_capacity(len as usize);
+        for _i in 0..trace_len {
+            let operation = Expression::read(&mut reader)?;
+            let index = Expression::read(&mut reader)?;
+            let value = Expression::read(&mut reader)?;
+            trace.push(MemOp { operation, index, value });
+        }
+        Ok(MemoryBlock { id: BlockId(id), len, trace })
+    }
+
+    pub fn write<W: Write>(&self, mut writer: W) -> std::io::Result<()> {
+        write_u32(&mut writer, self.id.0)?;
+        write_u32(&mut writer, self.len)?;
+        write_u32(&mut writer, self.trace.len() as u32)?;
+
+        for op in &self.trace {
+            op.operation.write(&mut writer)?;
+            op.index.write(&mut writer)?;
+            op.value.write(&mut writer)?;
+        }
+        Ok(())
+    }
+
+    /// Returns the initialization vector of the MemoryBlock
+    pub fn init_phase(&self) -> Vec<Expression> {
+        let mut init = Vec::new();
+        for i in 0..self.len as usize {
+            assert_eq!(
+                self.trace[i].operation,
+                Expression::one(),
+                "Block initialization require a write"
+            );
+            let index = self.trace[i]
+                .index
+                .to_const()
+                .expect("Non-const index during Block initialization");
+            if index != FieldElement::from(i as i128) {
+                todo!(
+                    "invalid index when initializing a block, we could try to sort the init phase"
+                );
+            }
+            let value = self.trace[i].value.clone();
+            assert!(value.is_degree_one_univariate(), "Block initialization requires a witness");
+            init.push(value);
+        }
+        init
+    }
+}
+
 #[derive(Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub enum Opcode {
     Arithmetic(Expression),
     BlackBoxFuncCall(BlackBoxFuncCall),
     Directive(Directive),
-    // Abstract read/write operations on a block of data
-    Block(BlockId, Vec<MemOp>),
+    /// Abstract read/write operations on a block of data. In particular;
+    /// It does not require an initialisation phase
+    /// Operations do not need to be constant, they can be any expression which resolves to 0 or 1.
+    Block(MemoryBlock),
+    /// Same as Block, but it starts with an initialisation phase and then have only read operation
+    /// - init: write operations with index from 0..MemoryBlock.len
+    /// - after MemoryBlock.len; all operations are read
+    /// ROM can be more efficiently handled because we do not need to check for the operation value (which is always 0).
+    ROM(MemoryBlock),
+    /// Same as ROM, but can have read or write operations
+    /// - init = write operations with index 0..MemoryBlock.len
+    /// - after MemoryBlock.len, all operations are constant expressions (0 or 1)
+    /// RAM is required for Aztec Backend as dynamic memory implementation in Barrentenberg requires an intialisation phase and can only handle constant values for operations.
+    RAM(MemoryBlock),
 }
 
 impl Opcode {
@@ -35,7 +114,9 @@ impl Opcode {
             Opcode::Arithmetic(_) => "arithmetic",
             Opcode::Directive(directive) => directive.name(),
             Opcode::BlackBoxFuncCall(g) => g.name.name(),
-            Opcode::Block(_, _) => "block",
+            Opcode::Block(_) => "block",
+            Opcode::RAM(_) => "ram",
+            Opcode::ROM(_) => "rom",
         }
     }
 
@@ -46,7 +127,9 @@ impl Opcode {
             Opcode::Arithmetic(_) => 0,
             Opcode::BlackBoxFuncCall(_) => 1,
             Opcode::Directive(_) => 2,
-            Opcode::Block(_, _) => 3,
+            Opcode::Block(_) => 3,
+            Opcode::ROM(_) => 4,
+            Opcode::RAM(_) => 5,
         }
     }
 
@@ -68,16 +151,8 @@ impl Opcode {
             Opcode::Arithmetic(expr) => expr.write(writer),
             Opcode::BlackBoxFuncCall(func_call) => func_call.write(writer),
             Opcode::Directive(directive) => directive.write(writer),
-            Opcode::Block(id, trace) => {
-                write_u32(&mut writer, id.0)?;
-                write_u32(&mut writer, trace.len() as u32)?;
-
-                for op in trace {
-                    op.operation.write(&mut writer)?;
-                    op.index.write(&mut writer)?;
-                    op.value.write(&mut writer)?;
-                }
-                Ok(())
+            Opcode::Block(mem_block) | Opcode::ROM(mem_block) | Opcode::RAM(mem_block) => {
+                mem_block.write(writer)
             }
         }
     }
@@ -101,16 +176,16 @@ impl Opcode {
                 Ok(Opcode::Directive(directive))
             }
             3 => {
-                let id = read_u32(&mut reader)?;
-                let len = read_u32(&mut reader)?;
-                let mut trace = Vec::with_capacity(len as usize);
-                for _i in 0..len {
-                    let operation = Expression::read(&mut reader)?;
-                    let index = Expression::read(&mut reader)?;
-                    let value = Expression::read(&mut reader)?;
-                    trace.push(MemOp { operation, index, value });
-                }
-                Ok(Opcode::Block(BlockId(id), trace))
+                let block = MemoryBlock::read(reader)?;
+                Ok(Opcode::Block(block))
+            }
+            4 => {
+                let block = MemoryBlock::read(reader)?;
+                Ok(Opcode::ROM(block))
+            }
+            5 => {
+                let block = MemoryBlock::read(reader)?;
+                Ok(Opcode::RAM(block))
             }
             _ => Err(std::io::ErrorKind::InvalidData.into()),
         }
@@ -186,9 +261,17 @@ impl std::fmt::Display for Opcode {
                     witnesses.last().unwrap().witness_index()
                 ),
             },
-            Opcode::Block(id, trace) => {
+            Opcode::Block(block) => {
                 write!(f, "BLOCK ")?;
-                write!(f, "(id: {}, len: {}) ", id.0, trace.len())
+                write!(f, "(id: {}, len: {}) ", block.id.0, block.trace.len())
+            }
+            Opcode::ROM(block) => {
+                write!(f, "ROM ")?;
+                write!(f, "(id: {}, len: {}) ", block.id.0, block.trace.len())
+            }
+            Opcode::RAM(block) => {
+                write!(f, "RAM ")?;
+                write!(f, "(id: {}, len: {}) ", block.id.0, block.trace.len())
             }
         }
     }

acvm/src/compiler.rs

@@ -11,7 +11,7 @@ use acir::{
 use indexmap::IndexMap;
 use optimizers::GeneralOptimizer;
 use thiserror::Error;
-use transformers::{CSatTransformer, FallbackTransformer, IsBlackBoxSupported, R1CSTransformer};
+use transformers::{CSatTransformer, FallbackTransformer, IsOpcodeSupported, R1CSTransformer};
 
 #[derive(PartialEq, Eq, Debug, Error)]
 pub enum CompileError {
@@ -22,14 +22,14 @@ pub enum CompileError {
 pub fn compile(
     acir: Circuit,
     np_language: Language,
-    is_black_box_supported: IsBlackBoxSupported,
+    is_opcode_supported: IsOpcodeSupported,
 ) -> Result<Circuit, CompileError> {
     // Instantiate the optimizer.
     // Currently the optimizer and reducer are one in the same
     // for CSAT
 
     // Fallback transformer pass
-    let acir = FallbackTransformer::transform(acir, is_black_box_supported)?;
+    let acir = FallbackTransformer::transform(acir, is_opcode_supported)?;
 
     // General optimizer pass
     let mut opcodes: Vec<Opcode> = Vec::new();

acvm/src/compiler/transformers/fallback.rs

@@ -6,47 +6,50 @@ use acir::{
 };
 
 // A predicate that returns true if the black box function is supported
-pub type IsBlackBoxSupported = fn(&BlackBoxFunc) -> bool;
+pub type IsOpcodeSupported = fn(&Opcode) -> bool;
 
 pub struct FallbackTransformer;
 
 impl FallbackTransformer {
     //ACIR pass which replace unsupported opcodes using arithmetic fallback
     pub fn transform(
         acir: Circuit,
-        is_supported: IsBlackBoxSupported,
+        is_supported: IsOpcodeSupported,
     ) -> Result<Circuit, CompileError> {
         let mut acir_supported_opcodes = Vec::with_capacity(acir.opcodes.len());
 
         let mut witness_idx = acir.current_witness_index + 1;
 
         for opcode in acir.opcodes {
-            let bb_func_call = match &opcode {
-                Opcode::Arithmetic(_) | Opcode::Directive(_) | Opcode::Block(_, _) => {
-                    // directive, arithmetic expression or block are handled by acvm
+            match &opcode {
+                Opcode::Arithmetic(_)
+                | Opcode::Directive(_)
+                | Opcode::Block(_)
+                | Opcode::ROM(_)
+                | Opcode::RAM(_) => {
+                    // directive, arithmetic expression or blocks are handled by acvm
                     acir_supported_opcodes.push(opcode);
                     continue;
                 }
                 Opcode::BlackBoxFuncCall(bb_func_call) => {
                     // We know it is an black box function. Now check if it is
                     // supported by the backend. If it is supported, then we can simply
                     // collect the opcode
-                    if is_supported(&bb_func_call.name) {
+                    if is_supported(&opcode) {
                         acir_supported_opcodes.push(opcode);
                         continue;
+                    } else {
+                        // If we get here then we know that this black box function is not supported
+                        // so we need to replace it with a version of the opcode which only uses arithmetic
+                        // expressions
+                        let (updated_witness_index, opcodes_fallback) =
+                            Self::opcode_fallback(bb_func_call, witness_idx)?;
+                        witness_idx = updated_witness_index;
+
+                        acir_supported_opcodes.extend(opcodes_fallback);
                     }
-                    bb_func_call
                 }
-            };
-
-            // If we get here then we know that this black box function is not supported
-            // so we need to replace it with a version of the opcode which only uses arithmetic
-            // expressions
-            let (updated_witness_index, opcodes_fallback) =
-                Self::opcode_fallback(bb_func_call, witness_idx)?;
-            witness_idx = updated_witness_index;
-
-            acir_supported_opcodes.extend(opcodes_fallback);
+            }
         }
 
         Ok(Circuit {

acvm/src/compiler/transformers/mod.rs

@@ -4,5 +4,5 @@ mod r1cs;
 
 pub use csat::CSatTransformer;
 pub use fallback::FallbackTransformer;
-pub use fallback::IsBlackBoxSupported;
+pub use fallback::IsOpcodeSupported;
 pub use r1cs::R1CSTransformer;

acvm/src/lib.rs

@@ -87,7 +87,9 @@ pub trait PartialWitnessGenerator {
                     Opcode::Directive(directive) => {
                         Self::solve_directives(initial_witness, directive)
                     }
-                    Opcode::Block(id, trace) => blocks.solve(*id, trace, initial_witness),
+                    Opcode::Block(block) | Opcode::ROM(block) | Opcode::RAM(block) => {
+                        blocks.solve(block.id, &block.trace, initial_witness)
+                    }
                 };
                 match resolution {
                     Ok(OpcodeResolution::Solved) => {
@@ -242,23 +244,27 @@ pub fn checksum_constraint_system(cs: &Circuit) -> u32 {
     note = "For backwards compatibility, this method allows you to derive _sensible_ defaults for black box function support based on the np language. \n Backends should simply specify what they support."
 )]
 // This is set to match the previous functionality that we had
-// Where we could deduce what black box functions were supported
+// Where we could deduce what opcodes were supported
 // by knowing the np complete language
-pub fn default_is_black_box_supported(
+pub fn default_is_opcode_supported(
     language: Language,
-) -> compiler::transformers::IsBlackBoxSupported {
-    // R1CS does not support any of the black box functions by default.
+) -> compiler::transformers::IsOpcodeSupported {
+    // R1CS does not support any of the opcode except Arithmetic by default.
     // The compiler will replace those that it can -- ie range, xor, and
-    fn r1cs_is_supported(_opcode: &BlackBoxFunc) -> bool {
-        false
+    fn r1cs_is_supported(opcode: &Opcode) -> bool {
+        matches!(opcode, Opcode::Arithmetic(_))
     }
 
-    // PLONK supports most of the black box functions by default
+    // PLONK supports most of the opcodes by default
     // The ones which are not supported, the acvm compiler will
     // attempt to transform into supported gates. If these are also not available
     // then a compiler error will be emitted.
-    fn plonk_is_supported(opcode: &BlackBoxFunc) -> bool {
-        !matches!(opcode, BlackBoxFunc::AES)
+    fn plonk_is_supported(opcode: &Opcode) -> bool {
+        !matches!(
+            opcode,
+            Opcode::BlackBoxFuncCall(BlackBoxFuncCall { name: BlackBoxFunc::AES, .. })
+                | Opcode::Block(_)
+        )
     }
 
     match language {

acvm/src/pwg/block.rs

@@ -41,17 +41,8 @@ struct BlockSolver {
 }
 
 impl BlockSolver {
-    fn insert_value(
-        &mut self,
-        index: u32,
-        value: FieldElement,
-    ) -> Result<(), OpcodeResolutionError> {
-        match self.block_value.insert(index, value) {
-            Some(existing_value) if value != existing_value => {
-                Err(OpcodeResolutionError::UnsatisfiedConstrain)
-            }
-            _ => Ok(()),
-        }
+    fn insert_value(&mut self, index: u32, value: FieldElement) {
+        self.block_value.insert(index, value);
     }
 
     fn get_value(&self, index: u32) -> Option<FieldElement> {
@@ -85,7 +76,7 @@ impl BlockSolver {
             let value = ArithmeticSolver::evaluate(&block_op.value, initial_witness);
             let value_witness = ArithmeticSolver::any_witness_from_expression(&value);
             if value.is_const() {
-                self.insert_value(index, value.q_c)?;
+                self.insert_value(index, value.q_c);
             } else if operation.is_zero() && value.is_linear() {
                 match ArithmeticSolver::solve_fan_in_term(&value, initial_witness) {
                     GateStatus::GateUnsolvable => return Err(missing_assignment(value_witness)),
@@ -95,7 +86,7 @@ impl BlockSolver {
                         insert_witness(w, (map_value - sum - value.q_c) / coef, initial_witness)?;
                     }
                     GateStatus::GateSatisfied(sum) => {
-                        self.insert_value(index, sum + value.q_c)?;
+                        self.insert_value(index, sum + value.q_c);
                     }
                 }
             } else {