Skip to content

Commit

Permalink
fix(avm): public dispatch in proving tests (#9331)
Browse files Browse the repository at this point in the history
fixes a bug in the poseidon2_permutation, where we were using the space
id from the main trace incorrectly. keccakF also missing an operand for
addressing
  • Loading branch information
IlyasRidhuan authored Oct 22, 2024
1 parent 66f59d6 commit 42e5221
Show file tree
Hide file tree
Showing 25 changed files with 253 additions and 238 deletions.
18 changes: 10 additions & 8 deletions barretenberg/cpp/pil/avm/gadgets/poseidon2.pil
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ namespace poseidon2(256);
pol commit mem_addr_write_b;
pol commit mem_addr_write_c;
pol commit mem_addr_write_d;
// Space Id
pol commit space_id;

// Accessed read / write addresses are contiguous blocks
sel_poseidon_perm * (mem_addr_read_a - input_addr) = 0;
Expand All @@ -53,43 +55,43 @@ namespace poseidon2(256);

// ==== READ MEM OPS =====
#[PERM_POS_MEM_READ_A]
sel_poseidon_perm {clk, main.space_id, mem_addr_read_a, a_0, main.zeroes}
sel_poseidon_perm {clk, space_id, mem_addr_read_a, a_0, main.zeroes}
is
mem.sel_op_poseidon_read_a {mem.clk, mem.space_id, mem.addr, mem.val, mem.rw};

#[PERM_POS_MEM_READ_B]
sel_poseidon_perm {clk, main.space_id, mem_addr_read_b, a_1, main.zeroes}
sel_poseidon_perm {clk, space_id, mem_addr_read_b, a_1, main.zeroes}
is
mem.sel_op_poseidon_read_b {mem.clk, mem.space_id, mem.addr, mem.val, mem.rw};

#[PERM_POS_MEM_READ_C]
sel_poseidon_perm {clk, main.space_id, mem_addr_read_c, a_2, main.zeroes}
sel_poseidon_perm {clk, space_id, mem_addr_read_c, a_2, main.zeroes}
is
mem.sel_op_poseidon_read_c {mem.clk, mem.space_id, mem.addr, mem.val, mem.rw};

#[PERM_POS_MEM_READ_D]
sel_poseidon_perm {clk, main.space_id, mem_addr_read_d, a_3, main.zeroes}
sel_poseidon_perm {clk, space_id, mem_addr_read_d, a_3, main.zeroes}
is
mem.sel_op_poseidon_read_d {mem.clk, mem.space_id, mem.addr, mem.val, mem.rw};

//// ==== WRITE MEM OPS =====
#[PERM_POS_MEM_WRITE_A]
sel_poseidon_perm {clk, main.space_id, mem_addr_write_a, b_0, sel_poseidon_perm}
sel_poseidon_perm {clk, space_id, mem_addr_write_a, b_0, sel_poseidon_perm}
is
mem.sel_op_poseidon_write_a {mem.clk, mem.space_id, mem.addr, mem.val, mem.rw};

#[PERM_POS_MEM_WRITE_B]
sel_poseidon_perm {clk, main.space_id, mem_addr_write_b, b_1, sel_poseidon_perm}
sel_poseidon_perm {clk, space_id, mem_addr_write_b, b_1, sel_poseidon_perm}
is
mem.sel_op_poseidon_write_b {mem.clk, mem.space_id, mem.addr, mem.val, mem.rw};

#[PERM_POS_MEM_WRITE_C]
sel_poseidon_perm {clk, main.space_id, mem_addr_write_c, b_2, sel_poseidon_perm}
sel_poseidon_perm {clk, space_id, mem_addr_write_c, b_2, sel_poseidon_perm}
is
mem.sel_op_poseidon_write_c {mem.clk, mem.space_id, mem.addr, mem.val, mem.rw};

#[PERM_POS_MEM_WRITE_D]
sel_poseidon_perm {clk, main.space_id, mem_addr_write_d, b_3, sel_poseidon_perm}
sel_poseidon_perm {clk, space_id, mem_addr_write_d, b_3, sel_poseidon_perm}
is
mem.sel_op_poseidon_write_d {mem.clk, mem.space_id, mem.addr, mem.val, mem.rw};

Expand Down
4 changes: 2 additions & 2 deletions barretenberg/cpp/pil/avm/main.pil
Original file line number Diff line number Diff line change
Expand Up @@ -555,9 +555,9 @@ namespace main(256);
// Mem_addr_a points to the start of the input array of length 4
// Mem_addr_b points to the start of the output array of length 4
#[PERM_MAIN_POS2_PERM]
sel_op_poseidon2 {clk, mem_addr_a, mem_addr_b}
sel_op_poseidon2 {clk, space_id, mem_addr_a, mem_addr_b}
is
poseidon2.sel_poseidon_perm {poseidon2.clk, poseidon2.input_addr, poseidon2.output_addr};
poseidon2.sel_poseidon_perm {poseidon2.clk, poseidon2.space_id, poseidon2.input_addr, poseidon2.output_addr};

// This will be enabled when we migrate just to keccakf1600, as getting keccak to work with it is tricky.
// #[PERM_MAIN_KECCAK]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -635,6 +635,7 @@ AvmCircuitBuilder::ProverPolynomials AvmCircuitBuilder::compute_polynomials() co
polys.poseidon2_mem_addr_write_d.set_if_valid_index(i, rows[i].poseidon2_mem_addr_write_d);
polys.poseidon2_output_addr.set_if_valid_index(i, rows[i].poseidon2_output_addr);
polys.poseidon2_sel_poseidon_perm.set_if_valid_index(i, rows[i].poseidon2_sel_poseidon_perm);
polys.poseidon2_space_id.set_if_valid_index(i, rows[i].poseidon2_space_id);
polys.range_check_alu_rng_chk.set_if_valid_index(i, rows[i].range_check_alu_rng_chk);
polys.range_check_clk.set_if_valid_index(i, rows[i].range_check_clk);
polys.range_check_cmp_hi_bits_rng_chk.set_if_valid_index(i, rows[i].range_check_cmp_hi_bits_rng_chk);
Expand Down
Loading

1 comment on commit 42e5221

@AztecBot
Copy link
Collaborator

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'C++ Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.05.

Benchmark suite Current: 42e5221 Previous: 8d75dd4 Ratio
wasmClientIVCBench/Full/6 94252.168168 ms/iter 87331.435856 ms/iter 1.08

This comment was automatically generated by workflow using github-action-benchmark.

CC: @ludamad @codygunton

Please sign in to comment.