Skip to content

Commit c27a662

Browse files
hero78119hero78119
authored
fix keccak precompile e2e (#1036)
build on top of #1034 & #1035 & #1038 This PR fix keccak precompile e2e error, some previous left over. - [x] rom check error => fixed ecall instrunction specs - [x] logup sum error => The logup sum != 0 error, see https://github.com/scroll-tech/ceno/pull/1036/files#r2301146794. Need to fix ceno-recursion verifier as well - [x] prod_r != prod_w => remove X11 read, which cause ts to bump and mismatch with circuit definition. https://github.com/scroll-tech/ceno/pull/1036/files#r2301152148
1 parent 1d2173e commit c27a662

File tree

21 files changed

+368
-294
lines changed

21 files changed

+368
-294
lines changed

‎.github/workflows/integration.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ jobs:
6565
RUSTFLAGS: "-C opt-level=3"
6666
run: cargo run --release --package ceno_zkvm --no-default-features --features goldilocks --bin e2e -- --field=goldilocks --platform=ceno --hints=10 --public-io=4191 examples/target/riscv32im-ceno-zkvm-elf/release/examples/fibonacci
6767

68+
- name: Run keccak_syscall (release)
69+
env:
70+
RUSTFLAGS: "-C opt-level=3"
71+
run: cargo run --release --package ceno_zkvm --bin e2e -- --platform=ceno examples/target/riscv32im-ceno-zkvm-elf/release/examples/keccak_syscall
6872

6973
- name: Install cargo make
7074
run: |

‎ceno_emul/src/syscalls/keccak_permute.rs‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ pub struct KeccakSpec;
1313
impl SyscallSpec for KeccakSpec {
1414
const NAME: &'static str = "KECCAK";
1515

16-
const REG_OPS_COUNT: usize = 2;
16+
const REG_OPS_COUNT: usize = 1;
1717
const MEM_OPS_COUNT: usize = KECCAK_WORDS;
1818
const CODE: u32 = ceno_rt::syscalls::KECCAK_PERMUTE;
1919
const HAS_LOOKUPS: bool = true;
@@ -55,22 +55,12 @@ impl From<KeccakState> for [Word; KECCAK_WORDS] {
5555
pub fn keccak_permute(vm: &VMState) -> SyscallEffects {
5656
let state_ptr = vm.peek_register(Platform::reg_arg0());
5757

58-
// for compatibility with sp1 spec
59-
assert_eq!(vm.peek_register(Platform::reg_arg1()), 0);
60-
6158
// Read the argument `state_ptr`.
62-
let reg_ops = vec![
63-
WriteOp::new_register_op(
64-
Platform::reg_arg0(),
65-
Change::new(state_ptr, state_ptr),
66-
0, // Cycle set later in finalize().
67-
),
68-
WriteOp::new_register_op(
69-
Platform::reg_arg1(),
70-
Change::new(0, 0),
71-
0, // Cycle set later in finalize().
72-
),
73-
];
59+
let reg_ops = vec![WriteOp::new_register_op(
60+
Platform::reg_arg0(),
61+
Change::new(state_ptr, state_ptr),
62+
0, // Cycle set later in finalize().
63+
)];
7464

7565
let mut state_view = MemoryView::<KECCAK_WORDS>::new(vm, state_ptr);
7666
let mut state = KeccakState::from(state_view.words());

‎ceno_emul/src/syscalls/secp256k1.rs‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -139,22 +139,12 @@ pub fn secp256k1_add(vm: &VMState) -> SyscallEffects {
139139
pub fn secp256k1_double(vm: &VMState) -> SyscallEffects {
140140
let p_ptr = vm.peek_register(Platform::reg_arg0());
141141

142-
// for compatibility with sp1 spec
143-
assert_eq!(vm.peek_register(Platform::reg_arg1()), 0);
144-
145142
// Read the argument pointers
146-
let reg_ops = vec![
147-
WriteOp::new_register_op(
148-
Platform::reg_arg0(),
149-
Change::new(p_ptr, p_ptr),
150-
0, // Cycle set later in finalize().
151-
),
152-
WriteOp::new_register_op(
153-
Platform::reg_arg1(),
154-
Change::new(0, 0),
155-
0, // Cycle set later in finalize().
156-
),
157-
];
143+
let reg_ops = vec![WriteOp::new_register_op(
144+
Platform::reg_arg0(),
145+
Change::new(p_ptr, p_ptr),
146+
0, // Cycle set later in finalize().
147+
)];
158148

159149
// P's memory segment
160150
let mut p_view = MemoryView::<SECP256K1_ARG_WORDS>::new(vm, p_ptr);

‎ceno_emul/src/syscalls/sha256.rs‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -45,22 +45,12 @@ pub fn sha_extend(w: &mut [u32]) {
4545
pub fn extend(vm: &VMState) -> SyscallEffects {
4646
let state_ptr = vm.peek_register(Platform::reg_arg0());
4747

48-
// for compatibility with sp1 spec
49-
assert_eq!(vm.peek_register(Platform::reg_arg1()), 0);
50-
5148
// Read the argument `state_ptr`.
52-
let reg_ops = vec![
53-
WriteOp::new_register_op(
54-
Platform::reg_arg0(),
55-
Change::new(state_ptr, state_ptr),
56-
0, // Cycle set later in finalize().
57-
),
58-
WriteOp::new_register_op(
59-
Platform::reg_arg1(),
60-
Change::new(0, 0),
61-
0, // Cycle set later in finalize().
62-
),
63-
];
49+
let reg_ops = vec![WriteOp::new_register_op(
50+
Platform::reg_arg0(),
51+
Change::new(state_ptr, state_ptr),
52+
0, // Cycle set later in finalize().
53+
)];
6454

6555
let mut state_view = MemoryView::<SHA_EXTEND_WORDS>::new(vm, state_ptr);
6656
let mut sha_extend_words = ShaExtendWords::from(state_view.words());

‎ceno_host/tests/test_elf.rs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -256,9 +256,8 @@ fn test_keccak_syscall() -> Result<()> {
256256

257257
// Check the syscall effects.
258258
for (witness, expect) in izip!(syscalls, keccak_first_iter_outs) {
259-
assert_eq!(witness.reg_ops.len(), 2);
259+
assert_eq!(witness.reg_ops.len(), 1);
260260
assert_eq!(witness.reg_ops[0].register_index(), Platform::reg_arg0());
261-
assert_eq!(witness.reg_ops[1].register_index(), Platform::reg_arg1());
262261

263262
assert_eq!(witness.mem_ops.len(), expect.len() * 2);
264263
let got = witness
@@ -353,7 +352,7 @@ fn test_secp256k1_double() -> Result<()> {
353352
assert_eq!(syscalls.len(), 1);
354353

355354
let witness = syscalls[0];
356-
assert_eq!(witness.reg_ops.len(), 2);
355+
assert_eq!(witness.reg_ops.len(), 1);
357356
assert_eq!(witness.reg_ops[0].register_index(), Platform::reg_arg0());
358357

359358
let p_address = witness.reg_ops[0].value.after;
@@ -444,9 +443,8 @@ fn test_sha256_extend() -> Result<()> {
444443
assert_eq!(syscalls.len(), 1);
445444

446445
let witness = syscalls[0];
447-
assert_eq!(witness.reg_ops.len(), 2);
446+
assert_eq!(witness.reg_ops.len(), 1);
448447
assert_eq!(witness.reg_ops[0].register_index(), Platform::reg_arg0());
449-
assert_eq!(witness.reg_ops[1].register_index(), Platform::reg_arg1());
450448

451449
let state_ptr = witness.reg_ops[0].value.after;
452450
assert_eq!(state_ptr, witness.reg_ops[0].value.before);

‎ceno_rt/src/syscalls.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ pub fn syscall_keccak_permute(state: &mut [u64; 25]) {
2828
"ecall",
2929
in("t0") KECCAK_PERMUTE,
3030
in("a0") state as *mut [u64; 25],
31-
in("a1") 0
3231
);
3332
}
3433
#[cfg(not(target_os = "zkvm"))]

‎ceno_zkvm/benches/keccak.rs‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,12 @@ use ceno_zkvm::{
77
e2e::{Checkpoint, Preset, run_e2e_with_checkpoint, setup_platform},
88
};
99
mod alloc;
10+
use ceno_zkvm::scheme::verifier::ZKVMVerifier;
1011
use criterion::*;
11-
1212
use ff_ext::BabyBearExt4;
1313
use gkr_iop::cpu::{CpuBackend, CpuProver};
1414
use mpcs::BasefoldDefault;
15+
use transcript::BasicTranscript;
1516

1617
criterion_group! {
1718
name = keccak_prove_group;
@@ -44,29 +45,28 @@ fn keccak_prove(c: &mut Criterion) {
4445
let _ = hints.write(&vec![1, 2, 3]);
4546
let max_steps = usize::MAX;
4647
// estimate proof size data first
47-
// let result = run_e2e_with_checkpoint::<E, Pcs>(
48-
// program.clone(),
49-
// platform.clone(),
50-
// &Vec::from(&hints),
51-
// &[],
52-
// max_steps,
53-
// MAX_NUM_VARIABLES,
54-
// SecurityLevel::default(),
55-
// Checkpoint::Complete,
56-
// );
57-
// let proof = result.proof.expect("PrepSanityCheck do not provide proof");
58-
// let vk = result.vk.expect("PrepSanityCheck do not provide verifier");
48+
let result = run_e2e_with_checkpoint::<E, Pcs, _, _>(
49+
CpuProver::new(backend.clone()),
50+
program.clone(),
51+
platform.clone(),
52+
&Vec::from(&hints),
53+
&[],
54+
max_steps,
55+
Checkpoint::Complete,
56+
);
57+
let proof = result.proof.expect("PrepSanityCheck do not provide proof");
58+
let vk = result.vk.expect("PrepSanityCheck do not provide verifier");
5959

60-
// println!("e2e proof {}", proof);
61-
// let transcript = BasicTranscript::new(b"riscv");
62-
// let verifier = ZKVMVerifier::<E, Pcs>::new(vk);
63-
// assert!(
64-
// verifier
65-
// .verify_proof_halt(proof, transcript, false)
66-
// .expect("verify proof return with error"),
67-
// );
68-
// println!();
69-
// println!("max_steps = {}", max_steps);
60+
println!("e2e proof {}", proof);
61+
let transcript = BasicTranscript::new(b"riscv");
62+
let verifier = ZKVMVerifier::<E, Pcs>::new(vk);
63+
assert!(
64+
verifier
65+
.verify_proof_halt(proof, transcript, true)
66+
.expect("verify proof return with error"),
67+
);
68+
println!();
69+
println!("max_steps = {}", max_steps);
7070

7171
// expand more input size once runtime is acceptable
7272
let mut group = c.benchmark_group(format!("keccak_max_steps_{}", max_steps));

‎ceno_zkvm/src/instructions.rs‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,20 +62,23 @@ pub trait Instruction<E: ExtensionField> {
6262
let (out_evals, mut chip) = (
6363
[
6464
// r_record
65-
(selector_type.clone(), (0..r_len).collect_vec()),
65+
(0..r_len).collect_vec(),
6666
// w_record
67-
(selector_type.clone(), (r_len..r_len + w_len).collect_vec()),
67+
(r_len..r_len + w_len).collect_vec(),
6868
// lk_record
69-
(
70-
selector_type.clone(),
71-
(r_len + w_len..r_len + w_len + lk_len).collect_vec(),
72-
),
69+
(r_len + w_len..r_len + w_len + lk_len).collect_vec(),
7370
// zero_record
74-
(selector_type, (0..zero_len).collect_vec()),
71+
(0..zero_len).collect_vec(),
7572
],
7673
Chip::new_from_cb(cb, 0),
7774
);
7875

76+
// register selector to legacy constrain system
77+
cb.cs.r_selector = Some(selector_type.clone());
78+
cb.cs.w_selector = Some(selector_type.clone());
79+
cb.cs.lk_selector = Some(selector_type.clone());
80+
cb.cs.zero_selector = Some(selector_type.clone());
81+
7982
let layer = Layer::from_circuit_builder(cb, "Rounds".to_string(), 0, out_evals);
8083
chip.add_layer(layer);
8184

0 commit comments

Comments
 (0)