Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore!: remove keccak256 opcode from ACIR/Brillig #9104

Merged
merged 11 commits into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions avm-transpiler/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 0 additions & 19 deletions avm-transpiler/src/transpile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -941,25 +941,6 @@ fn handle_black_box_function(avm_instrs: &mut Vec<AvmInstruction>, operation: &B
..Default::default()
});
}
BlackBoxOp::Keccak256 { 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, "Keccak256 output size must be 32!");

avm_instrs.push(AvmInstruction {
opcode: AvmOpcode::KECCAK,
indirect: Some(AvmOperand::U8 {
value: ZEROTH_OPERAND_INDIRECT | FIRST_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 },
],
..Default::default()
});
}
BlackBoxOp::Keccakf1600 { message, output } => {
let message_offset = message.pointer.0;
let message_size_offset = message.size.0;
Expand Down
1 change: 0 additions & 1 deletion noir/noir-repo/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,6 @@ pub enum BlackBoxFunc {
/// scalar $a$: `a=low+high*2^{128}`, with `low, high < 2^{128}`
MultiScalarMul,

/// Computes the Keccak-256 (Ethereum version) of the inputs.
/// - inputs: Vector of bytes (witness, 8)
/// - outputs: Array of 32 bytes (witness, 8)
Keccak256,

/// Keccak Permutation function of width 1600
/// - inputs: An array of 25 64-bit Keccak lanes that represent a keccak sponge of 1600 bits
/// - outputs: The result of a keccak f1600 permutation on the input state. Also an array of 25 Keccak lanes.
Expand Down Expand Up @@ -216,7 +211,6 @@ impl BlackBoxFunc {
BlackBoxFunc::AND => "and",
BlackBoxFunc::XOR => "xor",
BlackBoxFunc::RANGE => "range",
BlackBoxFunc::Keccak256 => "keccak256",
BlackBoxFunc::Keccakf1600 => "keccakf1600",
BlackBoxFunc::RecursiveAggregation => "recursive_aggregation",
BlackBoxFunc::EcdsaSecp256r1 => "ecdsa_secp256r1",
Expand Down Expand Up @@ -246,7 +240,6 @@ impl BlackBoxFunc {
"and" => Some(BlackBoxFunc::AND),
"xor" => Some(BlackBoxFunc::XOR),
"range" => Some(BlackBoxFunc::RANGE),
"keccak256" => Some(BlackBoxFunc::Keccak256),
"keccakf1600" => Some(BlackBoxFunc::Keccakf1600),
"recursive_aggregation" => Some(BlackBoxFunc::RecursiveAggregation),
"bigint_add" => Some(BlackBoxFunc::BigIntAdd),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,6 @@ pub enum BlackBoxFuncCall<F> {
input2: Box<[FunctionInput<F>; 3]>,
outputs: (Witness, Witness, Witness),
},
Keccak256 {
inputs: Vec<FunctionInput<F>>,
/// This is the number of bytes to take
/// from the input. Note: if `var_message_size`
/// is more than the number of bytes in the input,
/// then an error is returned.
var_message_size: FunctionInput<F>,
outputs: Box<[Witness; 32]>,
},
Keccakf1600 {
inputs: Box<[FunctionInput<F>; 25]>,
outputs: Box<[Witness; 25]>,
Expand Down Expand Up @@ -254,7 +245,6 @@ impl<F: Copy> BlackBoxFuncCall<F> {
BlackBoxFuncCall::EcdsaSecp256r1 { .. } => BlackBoxFunc::EcdsaSecp256r1,
BlackBoxFuncCall::MultiScalarMul { .. } => BlackBoxFunc::MultiScalarMul,
BlackBoxFuncCall::EmbeddedCurveAdd { .. } => BlackBoxFunc::EmbeddedCurveAdd,
BlackBoxFuncCall::Keccak256 { .. } => BlackBoxFunc::Keccak256,
BlackBoxFuncCall::Keccakf1600 { .. } => BlackBoxFunc::Keccakf1600,
BlackBoxFuncCall::RecursiveAggregation { .. } => BlackBoxFunc::RecursiveAggregation,
BlackBoxFuncCall::BigIntAdd { .. } => BlackBoxFunc::BigIntAdd,
Expand Down Expand Up @@ -361,11 +351,6 @@ impl<F: Copy> BlackBoxFuncCall<F> {
inputs.extend(hashed_message.iter().copied());
inputs
}
BlackBoxFuncCall::Keccak256 { inputs, var_message_size, .. } => {
let mut inputs = inputs.clone();
inputs.push(*var_message_size);
inputs
}
BlackBoxFuncCall::RecursiveAggregation {
verification_key: key,
proof,
Expand All @@ -386,8 +371,7 @@ impl<F: Copy> BlackBoxFuncCall<F> {
pub fn get_outputs_vec(&self) -> Vec<Witness> {
match self {
BlackBoxFuncCall::Blake2s { outputs, .. }
| BlackBoxFuncCall::Blake3 { outputs, .. }
| BlackBoxFuncCall::Keccak256 { outputs, .. } => outputs.to_vec(),
| BlackBoxFuncCall::Blake3 { outputs, .. } => outputs.to_vec(),

BlackBoxFuncCall::Keccakf1600 { outputs, .. } => outputs.to_vec(),

Expand Down
2 changes: 1 addition & 1 deletion noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ fn get_hash_input<F: AcirField>(
// in the message, then we error.
if num_bytes_to_take > message_input.len() {
return Err(OpcodeResolutionError::BlackBoxFunctionFailed(
acir::BlackBoxFunc::Keccak256,
acir::BlackBoxFunc::Blake2s,
format!("the number of bytes to take from the message is more than the number of bytes in the message. {} > {}", num_bytes_to_take, message_input.len()),
));
}
Expand Down
12 changes: 1 addition & 11 deletions noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use acir::{
native_types::{Witness, WitnessMap},
AcirField,
};
use acvm_blackbox_solver::{blake2s, blake3, keccak256, keccakf1600};
use acvm_blackbox_solver::{blake2s, blake3, keccakf1600};

use self::{
aes128::solve_aes128_encryption_opcode, bigint::AcvmBigIntSolver,
Expand Down Expand Up @@ -90,16 +90,6 @@ pub(crate) fn solve<F: AcirField>(
BlackBoxFuncCall::Blake3 { inputs, outputs } => {
solve_generic_256_hash_opcode(initial_witness, inputs, None, outputs, blake3)
}

BlackBoxFuncCall::Keccak256 { inputs, var_message_size, outputs } => {
solve_generic_256_hash_opcode(
initial_witness,
inputs,
Some(var_message_size),
outputs,
keccak256,
)
}
BlackBoxFuncCall::Keccakf1600 { inputs, outputs } => {
let mut state = [0; 25];
for (it, input) in state.iter_mut().zip(inputs.as_ref()) {
Expand Down
1 change: 0 additions & 1 deletion noir/noir-repo/acvm-repo/blackbox_solver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ num-bigint = "0.4"
blake2 = "0.10.6"
blake3 = "1.5.0"
sha2.workspace = true
sha3.workspace = true
keccak = "0.1.4"
k256 = { version = "0.11.0", features = [
"ecdsa",
Expand Down
6 changes: 0 additions & 6 deletions noir/noir-repo/acvm-repo/blackbox_solver/src/hash.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use acir::BlackBoxFunc;
use blake2::digest::generic_array::GenericArray;
use blake2::{Blake2s256, Digest};
use sha3::Keccak256;

use crate::BlackBoxResolutionError;

Expand All @@ -22,11 +21,6 @@ pub fn blake3(inputs: &[u8]) -> Result<[u8; 32], BlackBoxResolutionError> {
Ok(blake3::hash(inputs).into())
}

pub fn keccak256(inputs: &[u8]) -> Result<[u8; 32], BlackBoxResolutionError> {
generic_hash_256::<Keccak256>(inputs)
.map_err(|err| BlackBoxResolutionError::Failed(BlackBoxFunc::Keccak256, err))
}

pub fn sha256_compression(state: &mut [u32; 8], msg_blocks: &[u32; 16]) {
let mut blocks = [0_u8; 64];
for (i, block) in msg_blocks.iter().enumerate() {
Expand Down
4 changes: 2 additions & 2 deletions noir/noir-repo/acvm-repo/blackbox_solver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

//! This crate provides the implementation of BlackBox functions of ACIR and Brillig.
//! For functions that are backend-dependent, it provides a Trait [BlackBoxFunctionSolver] that must be implemented by the backend.
//! For functions that have a reference implementation, such as [keccak256], this crate exports the reference implementation directly.
//! For functions that have a reference implementation, such as [keccakf1600], this crate exports the reference implementation directly.

use acir::BlackBoxFunc;
use thiserror::Error;
Expand All @@ -21,7 +21,7 @@ pub use aes128::aes128_encrypt;
pub use bigint::BigIntSolver;
pub use curve_specific_solver::{BlackBoxFunctionSolver, StubbedBlackBoxSolver};
pub use ecdsa::{ecdsa_secp256k1_verify, ecdsa_secp256r1_verify};
pub use hash::{blake2s, blake3, keccak256, keccakf1600, sha256_compression};
pub use hash::{blake2s, blake3, keccakf1600, sha256_compression};
pub use logic::{bit_and, bit_xor};

#[derive(Clone, PartialEq, Eq, Debug, Error)]
Expand Down
5 changes: 0 additions & 5 deletions noir/noir-repo/acvm-repo/brillig/src/black_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ pub enum BlackBoxOp {
message: HeapVector,
output: HeapArray,
},
/// Calculates the Keccak256 hash of the inputs.
Keccak256 {
message: HeapVector,
output: HeapArray,
},
/// Keccak Permutation function of 1600 width
Keccakf1600 {
message: HeapVector,
Expand Down
11 changes: 2 additions & 9 deletions noir/noir-repo/acvm-repo/brillig_vm/src/black_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use acir::brillig::{BlackBoxOp, HeapArray, HeapVector, IntegerBitSize};
use acir::{AcirField, BlackBoxFunc};
use acvm_blackbox_solver::BigIntSolver;
use acvm_blackbox_solver::{
aes128_encrypt, blake2s, blake3, ecdsa_secp256k1_verify, ecdsa_secp256r1_verify, keccak256,
keccakf1600, sha256_compression, BlackBoxFunctionSolver, BlackBoxResolutionError,
aes128_encrypt, blake2s, blake3, ecdsa_secp256k1_verify, ecdsa_secp256r1_verify, keccakf1600,
sha256_compression, BlackBoxFunctionSolver, BlackBoxResolutionError,
};
use num_bigint::BigUint;
use num_traits::Zero;
Expand Down Expand Up @@ -77,12 +77,6 @@ pub(crate) fn evaluate_black_box<F: AcirField, Solver: BlackBoxFunctionSolver<F>
memory.write_slice(memory.read_ref(output.pointer), &to_value_vec(&bytes));
Ok(())
}
BlackBoxOp::Keccak256 { message, output } => {
let message = to_u8_vec(read_heap_vector(memory, message));
let bytes = keccak256(message.as_slice())?;
memory.write_slice(memory.read_ref(output.pointer), &to_value_vec(&bytes));
Ok(())
}
BlackBoxOp::Keccakf1600 { message, output } => {
let state_vec: Vec<u64> = read_heap_vector(memory, message)
.iter()
Expand Down Expand Up @@ -447,7 +441,6 @@ fn black_box_function_from_op(op: &BlackBoxOp) -> BlackBoxFunc {
BlackBoxOp::AES128Encrypt { .. } => BlackBoxFunc::AES128Encrypt,
BlackBoxOp::Blake2s { .. } => BlackBoxFunc::Blake2s,
BlackBoxOp::Blake3 { .. } => BlackBoxFunc::Blake3,
BlackBoxOp::Keccak256 { .. } => BlackBoxFunc::Keccak256,
BlackBoxOp::Keccakf1600 { .. } => BlackBoxFunc::Keccakf1600,
BlackBoxOp::EcdsaSecp256k1 { .. } => BlackBoxFunc::EcdsaSecp256k1,
BlackBoxOp::EcdsaSecp256r1 { .. } => BlackBoxFunc::EcdsaSecp256r1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ use acvm::{
};

use crate::brillig::brillig_ir::{
brillig_variable::{BrilligVariable, SingleAddrVariable},
debug_show::DebugToString,
registers::RegisterAllocator,
brillig_variable::BrilligVariable, debug_show::DebugToString, registers::RegisterAllocator,
BrilligBinaryOp, BrilligContext,
};

Expand Down Expand Up @@ -61,33 +59,6 @@ pub(crate) fn convert_black_box_call<F: AcirField + DebugToString, Registers: Re
unreachable!("ICE: Blake3 expects one array argument and one array result")
}
}
BlackBoxFunc::Keccak256 => {
if let (
[message, BrilligVariable::SingleAddr(message_size)],
[BrilligVariable::BrilligArray(result_array)],
) = (function_arguments, function_results)
{
let message_vector = convert_array_or_vector(brillig_context, *message, bb_func);
let output_heap_array =
brillig_context.codegen_brillig_array_to_heap_array(*result_array);

// Message_size is not usize
brillig_context.cast_instruction(
SingleAddrVariable::new_usize(message_vector.size),
*message_size,
);

brillig_context.black_box_op_instruction(BlackBoxOp::Keccak256 {
message: message_vector,
output: output_heap_array,
});

brillig_context.deallocate_heap_vector(message_vector);
brillig_context.deallocate_heap_array(output_heap_array);
} else {
unreachable!("ICE: Keccak256 expects message, message size and result array")
}
}
BlackBoxFunc::Keccakf1600 => {
if let ([message], [BrilligVariable::BrilligArray(result_array)]) =
(function_arguments, function_results)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,6 @@ impl DebugShow {
outputs
);
}
BlackBoxOp::Keccak256 { message, output } => {
debug_println!(self.enable_debug_trace, " KECCAK256 {} -> {}", message, output);
}
BlackBoxOp::Keccakf1600 { message, output } => {
debug_println!(self.enable_debug_trace, " KECCAKF1600 {} -> {}", message, output);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,6 @@ impl<F: AcirField> GeneratedAcir<F> {
input2: Box::new([inputs[3][0], inputs[4][0], inputs[5][0]]),
outputs: (outputs[0], outputs[1], outputs[2]),
},
BlackBoxFunc::Keccak256 => {
unreachable!("unexpected BlackBox {}", func_name.to_string())
}
BlackBoxFunc::Keccakf1600 => BlackBoxFuncCall::Keccakf1600 {
inputs: inputs[0]
.clone()
Expand Down Expand Up @@ -475,7 +472,7 @@ impl<F: AcirField> GeneratedAcir<F> {
///
/// This equation however falls short when `t != 0` because then `t`
/// may not be `1`. If `t` is non-zero, then `y` is also non-zero due to
/// `y == 1 - t` and the equation `y * t == 0` fails.
/// `y == 1 - t` and the equation `y * t == 0` fails.
///
/// To fix, we introduce another free variable called `z` and apply the following
/// constraint instead: `y == 1 - t * z`.
Expand All @@ -485,7 +482,7 @@ impl<F: AcirField> GeneratedAcir<F> {
///
/// We now arrive at the conclusion that when `t == 0`, `y` is `1` and when
/// `t != 0`, then `y` is `0`.
///
///
/// Bringing it all together, We introduce two variables `y` and `z`,
/// With the following equations:
/// - `y == 1 - tz` (`z` is a value that is chosen to be the inverse of `t` by the prover)
Expand Down Expand Up @@ -644,7 +641,6 @@ fn black_box_func_expected_input_size(name: BlackBoxFunc) -> Option<usize> {
// All of the hash/cipher methods will take in a
// variable number of inputs.
BlackBoxFunc::AES128Encrypt
| BlackBoxFunc::Keccak256
| BlackBoxFunc::Blake2s
| BlackBoxFunc::Blake3
| BlackBoxFunc::PedersenCommitment
Expand Down Expand Up @@ -696,7 +692,7 @@ fn black_box_expected_output_size(name: BlackBoxFunc) -> Option<usize> {
BlackBoxFunc::AND | BlackBoxFunc::XOR => Some(1),

// 32 byte hash algorithms
BlackBoxFunc::Keccak256 | BlackBoxFunc::Blake2s | BlackBoxFunc::Blake3 => Some(32),
BlackBoxFunc::Blake2s | BlackBoxFunc::Blake3 => Some(32),

BlackBoxFunc::Keccakf1600 => Some(25),
// The permutation returns a fixed number of outputs, equals to the inputs length which depends on the proving system implementation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -517,9 +517,6 @@ fn simplify_black_box_func(
SimplifyResult::None
}
}
BlackBoxFunc::Keccak256 => {
unreachable!("Keccak256 should have been replaced by calls to Keccakf1600")
}
BlackBoxFunc::Poseidon2Permutation => {
blackbox::simplify_poseidon2_permutation(dfg, solver, arguments)
}
Expand Down
2 changes: 0 additions & 2 deletions noir/noir-repo/tooling/profiler/src/opcode_formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ fn format_blackbox_function<F>(call: &BlackBoxFuncCall<F>) -> String {
BlackBoxFuncCall::EcdsaSecp256r1 { .. } => "ecdsa_secp256r1".to_string(),
BlackBoxFuncCall::MultiScalarMul { .. } => "multi_scalar_mul".to_string(),
BlackBoxFuncCall::EmbeddedCurveAdd { .. } => "embedded_curve_add".to_string(),
BlackBoxFuncCall::Keccak256 { .. } => "keccak256".to_string(),
BlackBoxFuncCall::Keccakf1600 { .. } => "keccakf1600".to_string(),
BlackBoxFuncCall::RecursiveAggregation { .. } => "recursive_aggregation".to_string(),
BlackBoxFuncCall::BigIntAdd { .. } => "big_int_add".to_string(),
Expand All @@ -49,7 +48,6 @@ fn format_blackbox_op(call: &BlackBoxOp) -> String {
BlackBoxOp::EcdsaSecp256r1 { .. } => "ecdsa_secp256r1".to_string(),
BlackBoxOp::MultiScalarMul { .. } => "multi_scalar_mul".to_string(),
BlackBoxOp::EmbeddedCurveAdd { .. } => "embedded_curve_add".to_string(),
BlackBoxOp::Keccak256 { .. } => "keccak256".to_string(),
BlackBoxOp::Keccakf1600 { .. } => "keccakf1600".to_string(),
BlackBoxOp::BigIntAdd { .. } => "big_int_add".to_string(),
BlackBoxOp::BigIntSub { .. } => "big_int_sub".to_string(),
Expand Down
Loading