feat!: remove sha256 opcode (#4571)

This PR resolves Noir issue 4330: noir-lang/noir#4330 by removing the sha256 opcode and replacing the sha256 function in the stdlib by the implementation using the sha256 compression opcode (also in the stdlib). --------- Co-authored-by: kevaundray <kevtheappdev@gmail.com> Co-authored-by: Tom French <tom@tomfren.ch> Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> Co-authored-by: dbanks12 <david@aztecprotocol.com> Co-authored-by: David Banks <47112877+dbanks12@users.noreply.github.com> Co-authored-by: fcarreiro <facundo@aztecprotocol.com>
AztecProtocol · Sep 23, 2024 · 4b4a0bf · 4b4a0bf
1 parent 07d43ea
commit 4b4a0bf
Show file tree

Hide file tree

Showing 72 changed files with 500 additions and 979 deletions.
diff --git a/avm-transpiler/src/instructions.rs b/avm-transpiler/src/instructions.rs
@@ -10,6 +10,7 @@ pub const ALL_DIRECT: u8 = 0b00000000;
 pub const ZEROTH_OPERAND_INDIRECT: u8 = 0b00000001;
 pub const FIRST_OPERAND_INDIRECT: u8 = 0b00000010;
 pub const SECOND_OPERAND_INDIRECT: u8 = 0b00000100;
+pub const THIRD_OPERAND_INDIRECT: u8 = 0b00001000;
 
 /// A simple representation of an AVM instruction for the purpose
 /// of generating an AVM bytecode from Brillig.

diff --git a/avm-transpiler/src/opcodes.rs b/avm-transpiler/src/opcodes.rs
@@ -88,16 +88,14 @@ pub enum AvmOpcode {
     // Gadgets
     KECCAK,
     POSEIDON2,
-    SHA256,   // temp - may be removed, but alot of contracts rely on it
+    SHA256COMPRESSION,
+    KECCAKF1600,
     PEDERSEN, // temp - may be removed, but alot of contracts rely on it
     ECADD,
     MSM,
     PEDERSENCOMMITMENT, // temp
     // Conversions
     TORADIXLE,
-    // Other
-    SHA256COMPRESSION,
-    KECCAKF1600,
 }
 
 impl AvmOpcode {
@@ -202,17 +200,15 @@ impl AvmOpcode {
 
             // Gadgets
             AvmOpcode::KECCAK => "KECCAK",
+            AvmOpcode::KECCAKF1600 => "KECCAKF1600",
             AvmOpcode::POSEIDON2 => "POSEIDON2",
-            AvmOpcode::SHA256 => "SHA256 ",
+            AvmOpcode::SHA256COMPRESSION => "SHA256COMPRESSION",
             AvmOpcode::PEDERSEN => "PEDERSEN",
             AvmOpcode::ECADD => "ECADD",
             AvmOpcode::MSM => "MSM",
             AvmOpcode::PEDERSENCOMMITMENT => "PEDERSENCOMMITMENT",
             // Conversions
             AvmOpcode::TORADIXLE => "TORADIXLE",
-            // Other
-            AvmOpcode::SHA256COMPRESSION => "SHA256COMPRESSION",
-            AvmOpcode::KECCAKF1600 => "KECCAKF1600",
         }
     }
 }
diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs
@@ -12,7 +12,7 @@ use noirc_errors::debug_info::DebugInfo;
 use crate::bit_traits::bits_needed_for;
 use crate::instructions::{
     AvmInstruction, AvmOperand, AvmTypeTag, ALL_DIRECT, FIRST_OPERAND_INDIRECT,
-    SECOND_OPERAND_INDIRECT, ZEROTH_OPERAND_INDIRECT,
+    SECOND_OPERAND_INDIRECT, THIRD_OPERAND_INDIRECT, ZEROTH_OPERAND_INDIRECT,
 };
 use crate::opcodes::AvmOpcode;
 use crate::utils::{dbg_print_avm_program, dbg_print_brillig_program, make_operand};
@@ -865,19 +865,24 @@ fn generate_mov_instruction(indirect: Option<u8>, source: u32, dest: u32) -> Avm
 /// (array goes in -> field element comes out)
 fn handle_black_box_function(avm_instrs: &mut Vec<AvmInstruction>, operation: &BlackBoxOp) {
     match operation {
-        BlackBoxOp::Sha256 { message, output } => {
-            let message_offset = message.pointer.0;
-            let message_size_offset = message.size.0;
-            let dest_offset = output.pointer.0;
-            assert_eq!(output.size, 32, "SHA256 output size must be 32!");
+        BlackBoxOp::Sha256Compression { input, hash_values, output } => {
+            let inputs_offset = input.pointer.0;
+            let inputs_size_offset = input.size.0;
+            let state_offset = hash_values.pointer.0;
+            let state_size_offset = hash_values.size.0;
+            let output_offset = output.pointer.0;
 
             avm_instrs.push(AvmInstruction {
-                opcode: AvmOpcode::SHA256,
-                indirect: Some(ZEROTH_OPERAND_INDIRECT | FIRST_OPERAND_INDIRECT),
+                opcode: AvmOpcode::SHA256COMPRESSION,
+                indirect: Some(
+                    ZEROTH_OPERAND_INDIRECT | FIRST_OPERAND_INDIRECT | THIRD_OPERAND_INDIRECT,
+                ),
                 operands: vec![
-                    AvmOperand::U32 { value: dest_offset as u32 },
-                    AvmOperand::U32 { value: message_offset as u32 },
-                    AvmOperand::U32 { value: message_size_offset as u32 },
+                    AvmOperand::U32 { value: output_offset as u32 },
+                    AvmOperand::U32 { value: state_offset as u32 },
+                    AvmOperand::U32 { value: state_size_offset as u32 },
+                    AvmOperand::U32 { value: inputs_offset as u32 },
+                    AvmOperand::U32 { value: inputs_size_offset as u32 },
                 ],
                 ..Default::default()
             });

diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.cpp
@@ -74,13 +74,6 @@ void build_constraints(Builder& builder,
     }
 
     // Add sha256 constraints
-    for (size_t i = 0; i < constraint_system.sha256_constraints.size(); ++i) {
-        const auto& constraint = constraint_system.sha256_constraints.at(i);
-        create_sha256_constraints(builder, constraint);
-        gate_counter.track_diff(constraint_system.gates_per_opcode,
-                                constraint_system.original_opcode_indices.sha256_constraints.at(i));
-    }
-
     for (size_t i = 0; i < constraint_system.sha256_compression.size(); ++i) {
         const auto& constraint = constraint_system.sha256_compression[i];
         create_sha256_compression_constraints(builder, constraint);

diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.hpp
@@ -41,7 +41,6 @@ struct AcirFormatOriginalOpcodeIndices {
     std::vector<size_t> logic_constraints;
     std::vector<size_t> range_constraints;
     std::vector<size_t> aes128_constraints;
-    std::vector<size_t> sha256_constraints;
     std::vector<size_t> sha256_compression;
     std::vector<size_t> schnorr_constraints;
     std::vector<size_t> ecdsa_k1_constraints;
@@ -90,7 +89,6 @@ struct AcirFormat {
     std::vector<LogicConstraint> logic_constraints;
     std::vector<RangeConstraint> range_constraints;
     std::vector<AES128Constraint> aes128_constraints;
-    std::vector<Sha256Constraint> sha256_constraints;
     std::vector<Sha256Compression> sha256_compression;
     std::vector<SchnorrConstraint> schnorr_constraints;
     std::vector<EcdsaSecp256k1Constraint> ecdsa_k1_constraints;
@@ -137,7 +135,6 @@ struct AcirFormat {
                    logic_constraints,
                    range_constraints,
                    aes128_constraints,
-                   sha256_constraints,
                    sha256_compression,
                    schnorr_constraints,
                    ecdsa_k1_constraints,

diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp
@@ -45,7 +45,6 @@ TEST_F(AcirFormatTests, TestASingleConstraintNoPubInputs)
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = {},
@@ -169,7 +168,6 @@ TEST_F(AcirFormatTests, TestLogicGateFromNoirCircuit)
         .logic_constraints = { logic_constraint },
         .range_constraints = { range_a, range_b },
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = {},
@@ -243,6 +241,7 @@ TEST_F(AcirFormatTests, TestSchnorrVerifyPass)
         .result = 76,
         .signature = signature,
     };
+
     AcirFormat constraint_system{
         .varnum = 81,
         .recursive = false,
@@ -251,7 +250,6 @@ TEST_F(AcirFormatTests, TestSchnorrVerifyPass)
         .logic_constraints = {},
         .range_constraints = range_constraints,
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = { schnorr_constraint },
         .ecdsa_k1_constraints = {},
@@ -360,7 +358,6 @@ TEST_F(AcirFormatTests, TestSchnorrVerifySmallRange)
         .logic_constraints = {},
         .range_constraints = range_constraints,
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = { schnorr_constraint },
         .ecdsa_k1_constraints = {},
@@ -482,7 +479,6 @@ TEST_F(AcirFormatTests, TestVarKeccak)
         .logic_constraints = {},
         .range_constraints = { range_a, range_b, range_c, range_d },
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = {},
@@ -564,7 +560,6 @@ TEST_F(AcirFormatTests, TestKeccakPermutation)
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = {},
@@ -598,11 +593,9 @@ TEST_F(AcirFormatTests, TestKeccakPermutation)
                            35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50 };
 
     auto builder = create_circuit(constraint_system, /*size_hint=*/0, witness);
-
     auto composer = Composer();
     auto prover = composer.create_ultra_with_keccak_prover(builder);
     auto proof = prover.construct_proof();
-
     auto verifier = composer.create_ultra_with_keccak_verifier(builder);
 
     EXPECT_EQ(verifier.verify_proof(proof), true);
@@ -643,7 +636,6 @@ TEST_F(AcirFormatTests, TestCollectsGateCounts)
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = {},
@@ -676,4 +668,4 @@ TEST_F(AcirFormatTests, TestCollectsGateCounts)
         create_circuit(constraint_system, /*size_hint*/ 0, witness, false, std::make_shared<bb::ECCOpQueue>(), true);
 
     EXPECT_EQ(constraint_system.gates_per_opcode, std::vector<size_t>({ 2, 1 }));
-}
+}
diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format_mocks.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_format_mocks.cpp
@@ -6,7 +6,6 @@ acir_format::AcirFormatOriginalOpcodeIndices create_empty_original_opcode_indice
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = {},
@@ -46,9 +45,6 @@ void mock_opcode_indices(acir_format::AcirFormat& constraint_system)
     for (size_t i = 0; i < constraint_system.aes128_constraints.size(); i++) {
         constraint_system.original_opcode_indices.aes128_constraints.push_back(current_opcode++);
     }
-    for (size_t i = 0; i < constraint_system.sha256_constraints.size(); i++) {
-        constraint_system.original_opcode_indices.sha256_constraints.push_back(current_opcode++);
-    }
     for (size_t i = 0; i < constraint_system.sha256_compression.size(); i++) {
         constraint_system.original_opcode_indices.sha256_compression.push_back(current_opcode++);
     }
@@ -127,4 +123,4 @@ void mock_opcode_indices(acir_format::AcirFormat& constraint_system)
     }
 
     constraint_system.num_acir_opcodes = static_cast<uint32_t>(current_opcode);
-}
+}
diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.cpp
@@ -354,21 +354,6 @@ void handle_blackbox_func_call(Program::Opcode::BlackBoxFuncCall const& arg,
                     af.constrained_witness.insert(output);
                 }
                 af.original_opcode_indices.aes128_constraints.push_back(opcode_index);
-
-            } else if constexpr (std::is_same_v<T, Program::BlackBoxFuncCall::SHA256>) {
-                af.sha256_constraints.push_back(Sha256Constraint{
-                    .inputs = map(arg.inputs,
-                                  [](auto& e) {
-                                      auto input_witness = get_witness_from_function_input(e);
-                                      return Sha256Input{
-                                          .witness = input_witness,
-                                          .num_bits = e.num_bits,
-                                      };
-                                  }),
-                    .result = map(arg.outputs, [](auto& e) { return e.value; }),
-                });
-                af.original_opcode_indices.sha256_constraints.push_back(opcode_index);
-
             } else if constexpr (std::is_same_v<T, Program::BlackBoxFuncCall::Sha256Compression>) {
                 af.sha256_compression.push_back(Sha256Compression{
                     .inputs = map(arg.inputs, [](auto& e) { return parse_input(e); }),
@@ -823,4 +808,4 @@ AcirProgramStack get_acir_program_stack(std::string const& bytecode_path,
     return { constraint_systems, witness_stack };
 }
 #endif
-} // namespace acir_format
+} // namespace acir_format
diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/bigint_constraint.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/bigint_constraint.test.cpp
@@ -178,7 +178,6 @@ TEST_F(BigIntTests, TestBigIntConstraintMultiple)
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = {},
@@ -254,7 +253,6 @@ TEST_F(BigIntTests, TestBigIntConstraintSimple)
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = {},
@@ -314,7 +312,6 @@ TEST_F(BigIntTests, TestBigIntConstraintReuse)
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = {},
@@ -379,7 +376,6 @@ TEST_F(BigIntTests, TestBigIntConstraintReuse2)
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = {},
@@ -465,7 +461,6 @@ TEST_F(BigIntTests, TestBigIntDIV)
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = {},

diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/block_constraint.test.cpp
@@ -146,7 +146,6 @@ TEST_F(UltraPlonkRAM, TestBlockConstraint)
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = {},
@@ -200,7 +199,6 @@ TEST_F(MegaHonk, Databus)
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = {},
@@ -309,7 +307,6 @@ TEST_F(MegaHonk, DatabusReturn)
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = {},

diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.test.cpp
@@ -68,7 +68,6 @@ TEST_F(EcOperations, TestECOperations)
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = {},
@@ -207,7 +206,6 @@ TEST_F(EcOperations, TestECMultiScalarMul)
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = {},

diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256k1.test.cpp
@@ -100,7 +100,6 @@ TEST_F(ECDSASecp256k1, TestECDSAConstraintSucceed)
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = { ecdsa_k1_constraint },
@@ -157,7 +156,6 @@ TEST_F(ECDSASecp256k1, TestECDSACompilesForVerifier)
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = { ecdsa_k1_constraint },
@@ -209,7 +207,6 @@ TEST_F(ECDSASecp256k1, TestECDSAConstraintFail)
         .logic_constraints = {},
         .range_constraints = {},
         .aes128_constraints = {},
-        .sha256_constraints = {},
         .sha256_compression = {},
         .schnorr_constraints = {},
         .ecdsa_k1_constraints = { ecdsa_k1_constraint },