align sierra-emu StarknetSyscallHandler trait/types with cairo-native#1610
align sierra-emu StarknetSyscallHandler trait/types with cairo-native#1610avi-starkware wants to merge 1 commit into
Conversation
|
✅ Code is now correctly formatted. |
Benchmarking resultsBenchmark for program
|
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
11.503 ± 0.103 | 11.389 | 11.740 | 5.76 ± 0.09 |
cairo-native (embedded AOT) |
2.002 ± 0.034 | 1.965 | 2.072 | 1.00 ± 0.02 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1.996 ± 0.028 | 1.955 | 2.036 | 1.00 |
Benchmark for program dict_snapshot
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
572.6 ± 14.7 | 563.4 | 612.4 | 1.00 |
cairo-native (embedded AOT) |
1718.6 ± 23.7 | 1671.8 | 1749.5 | 3.00 ± 0.09 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1752.8 ± 49.7 | 1710.1 | 1873.3 | 3.06 ± 0.12 |
Benchmark for program factorial_2M
Open benchmarks
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.960 ± 0.024 | 4.939 | 5.024 | 2.19 ± 0.03 |
cairo-native (embedded AOT) |
2.264 ± 0.021 | 2.230 | 2.290 | 1.00 ± 0.02 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2.261 ± 0.031 | 2.217 | 2.303 | 1.00 |
Benchmark for program fib_2M
Open benchmarks
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.855 ± 0.060 | 4.813 | 5.019 | 2.69 ± 0.05 |
cairo-native (embedded AOT) |
1.803 ± 0.028 | 1.753 | 1.840 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1.816 ± 0.015 | 1.793 | 1.837 | 1.01 ± 0.02 |
Benchmark for program linear_search
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
625.0 ± 35.4 | 598.6 | 713.0 | 1.00 |
cairo-native (embedded AOT) |
1754.3 ± 21.4 | 1718.0 | 1774.5 | 2.81 ± 0.16 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1757.9 ± 16.7 | 1734.1 | 1788.5 | 2.81 ± 0.16 |
Benchmark for program logistic_map
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
522.5 ± 9.4 | 511.5 | 536.4 | 1.00 |
cairo-native (embedded AOT) |
1915.7 ± 33.9 | 1874.7 | 1965.6 | 3.67 ± 0.09 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2006.7 ± 20.6 | 1974.8 | 2028.1 | 3.84 ± 0.08 |
Benchmark results Main vs HEAD.Base
Head
Base
Head
Base
Head
Base
Head
Base
Head
Base
Head
|
7698d47 to
a37920a
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 6 files and made 2 comments.
Reviewable status: 6 of 8 files reviewed, 2 unresolved discussions (waiting on avi-starkware, TomerStarkware, and Yoni-Starkware).
debug_utils/sierra-emu/src/starknet.rs line 580 at r1 (raw file):
Coordinates::Uncompressed { x, y } => (x, y), Coordinates::Identity => { // k * P can be the identity (e.g. k = ord(P)).
Suggestion:
// m * P can be the identity (e.g. m = ord(P)).debug_utils/sierra-emu/src/starknet/secp256r1_point.rs line 8 at r1 (raw file):
pub x: U256, pub y: U256, pub is_infinity: bool,
why add is_infinity? what was bad about (0, 0)?
a37920a to
63b478e
Compare
|
Previously, orizi wrote…
The |
63b478e to
26d9249
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 3 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware and Yoni-Starkware).
|
Done. |
Sierra-emu adopts cairo-native's syscall surface so the two traits/types
match name-for-name. Prerequisite for extracting them into a shared
crate (next PR).
- Slice signatures: deploy/library_call/call_contract/emit_event/
send_message_to_l1/keccak/meta_tx_v0/cheatcode now take &[Felt] / &[u64]
rather than owned Vec.
- Secp256{k1,r1}Point gain an explicit is_infinity flag. The Sierra
Value encoding canonicalizes (0, 0) for the identity element so the
round-trip via from_value/into_value is lossless: into_value forces
(x, y) = (0, 0) when is_infinity is set; from_value recovers the flag
from the sentinel. (0, 0) is not a real curve point, so the aliasing
is unambiguous.
- Stub secp arithmetic now handles `Coordinates::Identity` from
to_encoded_point(false), returning the canonical (0, 0) + is_infinity
point instead of tripping unreachable!(). Pre-existing bug surfaced
by the trait alignment.
- sha256_process_block matches cairo-native's mutating signature
(&mut [u32; 8], &[u32; 16]) -> SyscallResult<()>.
- Add ExecutionInfoV3 / TxV3Info types and a get_execution_info_v3 trait
method. The Sierra-level Value lowering for v3 isn't wired in the VM
yet -- eval_get_execution_info_v3 soft-fails with a descriptive felt
rather than panicking with todo!(), so any contract calling the v3
syscall sees a graceful syscall error. Full v3 wiring is a separate
effort.
- cairo-native: cheatcode trait method becomes unconditional (was gated
behind with-cheatcode). The default impl is unimplemented!(), so this
is backwards-compatible for existing impls; only the runtime/codegen
side stays feature-gated.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
26d9249 to
068846b
Compare
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware reviewed 1 file and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on Yoni-Starkware).
TomerStarkware
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! all files reviewed, all discussions resolved (waiting on Yoni-Starkware).
Summary
Aligns
sierra-emu'sStarknetSyscallHandlertrait and supporting types withcairo-native's shape, so the two surfaces match name-for-name. This is a prerequisite for extracting them into a shared crate (next PR in the stack), which in turn lets us delete the bridge code that PR #1597 / #1607 introduced.Changes
sierra-emuadoptscairo-native's shapedeploy,library_call,call_contract,emit_event,send_message_to_l1,keccak,meta_tx_v0,cheatcodenow take&[Felt]/&[u64]rather than ownedVec. VM call sites invm/starknet.rsupdated to pass borrows.is_infinity: boolflag. The Sierra-level encoding insidesierra-emustill uses(0, 0)to represent the identity element —from_valuerecovers the flag from that sentinel;into_valuedrops it (callers that materialize the point at infinity already produce(0, 0), so round-tripping is preserved).sha256_process_blockmatches cairo-native:(&mut [u32; 8], &[u32; 16]) -> SyscallResult<()>(mutates in place, returns unit).ExecutionInfoV3/TxV3Infotypes + aget_execution_info_v3trait method (defaulted tounimplemented!()). The VM-level Sierra value lowering for v3 staystodo!()— wiring it is out of scope for this PR.cairo-nativecheatcodetrait method becomes unconditional. The runtime/codegen plumbing for cheatcode (vtable entry,wrap_cheatcode,cairo_native__vtable_cheatcode, thewith-cheatcodecargo feature) stays feature-gated as before — only the trait method comes out from behind thecfg. The default impl isunimplemented!().Trace dump
metadata::trace_dumppopulatesis_infinitywhen constructing the sierra-emu secp points (it was already reading the field from cairo-native'sSecp256Point).Backwards compatibility
cairo-native: backwards-compatible. The only public change is unconditionally exposingcheatcodeon the trait, with a defaultunimplemented!()impl — existing impls compile unchanged, callers compile unchanged, the trait stays object-safe. Cargo featurewith-cheatcodestill controls the runtime/codegen side.sierra-emu: breaking — slice-vs-Vec, the newis_infinityfield, sha256 sig change.sierra-emulives in this repo and isn't published as an external API surface, so the blast radius is contained.Stack
This is the first of four PRs replacing the bridge approach in #1597 / #1607:
cairo-starknet-syscallscrate; both crates re-export from it; the bridge (added in add SierraEmuSyscallBridge for cairo-native ↔ sierra-emu interop #1607) becomes unnecessary and is deleted.ContractExecutordispatch enum (supersedes add ContractExecutor dispatch enum (Aot + Emu) #1598 / add ContractExecutor dispatch enum (Aot + Emu) #1608); theEmuarm calls cairo-native's syscall handler directly, no bridge needed.run_with_libfunc_profile+AotWithProgram(supersedes add run_with_libfunc_profile + AotWithProgram variant for ContractExecutor #1599 / add run_with_libfunc_profile + AotWithProgram variant for ContractExecutor #1609).Test plan
cargo checkclean forcairo-native(default features,with-cheatcode,with-trace-dump)cargo check -p sierra-emuclean (lib + bins + tests)cargo check --workspace --all-featurescleanThis change is