blockifier: adopt cairo_native::ContractExecutor for AOT + sierra-emu dispatch#13880
blockifier: adopt cairo_native::ContractExecutor for AOT + sierra-emu dispatch#13880avi-starkware wants to merge 5 commits into
Conversation
PR SummaryHigh Risk Overview Adds feature-gated libfunc profiling: Switches workspace Reviewed by Cursor Bugbot for commit f72014c. Bugbot is set up for automated code reviews on this repo. Configure here. |
Yoni-Starkware
left a comment
There was a problem hiding this comment.
@Yoni-Starkware made 1 comment.
Reviewable status: 0 of 8 files reviewed, 2 unresolved discussions (waiting on avi-starkware and TomerStarkware).
crates/blockifier/src/execution/native/entry_point_execution.rs line 104 at r2 (raw file):
#[cfg(not(feature = "with-libfunc-profiling"))] let execution_result = compiled_class.executor.run( selector,
Suggestion:
let selector = entry_point.selector.0;
let calldata = syscall_handler.base.call.calldata.0.clone();
#[cfg(feature = "with-libfunc-profiling")]
let execution_result = run_with_profile(
selector,
&calldata,
call_initial_gas,
Some(builtin_costs),
&mut syscall_handler,
on_profile
)
}
#[cfg(not(feature = "with-libfunc-profiling"))]
let execution_result = compiled_class.executor.run(
selector,9630be2 to
88f60d0
Compare
|
I extracted the entire logic (including the feature gate) into |
Yoni-Starkware
left a comment
There was a problem hiding this comment.
@Yoni-Starkware partially reviewed 5 files, made 1 comment, and resolved 1 discussion.
Reviewable status: 2 of 9 files reviewed, 1 unresolved discussion (waiting on avi-starkware and TomerStarkware).
Yoni-Starkware
left a comment
There was a problem hiding this comment.
@Yoni-Starkware partially reviewed 7 files and made 1 comment.
Reviewable status: 8 of 9 files reviewed, 1 unresolved discussion (waiting on avi-starkware and TomerStarkware).
Yoni-Starkware
left a comment
There was a problem hiding this comment.
@Yoni-Starkware reviewed 1 file and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on avi-starkware and TomerStarkware).
88f60d0 to
7f89015
Compare
| # (starkware-libs/cairo_native#1610 → #1611 → #1612 → #1613) containing | ||
| # `ContractExecutor`, `EmuContractInfo`, `AotWithProgram`, and `run_with_profile`. | ||
| # Switch back to a crates.io version once those land in a published cairo-native release. | ||
| cairo-native = { git = "https://github.com/starkware-libs/cairo_native.git", rev = "78bf3e4b01a085261f61367c29391e98deb98460" } |
There was a problem hiding this comment.
Unpublished git dependency committed for production use
Medium Severity
The cairo-native dependency is pinned to a specific git revision from an unreleased PR stack instead of a published crates.io version. The comment says "TEMP" and the PR description warns not to merge until upstream lands, but this temporary git dep could easily be forgotten and shipped. A production dependency on an unpublished git rev risks reproducibility issues and blocks downstream consumers who can't resolve the git source.
Reviewed by Cursor Bugbot for commit 7f89015. Configure here.
There was a problem hiding this comment.
Acknowledged — this is the intended merge gate, not a forgotten TEMP. The git rev pins the tip of the cairo_native PR stack (#1610 → #1611 → #1607 → #1612 → #1613); merge of this PR is blocked on that stack landing in a published cairo-native release. The # TEMP: block above the dep line names the upstream PRs and the planned crates.io swap.
7f89015 to
8d7bbc6
Compare
8d7bbc6 to
bfe58c8
Compare
Yoni-Starkware
left a comment
There was a problem hiding this comment.
Rebase over main?
@Yoni-Starkware made 1 comment.
Reviewable status: 0 of 9 files reviewed, 2 unresolved discussions (waiting on avi-starkware and TomerStarkware).
bfe58c8 to
c7a3fac
Compare
a962748 to
441098f
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
|
Done. |
…utor + libfunc profiling Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
c7a3fac to
52cfdf6
Compare
Cargo accepts both git+rev and a version field together; the field is a SemVer constraint on the package at the git rev. The rev's Cargo.toml already pins 0.9.0-rc.6, matching native_compiler_version.txt, so the constants test required_cairo_native_version_test parses the version and succeeds. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c8cca67. Configure here.
- Drop unused `NativeCompiledClassV1::new_from_emu`; it had no in-tree callers, and the sierra-emu construction path will be re-added when the benchmarking/replay tool that needs it lands. - Revert `process_compilation_request`'s `.map(...)` shape to a direct `match`, moving `casm` into `NativeCompiledClassV1::new(_with_program)` in the `Ok` arm at zero cost instead of cloning it. - Bump pinned cairo-native rev to pick up `cargo fmt` fix on starkware-libs/cairo_native#1613. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cairo_native PRs #1610–#1612 were rebased onto a newer cairo_native main (now at 2463ca0). Rebase libfunc-profiling-2 (#1613) onto the new contract-executor-2 (#1612) and update the pinned rev accordingly. No content change in the libfunc-profiling-2 commit itself. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`new_from_emu` was deleted in 88b68af after Cursor flagged it as unused. Restore it (with a doc comment naming the planned consumer) so the benchmarking / replay tooling — which will live as a feature branch and is not expected to merge here — can reach it without patching this file. No `#[expect(dead_code)]` needed: `dead_code` doesn't fire on `pub fn` in a library crate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>



Summary
Replaces the local
AotContractExecutorfield onNativeCompiledClassV1Innerwith cairo-native's newContractExecutorenum and adopts cairo-native's new libfunc-profiling APIs. The conversion glue between cairo-native and sierra-emu syscall traits, and the libfunc profiling instrumentation — both previously maintained in blockifier — now live in cairo-native. Sierra-program extraction for profiling moves intoapollo_compile_to_native::SierraToNativeCompiler::compile_with_program, so the blockifier no longer touchescairo-lang-sierraat all.Important
Depends on the unreleased cairo_native PR stack (in order):
StarknetSyscallHandlertrait/types with cairo-nativecairo-native-syscallscrateSierraEmuSyscallBridgefor cairo-native ↔ sierra-emu interopContractExecutorenum (Aot+Emu) +EmuContractInforun_with_libfunc_profile+AotWithProgram+ContractExecutor::run_with_profile(.., FnOnce(Profile, Arc<Program>))+pub use cairo_lang_sierra::program::Program;re-exportThe workspace
Cargo.tomlcurrently uses agit/revdep pinned to the tip of #1613. Do not merge until that stack lands and a cairo-native release is published, then swap thegit/revdep back to a publishedversion = "...".Commits
blockifier,apollo_compile_to_native: adopt cairo_native::ContractExecutor + libfunc profiling— switchesexecutor: AotContractExecutor→executor: ContractExecutor; addssierra-emu+with-libfunc-profilingfeatures; addsNativeCompiledClassV1::new_with_program(AotWithProgram, _); addsSierraToNativeCompiler::compile_with_programsoprocess_compilation_requestdoes compile + Sierra-program extraction in one call; routesentry_point_executionthroughrun_with_profileviarun_dispatch::run_native_executor.workspace: declare cairo-native version on git dep for version sync test— required forrequired_cairo_native_version_testto parse the pinned dep.blockifier: address review feedback on native compilation path— revertsprocess_compilation_requestto a directmatchsocasmmoves intoNativeCompiledClassV1::new(_with_program)at zero cost; bumps the pinned cairo-native rev for thecargo fmtfix on fix: add abi to protobuf to deprecated_contact_class conversion #1613.workspace: bump cairo-native rev to rebased libfunc-profiling-2 tip— picks up the rebase of fix: add abi to protobuf to deprecated_contact_class conversion #1613 onto the new fix: add program to protobuf to deprecated_contract_class conversion #1612 head after refactor(batcher): block builder test, add empty block case #1610/chore: implement decode_and_decompress + test #1611/fix: add program to protobuf to deprecated_contract_class conversion #1612 were rebased onto a newer cairo-native main.blockifier: re-add new_from_emu for out-of-tree benchmarking branch— restores theNativeCompiledClassV1::new_from_emu(EmuContractInfo, _)constructor so the out-of-tree benchmarking / replay feature branch (which is not expected to merge here) can construct sierra-emu-backed compiled classes without patching this file.Replaces
The (still-open) #13542 / #13543 / #13755 stack. The
ContractExecutorenum, the ~340 lines of sierra-emu syscall conversions, and the libfunc profiling primitive (ProfilerGuard,Arc<Program>plumbing, trace_id mutation) all move out of blockifier and into cairo-native, where they belong (per @Yoni-Starkware's review on #13543).Test plan
--features cairo_native--features cairo_native,sierra-emu--features cairo_native,with-libfunc-profiling--features cairo_native,sierra-emu,with-libfunc-profilinggit/revdep back to a crates.ioversion = "..."and re-run CI🤖 Generated with Claude Code