feat: new keccak

[GunaDD]

Description

New Keccak with Xorin and Keccakf chip and opcode

Checklist

• I have performed a self-review of my own code
• Add negative tests for xorin chip
• Add negative tests for keccakf chip
• Add unit test to CI
• Add new guest code for E2E test to CI (the keccak example is updated, but I am thinking of adding another one)
• Check with Ayush if I implemented the SizedRecord trait correctly
• Rebase to include Zach's new Plonky3 update and update the keccakf trace gen to not have to transpose any more before giving it into the input

To reviewer: I will still have to complete the above checklist. But you can start reviewing if you would like to.

Closes INT-5017, INT-5721, INT-5720, INT-5718, INT-5717, INT-5646, INT-5018

[jonathanpwang]

why'd you change this? unless you updated the plonky3 commit; this is just definitional

[GunaDD]

because somehow some bus which was unbalanced before became balanced after this change but I am still trying to understand what is the correct way to do it since the memory read / write busses are still unbalanced because the data being written after the keccakf operation does not match

[GunaDD]

fn generate_trace_rows_for_perm<F: PrimeField64>(rows: &mut [KeccakCols<F>], input: [u64; 25]) {
    let mut current_state: [[u64; 5]; 5] = unsafe { transmute(input) };

    let initial_state: [[[F; 4]; 5]; 5] =
        array::from_fn(|y| array::from_fn(|x| u64_to_16_bit_limbs(current_state[x][y])));

turns out inside plonky3's generate_trace_rows the input gets transposed which is why the bus becoming balanced makes sense now

[GunaDD]

turns out in the current keccak trace gen we initially transpose the state first

openvm/extensions/keccak256/circuit/src/trace.rs

Lines 403 to 410 in 77adf7e

	// We need to transpose state matrices due to a plonky3 issue: https://github.com/Plonky3/Plonky3/issues/672
	// Note: the fix for this issue will be a commit after the major Field crate refactor PR https://github.com/Plonky3/Plonky3/pull/640
	// which will require a significant refactor to switch to.
	let state = from_fn(|i| {
	let x = i / 5;
	let y = i % 5;
	states[block_idx][x + 5 * y]
	});

which is why we don't swap y and x there

[GunaDD]

turns out to fix it I just have to transpose the input to the plonky3 trace gen and since the transpose of the transpose is the original matrix, everything will be as expected. fixed it here

openvm/extensions/new-keccak256/circuit/src/keccakf/trace.rs

Lines 233 to 236 in f1d6a76

	// the reason we give the transpose instead is inside, plonky3 transpose the input
	// so transpose of transpose fixes it
	let p3_trace: RowMajorMatrix<F> = generate_trace_rows(vec![preimage_buffer_bytes_u64_transpose], 0);
	row[..NUM_KECCAK_PERM_COLS].copy_from_slice(

[GunaDD]

@jonathanpwang when the plonky3 thing gets fixed the change to the code should be to just revert into passing in vec![preimage_buffer_bytes_u64 instead of the transpose to generate_trace_rows

[codspeed-hq]

CodSpeed Performance Report

Merging #2303 will improve performances by ×2.2

_{Comparing feat/new-keccak (e1c71cf) with main (77adf7e)¹}

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

Summary

⚡ 1 improvement
✅ 23 untouched
⏩ 36 skipped²

Benchmarks breakdown

	Mode	Benchmark	BASE	HEAD	Change
⚡	WallTime	benchmark_execute_metered[quicksort]	19.5 ms	9 ms	×2.2

Footnotes

1. No successful run was found on develop-new-keccak (77adf7e) during the generation of this report, so main (77adf7e) was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

2. 36 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[github-actions]

group	app.proof_time_ms	app.cycles	app.cells_used	leaf.proof_time_ms	leaf.cycles	leaf.cells_used
verify_fibair	226	322,610	2,058,654	-	-	-
fibonacci	1,045	1,500,209	2,100,402	-	-	-
regex	2,313	4,137,502	17,695,216	-	-	-
ecrecover	744	122,859	2,263,998	-	-	-
pairing	1,494	1,745,742	25,408,302	-	-	-

Commit: e1c71cf

Benchmark Workflow

[shuklaayush]

dropped a few comments. still reviewing

[shuklaayush]

use IS_E1 to switch between slice and normal reads

[GunaDD]

thank you for pointing this out

[shuklaayush]

do these need to be range checked?

[GunaDD]

yes it needs to be and I forgot about it. thank you for pointing this out

[GunaDD]

I am now wondering what constraints the correctness of preimage_state_hi as the upper u8 limb of preimage in the current keccak256 chip