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) |
10.849 ± 0.044 | 10.774 | 10.901 | 5.73 ± 0.06 |
cairo-native (embedded AOT) |
1.892 ± 0.018 | 1.859 | 1.918 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1.933 ± 0.028 | 1.888 | 1.959 | 1.02 ± 0.02 |
Benchmark for program dict_snapshot
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
541.9 ± 8.3 | 530.1 | 558.2 | 1.00 |
cairo-native (embedded AOT) |
1692.7 ± 15.0 | 1672.8 | 1724.6 | 3.12 ± 0.06 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1717.9 ± 17.4 | 1695.6 | 1741.3 | 3.17 ± 0.06 |
Benchmark for program factorial_2M
Open benchmarks
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.907 ± 0.117 | 4.852 | 5.236 | 2.28 ± 0.06 |
cairo-native (embedded AOT) |
2.151 ± 0.023 | 2.106 | 2.179 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2.174 ± 0.018 | 2.154 | 2.204 | 1.01 ± 0.01 |
Benchmark for program fib_2M
Open benchmarks
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.812 ± 0.030 | 4.782 | 4.887 | 2.79 ± 0.03 |
cairo-native (embedded AOT) |
1.730 ± 0.028 | 1.692 | 1.789 | 1.00 ± 0.02 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1.726 ± 0.014 | 1.703 | 1.752 | 1.00 |
Benchmark for program linear_search
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
591.8 ± 7.5 | 579.3 | 601.5 | 1.00 |
cairo-native (embedded AOT) |
1700.9 ± 15.0 | 1680.5 | 1732.8 | 2.87 ± 0.04 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1750.3 ± 16.8 | 1716.3 | 1780.4 | 2.96 ± 0.05 |
Benchmark for program logistic_map
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
503.9 ± 6.5 | 495.7 | 514.8 | 1.00 |
cairo-native (embedded AOT) |
1885.5 ± 26.3 | 1861.8 | 1940.1 | 3.74 ± 0.07 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1976.2 ± 12.4 | 1957.0 | 1990.5 | 3.92 ± 0.06 |
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. |
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).
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>
068846b to
b757ad4
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 4 files and all commit messages.
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