Skip to content

Refactor vm.Execute #3602

Merged
rodrodros merged 7 commits intomainfrom
granza/split-cairo-vm-endpoints
May 7, 2026
Merged

Refactor vm.Execute #3602
rodrodros merged 7 commits intomainfrom
granza/split-cairo-vm-endpoints

Conversation

@RafaelGranza
Copy link
Copy Markdown
Contributor

@RafaelGranza RafaelGranza commented May 5, 2026

Summary

Replaces the 7 positional boolean flags on vm.VM.Execute with typed, semantic wrappers, so callers describe what they're doing (Simulate / Trace / BuildBlock) instead of memorizing flag positions.
Also breaks down the method into smaller helpers.

Why

vm.Execute takes 7 positional bool parameters and is invoked from 8 production sites and ~38 test setups. The shape has caused real documentation drift:

Every existing call site falls into one of three semantic buckets:

  • Trace: replay an existing block (only returnInitialReads varies)
  • Simulate: RPC simulate_transactions / estimate_fee (5 flags vary)
  • BuildBlock: block production: builder + genesis (3 flags vary)

Notes for reviewers

  • No Rust / FFI signature changes (it was introducing more code/complexity than needed).
  • Execute still exists to bring more options if needed.
  • Execute was broken down into small helpers
  • Since I took a look at rust code, I removed unnecessary cloning and memory allocation from cair_vm_execute.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2026

Codecov Report

❌ Patch coverage is 78.33333% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.84%. Comparing base (605c6fe) to head (9e64a35).

Files with missing lines Patch % Lines
node/throttled_vm.go 10.34% 23 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3602      +/-   ##
==========================================
- Coverage   75.94%   75.84%   -0.10%     
==========================================
  Files         381      381              
  Lines       34275    34377     +102     
==========================================
+ Hits        26030    26074      +44     
- Misses       6421     6479      +58     
  Partials     1824     1824              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@RafaelGranza RafaelGranza force-pushed the granza/split-cairo-vm-endpoints branch from a0ce8f5 to 1cf6efc Compare May 5, 2026 12:14
@RafaelGranza RafaelGranza changed the base branch from granza/migrate-vm-execute-calls to main May 5, 2026 12:19
@RafaelGranza RafaelGranza self-assigned this May 5, 2026
@RafaelGranza RafaelGranza changed the title refactor: Split vm.execute into helpers Refactor ´vm.Execute´ May 5, 2026
@RafaelGranza RafaelGranza changed the title Refactor ´vm.Execute´ Refactor ´vm.Execute´ May 5, 2026
@RafaelGranza RafaelGranza changed the title Refactor ´vm.Execute´ Refactor vm.Execute May 5, 2026
@RafaelGranza RafaelGranza added Refactor go Pull requests that update Go code labels May 5, 2026
Comment thread vm/rust/src/entrypoint/execute/execute.rs
@RafaelGranza RafaelGranza marked this pull request as ready for review May 5, 2026 15:20
Copy link
Copy Markdown
Contributor

@rodrodros rodrodros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, some nitpicks

Comment thread genesis/genesis.go Outdated
Comment thread genesis/genesis.go Outdated
Comment thread rpc/v10/trace_test.go Outdated
Comment thread vm/vm.go Outdated
Comment thread vm/vm.go Outdated
Comment thread vm/vm.go Outdated
Comment thread vm/vm.go Outdated
Comment thread vm/vm.go Outdated
@rodrodros rodrodros mentioned this pull request May 7, 2026
2 tasks
@rodrodros rodrodros merged commit 1d0f6df into main May 7, 2026
26 checks passed
@rodrodros rodrodros deleted the granza/split-cairo-vm-endpoints branch May 7, 2026 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go Pull requests that update Go code Refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants