blockifier: add sierra-emu execution behind feature flag#13543
blockifier: add sierra-emu execution behind feature flag#13543avi-starkware wants to merge 1 commit intoavi/blockifier/contract-executor-abstractionfrom
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| y: convert_from_u256(x.y), | ||
| is_infinity: false, | ||
| } | ||
| } |
There was a problem hiding this comment.
Hardcoded is_infinity: false loses point-at-infinity state
Medium Severity
The convert_from_secp_256_k1_point and convert_from_secp_256_r1_point functions hardcode is_infinity: false when converting from sierra-emu to cairo-native point types. Since sierra-emu's point types only have x and y fields (no infinity flag), the point at infinity is represented as (0, 0). When this is converted back to a cairo_native point with is_infinity: false, the downstream ark-ec operations will treat it as the literal finite point (0, 0) — which is not on the secp256k1/r1 curves — rather than the identity element. This causes incorrect results for elliptic curve add and mul operations involving the point at infinity.
Additional Locations (1)
350d48a to
3816a9e
Compare
103c31b to
bc8a9d8
Compare
3816a9e to
8470659
Compare
bc8a9d8 to
830c7a4
Compare
8470659 to
efa9bd7
Compare
830c7a4 to
bd48103
Compare
Yoni-Starkware
left a comment
There was a problem hiding this comment.
Are you sure this must be in our repo?
@Yoni-Starkware made 1 comment.
Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on avi-starkware, noaov1, and TomerStarkware).
bd48103 to
b327019
Compare
efa9bd7 to
05d7201
Compare
b327019 to
e40fc15
Compare
f16bb25 to
f83e2b7
Compare
e40fc15 to
992d84c
Compare
Yoni-Starkware
left a comment
There was a problem hiding this comment.
@Yoni-Starkware made 1 comment.
Reviewable status: 0 of 5 files reviewed, 2 unresolved discussions (waiting on avi-starkware, noaov1, and TomerStarkware).
crates/blockifier/src/execution/native/executor.rs line 1 at r3 (raw file):
#[cfg(feature = "with-trace-dump")]
I think that this logic could be implemented on Native repo. Please try this approach.
992d84c to
6a272ae
Compare
f83e2b7 to
0a80afe
Compare
PR SummaryMedium Risk Overview Adds optional trace dumping via Reviewed by Cursor Bugbot for commit db24055. Bugbot is set up for automated code reviews on this repo. Configure here. |
6a272ae to
ceacfac
Compare
0a80afe to
09dda81
Compare
ceacfac to
eab2995
Compare
09dda81 to
7d75d71
Compare
7d75d71 to
5be30be
Compare
eab2995 to
2628a2d
Compare
2628a2d to
eb1a633
Compare
5be30be to
73734ae
Compare
Adds sierra-emu as an alternative execution backend, gated behind the \`sierra-emu\` feature flag. Also adds \`with-trace-dump\` and \`with-libfunc-profiling\` feature flags for execution tracing and profiling support. When no new flags are enabled, behavior is identical to upstream.
73734ae to
db24055
Compare
eb1a633 to
db5d4f9
Compare



Adds sierra-emu as an alternative execution backend, gated behind the
`sierra-emu` feature flag. Also adds `with-trace-dump` and
`with-libfunc-profiling` feature flags for execution tracing and
profiling support.
When no new flags are enabled, behavior is identical to upstream.
Note
Medium Risk
Execution-path changes and new unsafe/IO-based instrumentation are introduced, but they are gated behind feature flags so default behavior should remain unchanged. Risk is mainly in correctness/compatibility when enabling
sierra-emu,with-trace-dump, orwith-libfunc-profiling.Overview
Adds an alternative contract execution backend in
blockifierbehind thesierra-emufeature flag by extendingContractExecutorto run viasierra-emu::VirtualMachineand wiring a syscall-handler adapter plus type conversions.Introduces optional instrumentation features:
with-trace-dump(writes execution traces totraces/nativeortraces/emu) andwith-libfunc-profiling(collects libfunc profiles into a globalLIBFUNC_PROFILES_MAPkeyed by tx hash), enabled by using anAotWithProgramexecutor variant.Updates workspace/crate manifests and lockfile to add the
sierra-emudependency and related transitive crypto deps, and addsNativeCompiledClassV1::new_from_executorto support constructing compiled classes from the new executor variants.Reviewed by Cursor Bugbot for commit f83e2b7. Bugbot is set up for automated code reviews on this repo. Configure here.