extract shared cairo-native-syscalls crate#1611
Conversation
|
✅ Code is now correctly formatted. |
40ef87d to
7698d47
Compare
b3a574c to
319a6be
Compare
7698d47 to
a37920a
Compare
319a6be to
92ecf03
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 15 files and all commit messages, and made 4 comments.
Reviewable status: 15 of 21 files reviewed, 4 unresolved discussions (waiting on avi-starkware, TomerStarkware, and Yoni-Starkware).
cairo-native-syscalls/Cargo.toml line 2 at r1 (raw file):
[package] name = "cairo-native-syscalls"
cairo-starknet-syscalls maybe? the native here is completely irrelevant.
cairo-native-syscalls/src/lib.rs line 179 at r1 (raw file):
pub x: U256, pub y: U256, pub is_infinity: bool,
i still don't understand why is this bool needed.
Code quote:
pub is_infinity: bool,debug_utils/sierra-emu/src/starknet.rs line 138 at r1 (raw file):
_remaining_gas: &mut u64, ) -> SyscallResult<Vec<Felt>> { unimplemented!()
add reasoning in each unimplemented.
Code quote:
unimplemented!()
}
fn get_class_hash_at(
&mut self,
_contract_address: Felt,
_remaining_gas: &mut u64,
) -> SyscallResult<Felt> {
unimplemented!()
}
fn meta_tx_v0(
&mut self,
_address: Felt,
_entry_point_selector: Felt,
_calldata: &[Felt],
_signature: &[Felt],
_remaining_gas: &mut u64,
) -> SyscallResult<Vec<Felt>> {
unimplemented!()debug_utils/sierra-emu/src/starknet/value_conv.rs line 12 at r1 (raw file):
use crate::Value; pub fn u256_into_value(x: U256) -> Value {
doc all fns
a37920a to
63b478e
Compare
3b3e5af to
24f61ba
Compare
|
Previously, orizi wrote…
The |
Per @orizi review on #1611: each `unimplemented!()` in StubSyscallHandler should say *why* it's not implemented, not just that it isn't. Adds: - A scope doc-comment on the type explaining what is and isn't modeled. - A one-line rationale on each stub (deploy needs constructor execution, call_contract needs address-to-class resolution, send_message_to_l1 needs an L1 message queue, etc.). No behavior change.
24f61ba to
0ae0d06
Compare
Per @orizi review on #1611: each `unimplemented!()` in StubSyscallHandler should say *why* it's not implemented, not just that it isn't. Adds: - A scope doc-comment on the type explaining what is and isn't modeled. - A one-line rationale on each stub (deploy needs constructor execution, call_contract needs address-to-class resolution, send_message_to_l1 needs an L1 message queue, etc.). No behavior change.
0ae0d06 to
0791b36
Compare
|
Previously, orizi wrote…
I renamed it, but I am not sure So I agree these syscalls have nothing inherently "native" about them, but this trait is only relevant to cairo native (btw when implementing this trait in the |
|
Previously, orizi wrote…
Done. |
|
Previously, orizi wrote…
Done. |
63b478e to
26d9249
Compare
0791b36 to
4e80562
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi partially reviewed 8 files and all commit messages, made 2 comments, and resolved 4 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware and Yoni-Starkware).
cairo-native-syscalls/Cargo.toml line 2 at r1 (raw file):
Previously, avi-starkware (Avi Cohen) wrote…
I renamed it, but I am not sure
cairo-starknet-syscallsis a better name.
This syscall handler is only used by cairo-native (and cairo-native-related crates) because the cairo-vm processes syscalls through the hint processor and has a completely different flow.So I agree these syscalls have nothing inherently "native" about them, but this trait is only relevant to cairo native (btw when implementing this trait in the
sequencerrepo, it is aliasedNativeSyscallHandler), so it might be clearer to have a cairo-native prefix in its name.
you are literally working here to make it work for sierra-emulation - this still has nothing to do with native directly.
doesn't even matter if it is defined within the native workspace, same way that they didn't .
same way you don't call it cairo-native-sierra-emu.
Both cairo-native and sierra-emu now re-export the StarknetSyscallHandler
trait and supporting types (U256, Block/Tx/Execution Info v{1,2,3},
ResourceBounds, Secp256{k1,r1}Point) from the new
`cairo-starknet-syscalls` crate. The crate is workspace-local at the
top level (sibling to `test_utils/`).
The two crates' types are now nominally identical, so a single handler
impl can drive both runtimes without an adapter / bridge.
Notes:
- `cairo-starknet-syscalls` rather than `cairo-native-syscalls`: the
crate's content doesn't depend on cairo-native semantically; the
cairo-native prefix would imply ownership it doesn't have.
- StubSyscallHandler's `unimplemented!()` stubs now carry a one-line
rationale each (deploy needs constructor execution, call_contract
needs address-to-class resolution, send_message_to_l1 needs an L1
queue, etc.), plus a scope doc-comment on the type itself.
- sierra-emu's previous inherent into_value/from_value methods (which
depend on sierra-emu's `Value` enum and can't follow the moved types)
become free functions in the new sub-module
`sierra_emu::starknet::value_conv`, each with a doc comment.
- `cheatcode` trait method's default impl returns an error felt instead
of panicking with unimplemented!(). On the sierra-emu path, a contract
invoking the cheatcode libfunc against a handler that didn't override
it would otherwise unwind through the VM and abort the host;
production handlers (e.g. blockifier's NativeSyscallHandler) don't
implement cheatcode, so the default is what runs.
- The per-type files under debug_utils/sierra-emu/src/starknet/* are
deleted now that the struct definitions live in the shared crate.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4e80562 to
34f9e15
Compare
26d9249 to
068846b
Compare
Summary
Extracts the
StarknetSyscallHandlertrait and its supporting types into a new workspace-local crate,cairo-starknet-syscalls. Bothcairo-nativeandsierra-emure-export from it, so the two crates' types are now nominally identical — a single handler impl can drive both runtimes without an adapter.Replaces the bridge approach from #1597 / #1607: with this PR landed, the bridge file isn't necessary, and #1612's
Emuarm can call the cairo-native handler directly. Those follow-ups land in subsequent PRs in this stack.Layout
Sibling to
test_utils/at workspace root — fits the repo's existing convention for shared, non-binary, non-debug-tool crates.What moved
From
cairo-native/src/starknet.rsandsierra-emu/src/starknet/*into the new crate:U256,BlockInfo,TxInfo,TxV2Info,TxV3Info,ResourceBounds,ExecutionInfo,ExecutionInfoV2,ExecutionInfoV3,Secp256k1Point,Secp256r1Point.StarknetSyscallHandler, including its existing defaultcheatcodeimpl.Secp256k1Point::new/Secp256r1Point::new(pure-data).SyscallResult<T>.Repr / serde / derive layouts preserved exactly (
#[repr(C, align(16))]onU256,Secp256{k1,r1}Point).What stayed put
ArrayAbi,Felt252Abi,DummySyscallHandler, the runtime/codegen vtables (handler::StarknetSyscallHandlerCallbacks,cairo_native__vtable_cheatcode, etc.) — all cairo-native-specific.StubSyscallHandler,StubEvent,ContractLogs, and the VM eval logic.Value-conversion methods
Sierra-emu previously had
impl <Type> { fn into_value(self) -> Value, fn from_value(v: Value) -> Self }on each type. Inherent impls can't follow the type into a foreign crate, so these become free functions in a newsierra_emu::starknet::value_convmodule:U256::from_value(v)u256_from_value(v)u.into_value()u256_into_value(u)Secp256k1Point::from_value(v)secp256k1_point_from_value(v)info.into_value(felt_ty)execution_info_into_value(info, felt_ty)Call sites in
sierra-emu/src/vm/starknet.rsandcairo-native/src/metadata/trace_dump.rsupdated.Backwards compatibility
cairo_native::starknet::U256,…::StarknetSyscallHandler, etc. still resolve viapub use. Existing impls and callers compile unchanged. No semver-breaking change.into_value/from_valuemethods are gone. Sierra-emu is debug-only and not an externally published API surface; callers in this repo are all updated.debug_utils/sierra-emu/src/starknet/*are deleted now that the struct defs live in the shared crate.Stack
ContractExecutordispatch enum without the bridge (supersedes add ContractExecutor dispatch enum (Aot + Emu) #1598 / add ContractExecutor dispatch enum (Aot + Emu) #1608)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 check --workspace --all-featurescleancargo check -p sierra-emu --all-targetscleanThis change is