Introduce semantic wrappers around Execute#3595
Conversation
| // SimulateOptions carries the per-request flags relevant to RPC simulate / | ||
| // estimateFee. errStack and allowBinarySearch are constants for this path | ||
| // and are set internally. |
There was a problem hiding this comment.
The documentation talking about flags that are constants or pre-defined will be removed after these methods stop depending on Execute
| // TraceOptions carries the per-request flags relevant to replaying an | ||
| // existing block. All execution flags are off; only errStack is on. | ||
| type TraceOptions struct { | ||
| ReturnInitialReads bool | ||
| } |
There was a problem hiding this comment.
I know it could be a single bollean, but I decided to keep this as a struct, for 2 reasons:
- it keeps the same signature pattern as the other wrappers
- if we ever need extra flags, adding a field to this struct is less conflicting than adding and extra argument to a method.
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #3595 +/- ##
==========================================
- Coverage 76.00% 75.94% -0.07%
==========================================
Files 381 381
Lines 34275 34294 +19
==========================================
- Hits 26052 26043 -9
- Misses 6401 6430 +29
+ Partials 1822 1821 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Move the changes to here: #3602 |
2 participants
Summary
First of four incremental PRs refactoring
vm.Execute(go) andcairo_vm_execute(rust) from a 7-positional-boolean signature into three semantic operations with typed options. This PR is purely additive. No call sites are migrated yet, andExecuteremains fully functional.Why
vm.Executetakes 7 positionalboolparameters and is invoked from 8 production sites and ~38 test setups. The shape has caused real documentation drift:rpc/v9/trace.goandrpc/v10/trace.godescribe parameters that don't exist in the signature (// skipNonceCharge,// allowZeroMaxFee,// allowNoSignature— none are realExecuteparams).Every existing call site falls into one of three semantic buckets:
returnInitialReadsvaries)simulate_transactions/estimate_fee(5 flags vary)Roadmap
Simulate/Trace/BuildBlockwrappers + options structscairo_vm_executeinto three dedicated entry points; wrappers call them directlyExecute(Go interface,*vm,ThrottledVM, mock) andcairo_vm_execute(Rust + FFI header)Changes in this PR
vm/vm.go: addsSimulateOptions,TraceOptions,BuildBlockOptionsand the three corresponding methods on theVMinterface and*vmimplementation. The methods delegate toExecutefor now.node/throttled_vm.go: mirrors the three methods with throttle wrappers.mocks/mock_vm.go: adds gomock implementations.Out of scope (covered by follow-up PRs)
Executeis still the method actually being invoked everywhere in production and tests.Execute. These will be replaced in PR 3 when each method gets its own Rust entry point and those flags cease to exist as configuration.