Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
5 changes: 4 additions & 1 deletion crates/examples/benches/keccak.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ fn bench_keccak_permutations(c: &mut Criterion) {
println!(" Permutations: {}", n_permutations);
println!("=======================================\n");

let params = Params { n_permutations };
let params = Params {
n_permutations,
no_intrinsic: false,
};
let instance = Instance {};

// Setup phase - do this once outside the benchmark loop
Expand Down
8 changes: 4 additions & 4 deletions crates/examples/snapshots/ethsign.snap
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
ethsign circuit
--
Number of gates: 264472
Number of evaluation instructions: 310572
Number of AND constraints: 359966
Number of gates: 258954
Number of evaluation instructions: 305054
Number of AND constraints: 355646
Number of MUL constraints: 25458
Length of value vec: 524288
Constants: 82
Inout: 29
Witness: 42
Internal: 479997
Internal: 475677
10 changes: 5 additions & 5 deletions crates/examples/snapshots/hash_based_sig.snap
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
hash_based_sig circuit
--
Number of gates: 3986883
Number of evaluation instructions: 4076982
Number of AND constraints: 4022460
Number of gates: 1421013
Number of evaluation instructions: 1511112
Number of AND constraints: 2013660
Number of MUL constraints: 0
Length of value vec: 4194304
Length of value vec: 2097152
Constants: 376
Inout: 20
Witness: 29886
Internal: 3978540
Internal: 1969740
10 changes: 5 additions & 5 deletions crates/examples/snapshots/keccak.snap
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
keccak circuit
--
Number of gates: 27625
Number of evaluation instructions: 27625
Number of AND constraints: 27625
Number of gates: 35
Number of evaluation instructions: 35
Number of AND constraints: 6025
Number of MUL constraints: 0
Length of value vec: 32768
Length of value vec: 8192
Constants: 23
Inout: 50
Witness: 0
Internal: 27600
Internal: 6000
6 changes: 5 additions & 1 deletion crates/examples/src/circuits/keccak.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ pub struct Params {
/// Number of Keccak-f\[1600\] permutations to chain together
#[arg(short = 'n', long, default_value_t = 10)]
pub n_permutations: usize,

/// Do not use Keccakf1600 intrinsic opcode, build primitive gate circuit instead
#[arg(long)]
pub no_intrinsic: bool,
}

#[derive(Args, Debug, Clone)]
Expand All @@ -42,7 +46,7 @@ impl ExampleCircuit for KeccakExample {
// Chain n permutations starting from initial state
let mut computed_state = initial_state;
for _ in 0..params.n_permutations {
Permutation::keccak_f1600(builder, &mut computed_state);
Permutation::keccak_f1600(builder, !params.no_intrinsic, &mut computed_state);
}

// Constrain computed final state to equal expected final state
Expand Down
26 changes: 25 additions & 1 deletion crates/frontend/src/circuits/keccak/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,30 @@ impl Keccak {
len_bytes: Wire,
digest: [Wire; N_WORDS_PER_DIGEST],
message: Vec<Wire>,
) -> Self {
let use_intrinsic = true;
Self::new_with_intrinsic(b, use_intrinsic, len_bytes, digest, message)
}

/// Create a new keccak circuit using the circuit builder
///
/// # Arguments
///
/// * `builder` - circuit builder object
/// * `use_intrinsic` - use `Keccakf1600` opcode instead of a circuit of more primitive gates.
/// * `max_len` - max message length in bytes for this circuit instance
/// * `len` - wire representing the claimed input message length in bytes
/// * `digest` - array of 4 wires representing the claimed 256-bit output digest
/// * `message` - vector of wires representing the claimed input message
///
/// ## Preconditions
/// * max_len > 0
pub fn new_with_intrinsic(
b: &CircuitBuilder,
use_intrinsic: bool,
len_bytes: Wire,
digest: [Wire; N_WORDS_PER_DIGEST],
message: Vec<Wire>,
) -> Self {
let max_len_bytes = message.len() << 3;
// number of blocks needed for the maximum sized message
Expand Down Expand Up @@ -74,7 +98,7 @@ impl Keccak {
b.bxor(state_in[i], padded_message[block_no * N_WORDS_PER_BLOCK + i]);
}

Permutation::keccak_f1600(b, &mut xored_state);
Permutation::keccak_f1600(b, use_intrinsic, &mut xored_state);

states.push(xored_state);
}
Expand Down
30 changes: 21 additions & 9 deletions crates/frontend/src/circuits/keccak/permutation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ impl Permutation {
///
/// ## Arguments
///
/// * `b` - The circuit builder to use.
pub fn new(b: &CircuitBuilder, input_state: State) -> Self {
/// * `b` - The circuit builder to use.
/// * `use_intrinsic` - Use `Keccakf1600` opcode instead of a circuit of more primitive gates.
/// * `input_state` - a Keccak-f\[1600\] state, in lane order (column major, `i = x + 5*y`).
pub fn new(b: &CircuitBuilder, use_intrinsic: bool, input_state: State) -> Self {
let mut output_state = input_state;
Self::keccak_f1600(b, &mut output_state.words);
Self::keccak_f1600(b, use_intrinsic, &mut output_state.words);

Self {
input_state,
Expand All @@ -47,15 +49,21 @@ impl Permutation {
}
}

/// Perform the Keccak f\[1600\] permutation.
/// Perform the Keccak-f\[1600\] permutation.
///
/// ## Arguments
///
/// * `b` - The circuit builder to use.
/// * `use_intrinsic` - Use `Keccakf1600` opcode instead of a circuit of more primitive gates.
/// * `state` - The state to perform the permutation on.
pub fn keccak_f1600(b: &CircuitBuilder, state: &mut [Wire; 25]) {
for round in 0..24 {
Self::keccak_permutation_round(b, state, round);
pub fn keccak_f1600(b: &CircuitBuilder, use_intrinsic: bool, state: &mut [Wire; 25]) {
if use_intrinsic {
let outputs = b.keccakf1600(*state);
*state = outputs;
} else {
for round in 0..24 {
Self::keccak_permutation_round(b, state, round)
}
}
}

Expand Down Expand Up @@ -151,7 +159,7 @@ mod tests {
words: std::array::from_fn(|_| builder.add_inout()),
};

let permutation = Permutation::new(&builder, input_words);
let permutation = Permutation::new(&builder, true, input_words);

let circuit = builder.build();

Expand Down Expand Up @@ -215,7 +223,11 @@ mod tests {
let mut rng = StdRng::seed_from_u64(0);
let input_state = rng.random::<[u64; 25]>();

validate_circuit_component(Permutation::keccak_f1600, keccak_f1600_reference, input_state);
validate_circuit_component(
|builder, state| Permutation::keccak_f1600(builder, true, state),
keccak_f1600_reference,
input_state,
);
}

#[test]
Expand Down
8 changes: 8 additions & 0 deletions crates/frontend/src/compiler/eval_form/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,14 @@ impl BytecodeBuilder {
self.emit_u32(error_id);
}

pub fn emit_keccakf1600(&mut self, rounds: [u32; (24 + 1) * 25]) {
self.n_eval_insn += 1;
self.emit_u8(0x70);
for reg in rounds {
self.emit_reg(reg);
}
}

// Hint calls
pub fn emit_hint(
&mut self,
Expand Down
60 changes: 60 additions & 0 deletions crates/frontend/src/compiler/eval_form/interpreter.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Bytecode interpreter for circuit evaluation

use std::array;

use binius_core::{ValueIndex, ValueVec, Word};

use crate::compiler::{circuit::PopulateError, hints::HintRegistry};
Expand Down Expand Up @@ -135,6 +137,9 @@ impl<'a> Interpreter<'a> {
0x64 => self.exec_assert_false(ctx),
0x65 => self.exec_assert_true(ctx),

// High level intrinsics
0x70 => self.exec_keccakf1600(ctx),

// Hint calls
0x80 => self.exec_hint(ctx),

Expand Down Expand Up @@ -451,6 +456,61 @@ impl<'a> Interpreter<'a> {
}
}

fn exec_keccakf1600(&mut self, ctx: &mut ExecutionContext<'_>) {
let mut a: [Word; 25] = array::from_fn(|_| {
let reg = self.read_reg();
self.load(ctx, reg)
});

use super::gate::keccakf1600::{CONSTS, R};

fn idx(x: usize, y: usize) -> usize {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: Is there a reason not to reuse the public idx function from keccakf1600?

(x % 5) + 5 * (y % 5)
}

fn rot(amount: u32, value: Word) -> Word {
value.rotr(64 - amount)
}

// ι round constants occupy first 24 words of Keccakf1600 gate constants
for &round_constant in &CONSTS[..24] {
// θ
let c: [Word; 5] =
array::from_fn(|i| (0..5).fold(Word::ZERO, |acc, j| acc ^ a[idx(i, j)]));
let d: [Word; 5] = array::from_fn(|i| c[(i + 4) % 5] ^ rot(1, c[(i + 1) % 5]));

for i in 0..25 {
a[i] = a[i] ^ d[i % 5];
}

// ρ & π
let mut b = [Word::ZERO; 25];
for x in 0..5 {
for y in 0..5 {
let i = idx(x, y);
b[idx(y, 2 * x + 3 * y)] = rot(R[i], a[i]);
}
}

// χ
for x in 0..5 {
for y in 0..5 {
let i = idx(x, y);
a[i] = b[i] ^ !b[idx(x + 1, y)] & b[idx(x + 2, y)];
}
}

// ι
a[0] = a[0] ^ round_constant;

// writeback
for val in a {
let reg = self.read_reg();
self.store(ctx, reg, val);
}
Comment on lines +507 to +510
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical Bug: Incorrect State Writeback in Keccak-f[1600] Implementation

There's a significant issue with the state writeback logic in the exec_keccakf1600 function. Currently, the writeback loop (lines 507-510) is inside the round loop, causing state values to be written back after each round:

for &round_constant in &CONSTS[..24] {
    // θ, ρ, π, χ, ι steps...
    
    // writeback - THIS IS THE PROBLEM
    for val in a {
        let reg = self.read_reg();
        self.store(ctx, reg, val);
    }
}

This is incorrect because:

  1. It overwrites intermediate state values 24 times
  2. It expects 24×25 output registers when the gate only provides 25
  3. It produces incorrect permutation results

The writeback loop should be moved outside and after the round loop, so it executes only once after all 24 rounds are complete:

for &round_constant in &CONSTS[..24] {
    // θ, ρ, π, χ, ι steps...
}

// writeback (only once, after all rounds)
for val in a {
    let reg = self.read_reg();
    self.store(ctx, reg, val);
}

This change will ensure the correct final state is written to the output registers.

Suggested change
for val in a {
let reg = self.read_reg();
self.store(ctx, reg, val);
}
}
// Writeback (only once, after all rounds)
for val in a {
let reg = self.read_reg();
self.store(ctx, reg, val);
}

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@onesk WDYT?

}
}

// Hint execution
fn exec_hint(&mut self, ctx: &mut ExecutionContext<'_>) {
let hint_id = self.read_u32() as usize;
Expand Down
Loading