fix: vm fatal errors - #21
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughThe executor aligns nested storage modes with ChangesNested storage state alignment
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to This change is merge-ready after normal checks and review; no actionable merge-blocking risk remains in the supplied evidence. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
executor/src/rt/mod.rs (1)
145-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the non-nested budget.
The test covers only
DetFuelBudget::new(Some(..)). TheNonecase is the top-level production path:remainingmust return the host value unchanged, andconsumemust not change that.🧪 Proposed additional test
#[tokio::test] async fn imported_deterministic_fuel_is_the_initial_budget() { let budget = DetFuelBudget::new(Some(primitive_types::U256::from(10))); assert_eq!( budget.remaining(primitive_types::U256::from(20)).await, primitive_types::U256::from(10) ); budget.consume(primitive_types::U256::from(3)).await; assert_eq!( budget.remaining(primitive_types::U256::from(20)).await, primitive_types::U256::from(7) ); } + + #[tokio::test] + async fn absent_budget_passes_host_fuel_through() { + let budget = DetFuelBudget::new(None); + assert_eq!( + budget.remaining(primitive_types::U256::from(20)).await, + primitive_types::U256::from(20) + ); + budget.consume(primitive_types::U256::from(5)).await; + assert_eq!( + budget.remaining(primitive_types::U256::from(20)).await, + primitive_types::U256::from(20) + ); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@executor/src/rt/mod.rs` around lines 145 - 162, Add a test alongside imported_deterministic_fuel_is_the_initial_budget covering DetFuelBudget::new(None); verify remaining returns the supplied host value unchanged before and after consume, confirming consume has no effect for the non-nested budget.executor/src/exe/run.rs (1)
316-317: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
memory_limitis accepted from the envelope without a local ceiling.
remaining_recursioninexecutor/src/lib.rs(Lines 401-405) clamps the supplied value with.min(public_abi::top_limits::VM_RECURSION), and the comment there states that a budget minted elsewhere is a remainder, not an authority.memory_limittakes the envelope value directly.The current effect is bounded, because the non-nested default is already
u32::MAX. A caller can therefore only fail to lower the limit, not raise it beyond today's top-level default. Apply the same clamp so the two limits follow one rule if a local memory ceiling is introduced later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@executor/src/exe/run.rs` around lines 316 - 317, Clamp the envelope-provided memory_limit in the run setup to public_abi::top_limits::VM_MEMORY (or the established memory top-limit symbol), matching the remaining_recursion handling in remaining_recursion. Preserve the existing nested lookup and u32::MAX default while ensuring supplied values cannot exceed the local ceiling.executor/src/wasi/genlayer_sdk.rs (1)
821-838: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBoth
CallContractpaths perform work the other path discards.
resolve_callcontract_executor(Lines 826-831) runs on everyCallContract, including the common same-executor call whererouting_payloadisNone. That adds one host round trip to the in-process path.
check_major_and_resolve_code_slot(Lines 833-838) also runs before the routing branch. It is a storage read, and on the routed path itscode_slotfeeds onlyvm_data.conf.topmost_runner_id, which the routed branch never uses — the envelope sends the literalNestedRunnerId("contract")instead.Move
check_major_and_resolve_code_slotand thevm_dataconstruction into therouting_payload.is_none()branch. Each path then pays only for the work it uses.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@executor/src/wasi/genlayer_sdk.rs` around lines 821 - 838, The CallContract flow performs unnecessary host and storage work before branching. Keep resolve_callcontract_executor for routing determination, but move check_major_and_resolve_code_slot and the dependent vm_data construction into the routing_payload.is_none() branch; ensure the routed branch continues using its existing literal NestedRunnerId("contract") envelope value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@executor/src/lib.rs`:
- Around line 193-212: Update convert_nested_permissions to reject forbidden
NestedPermissions bits before constructing wasi::base::Config, matching the
validation performed by handle in executor/src/exe/run.rs. Ensure WRITE_STORAGE
and SEND_MESSAGES are rejected for nested calls rather than mapped into the
child configuration, while preserving conversion of allowed permissions.
In `@executor/src/wasi/genlayer_sdk.rs`:
- Line 986: The nested memory limit currently crosses the executor boundary
without a shared ceiling. In executor/src/wasi/genlayer_sdk.rs:986, send a
reduced memory share or document the manager-side aggregate ceiling that makes
the full remaining value safe; in executor/src/exe/run.rs:316-317, clamp
n.memory_limit to a local maximum before passing it to create_supervisor,
following the existing remaining_recursion clamp pattern.
---
Nitpick comments:
In `@executor/src/exe/run.rs`:
- Around line 316-317: Clamp the envelope-provided memory_limit in the run setup
to public_abi::top_limits::VM_MEMORY (or the established memory top-limit
symbol), matching the remaining_recursion handling in remaining_recursion.
Preserve the existing nested lookup and u32::MAX default while ensuring supplied
values cannot exceed the local ceiling.
In `@executor/src/rt/mod.rs`:
- Around line 145-162: Add a test alongside
imported_deterministic_fuel_is_the_initial_budget covering
DetFuelBudget::new(None); verify remaining returns the supplied host value
unchanged before and after consume, confirming consume has no effect for the
non-nested budget.
In `@executor/src/wasi/genlayer_sdk.rs`:
- Around line 821-838: The CallContract flow performs unnecessary host and
storage work before branching. Keep resolve_callcontract_executor for routing
determination, but move check_major_and_resolve_code_slot and the dependent
vm_data construction into the routing_payload.is_none() branch; ensure the
routed branch continues using its existing literal NestedRunnerId("contract")
envelope value.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 978c5b0c-4890-46d0-bf21-98fb5927df0a
⛔ Files ignored due to path filters (314)
executor/Cargo.lockis excluded by!**/*.lock,!**/*.lockexecutor/crates/common/Cargo.lockis excluded by!**/*.lock,!**/*.lockexecutor/crates/modules-interfaces/Cargo.lockis excluded by!**/*.lock,!**/*.locktests/integration/prompt/json_random/json_random.jsonnetis excluded by!tests/**tests/integration/stable/agentic/wasi/environ_args/environ_args.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/agentic/wasi/environ_args/environ_args.jsonnetis excluded by!tests/**tests/integration/stable/agentic/wasi/hash_random/hash_random.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/agentic/wasi/hash_random/hash_random.jsonnetis excluded by!tests/**tests/integration/stable/agentic/wasi/id_repr/id_repr.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/agentic/wasi/id_repr/id_repr.jsonnetis excluded by!tests/**tests/integration/stable/agentic/wasi/set_order/set_order.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/agentic/wasi/set_order/set_order.jsonnetis excluded by!tests/**tests/integration/stable/agentic/wasi/wasi_clock/wasi_clock.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/agentic/wasi/wasi_clock/wasi_clock.jsonnetis excluded by!tests/**tests/integration/stable/agentic/wasi/wasi_random/wasi_random.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/agentic/wasi/wasi_random/wasi_random.jsonnetis excluded by!tests/**tests/integration/stable/bench/read_tree_map.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/bench/read_tree_map.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/bench/read_tree_map.jsonnetis excluded by!tests/**tests/integration/stable/exploits/call_wasi_extra.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/exploits/call_wasi_extra.jsonnetis excluded by!tests/**tests/integration/stable/exploits/disagree_in_sandbox.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/exploits/disagree_in_sandbox.jsonnetis excluded by!tests/**tests/integration/stable/exploits/flt.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/exploits/flt.jsonnetis excluded by!tests/**tests/integration/stable/exploits/fork_bomb.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/exploits/fork_bomb.jsonnetis excluded by!tests/**tests/integration/stable/exploits/inf-loop.jsonnetis excluded by!tests/**tests/integration/stable/exploits/method_init.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/exploits/method_init.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/exploits/method_init.jsonnetis excluded by!tests/**tests/integration/stable/exploits/method_private.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/exploits/method_private.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/exploits/method_private.jsonnetis excluded by!tests/**tests/integration/stable/exploits/oom.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/exploits/oom.jsonnetis excluded by!tests/**tests/integration/stable/exploits/rec.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/exploits/rec.jsonnetis excluded by!tests/**tests/integration/stable/exploits/rec_1023.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/exploits/rec_1023.jsonnetis excluded by!tests/**tests/integration/stable/exploits/rec_1024.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/exploits/rec_1024.jsonnetis excluded by!tests/**tests/integration/stable/exploits/rec_tail.jsonnetis excluded by!tests/**tests/integration/stable/exploits/storage_rw_long.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/exploits/storage_rw_long.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/exploits/storage_rw_long.jsonnetis excluded by!tests/**tests/integration/stable/exploits/unreachable.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/exploits/unreachable.jsonnetis excluded by!tests/**tests/integration/stable/nondet/leader_errors/leader_no_nondet.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/leader_errors/leader_no_nondet.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/leader_errors/leader_no_nondet.jsonnetis excluded by!tests/**tests/integration/stable/nondet/leader_errors/simple_leader.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/leader_errors/simple_leader.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/leader_errors/simple_leader.jsonnetis excluded by!tests/**tests/integration/stable/nondet/leader_errors/simple_valid_err_err.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/leader_errors/simple_valid_err_err.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/leader_errors/simple_valid_err_err.jsonnetis excluded by!tests/**tests/integration/stable/nondet/leader_errors/simple_valid_err_exit.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/leader_errors/simple_valid_err_exit.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/leader_errors/simple_valid_err_exit.jsonnetis excluded by!tests/**tests/integration/stable/nondet/leader_errors/simple_valid_err_exit_wrong_err.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/leader_errors/simple_valid_err_exit_wrong_err.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/leader_errors/simple_valid_err_exit_wrong_err.jsonnetis excluded by!tests/**tests/integration/stable/nondet/leader_errors/simple_valid_err_nerr.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/leader_errors/simple_valid_err_nerr.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/leader_errors/simple_valid_err_nerr.jsonnetis excluded by!tests/**tests/integration/stable/nondet/metod_det_get_webpage.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/metod_det_get_webpage.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/metod_det_get_webpage.jsonnetis excluded by!tests/**tests/integration/stable/nondet/trivial.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/trivial.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/trivial.jsonnetis excluded by!tests/**tests/integration/stable/nondet/validator/rollback_agree.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/validator/rollback_agree.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/validator/rollback_agree.jsonnetis excluded by!tests/**tests/integration/stable/nondet/validator/rollback_disagree.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/validator/rollback_disagree.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/validator/rollback_disagree.jsonnetis excluded by!tests/**tests/integration/stable/nondet/validator/rollback_imm.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/validator/rollback_imm.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/validator/rollback_imm.1.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/validator/rollback_imm.1_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/validator/rollback_imm.jsonnetis excluded by!tests/**tests/integration/stable/nondet/validator/sync.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/validator/sync.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/validator/sync.jsonnetis excluded by!tests/**tests/integration/stable/nondet/validator/sync_err.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/validator/sync_err.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/nondet/validator/sync_err.jsonnetis excluded by!tests/**tests/integration/stable/py/balances/balance.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/balances/balance.jsonnetis excluded by!tests/**tests/integration/stable/py/balances/balance_eth.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/balances/balance_eth.jsonnetis excluded by!tests/**tests/integration/stable/py/balances/sandbox_overspend.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/balances/sandbox_overspend.jsonnetis excluded by!tests/**tests/integration/stable/py/balances/sandbox_overspend_2.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/balances/sandbox_overspend_2.jsonnetis excluded by!tests/**tests/integration/stable/py/balances/undefined_all.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/balances/undefined_all.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/balances/undefined_all.0_0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/balances/undefined_all.jsonnetis excluded by!tests/**tests/integration/stable/py/balances/undefined_method.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/balances/undefined_method.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/balances/undefined_method.0_0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/balances/undefined_method.jsonnetis excluded by!tests/**tests/integration/stable/py/balances/undefined_method_payable.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/balances/undefined_method_payable.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/balances/undefined_method_payable.0_0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/balances/undefined_method_payable.jsonnetis excluded by!tests/**tests/integration/stable/py/balances/undefined_receive.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/balances/undefined_receive.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/balances/undefined_receive.0_0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/balances/undefined_receive.jsonnetis excluded by!tests/**tests/integration/stable/py/embeddings/simple.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/embeddings/simple.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/embeddings/simple.jsonnetis excluded by!tests/**tests/integration/stable/py/embeddings/simple_det.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/embeddings/simple_det.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/embeddings/simple_det.jsonnetis excluded by!tests/**tests/integration/stable/py/embeddings/simple_tokenizer.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/embeddings/simple_tokenizer.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/embeddings/simple_tokenizer.jsonnetis excluded by!tests/**tests/integration/stable/py/embeddings/vecdb.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/embeddings/vecdb.jsonnetis excluded by!tests/**tests/integration/stable/py/events/post_event.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/events/post_event.jsonnetis excluded by!tests/**tests/integration/stable/py/intercontract/call_view.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/intercontract/call_view.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/intercontract/call_view.0_0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/intercontract/call_view.jsonnetis excluded by!tests/**tests/integration/stable/py/intercontract/call_view_iface.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/intercontract/call_view_iface.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/intercontract/call_view_iface.0_0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/intercontract/call_view_iface.jsonnetis excluded by!tests/**tests/integration/stable/py/intercontract/deploy.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/intercontract/deploy.jsonnetis excluded by!tests/**tests/integration/stable/py/intercontract/deploy_salt.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/intercontract/deploy_salt.jsonnetis excluded by!tests/**tests/integration/stable/py/intercontract/send_message.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/intercontract/send_message.jsonnetis excluded by!tests/**tests/integration/stable/py/intercontract/send_message_eth.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/intercontract/send_message_eth.jsonnetis excluded by!tests/**tests/integration/stable/py/intercontract/send_message_on.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/intercontract/send_message_on.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/intercontract/send_message_on.0_0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/intercontract/send_message_on.jsonnetis excluded by!tests/**tests/integration/stable/py/other/meth/method_init.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/meth/method_init.jsonnetis excluded by!tests/**tests/integration/stable/py/other/meth/method_init_wrong_name.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/meth/method_init_wrong_name.jsonnetis excluded by!tests/**tests/integration/stable/py/other/meth/method_public.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/meth/method_public.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/meth/method_public.jsonnetis excluded by!tests/**tests/integration/stable/py/other/meth/method_retn.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/meth/method_retn.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/meth/method_retn.jsonnetis excluded by!tests/**tests/integration/stable/py/other/meth/method_retn_view.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/meth/method_retn_view.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/meth/method_retn_view.jsonnetis excluded by!tests/**tests/integration/stable/py/other/meth/method_rollback.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/meth/method_rollback.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/meth/method_rollback.jsonnetis excluded by!tests/**tests/integration/stable/py/other/ret/returns.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.1.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.1_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.2.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.2_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.3.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.3_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.4.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.4_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.5.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.5_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.6.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.6_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.7.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.7_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.8.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.8_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.9.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.9_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/other/ret/returns.jsonnetis excluded by!tests/**tests/integration/stable/py/pitfalls/error_msg.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/pitfalls/error_msg.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/pitfalls/error_msg.jsonnetis excluded by!tests/**tests/integration/stable/py/pitfalls/error_msg_overridden.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/pitfalls/error_msg_overridden.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/pitfalls/error_msg_overridden.jsonnetis excluded by!tests/**tests/integration/stable/py/pitfalls/multi_contract.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/pitfalls/multi_contract.jsonnetis excluded by!tests/**tests/integration/stable/py/pitfalls/pub_ctor.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/pitfalls/pub_ctor.jsonnetis excluded by!tests/**tests/integration/stable/py/pitfalls/store_proxy.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/pitfalls/store_proxy.jsonnetis excluded by!tests/**tests/integration/stable/py/rollbacks/call_view.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/rollbacks/call_view.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/rollbacks/call_view.0_0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/rollbacks/call_view.jsonnetis excluded by!tests/**tests/integration/stable/py/rollbacks/nondet.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/rollbacks/nondet.jsonnetis excluded by!tests/**tests/integration/stable/py/rollbacks/simple.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/rollbacks/simple.jsonnetis excluded by!tests/**tests/integration/stable/py/sandbox/det/s/assign-json.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/det/s/assign-json.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/det/s/assign-json.jsonnetis excluded by!tests/**tests/integration/stable/py/sandbox/det/s/exit.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/det/s/exit.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/det/s/exit.jsonnetis excluded by!tests/**tests/integration/stable/py/sandbox/det/s/print.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/det/s/print.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/det/s/print.jsonnetis excluded by!tests/**tests/integration/stable/py/sandbox/det/s/rollback.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/det/s/rollback.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/det/s/rollback.jsonnetis excluded by!tests/**tests/integration/stable/py/sandbox/det/s/sandbox.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/det/s/sandbox.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/det/s/sandbox.jsonnetis excluded by!tests/**tests/integration/stable/py/sandbox/det/sandbox_write.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/det/sandbox_write.jsonnetis excluded by!tests/**tests/integration/stable/py/sandbox/non-det/s/assign-json.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/non-det/s/assign-json.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/non-det/s/assign-json.jsonnetis excluded by!tests/**tests/integration/stable/py/sandbox/non-det/s/exit.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/non-det/s/exit.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/non-det/s/exit.jsonnetis excluded by!tests/**tests/integration/stable/py/sandbox/non-det/s/print.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/non-det/s/print.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/non-det/s/print.jsonnetis excluded by!tests/**tests/integration/stable/py/sandbox/non-det/s/rollback.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/non-det/s/rollback.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/sandbox/non-det/s/rollback.jsonnetis excluded by!tests/**tests/integration/stable/py/schemas/complex_types.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/schemas/complex_types.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/schemas/complex_types.jsonnetis excluded by!tests/**tests/integration/stable/py/schemas/prim_types.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/schemas/prim_types.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/schemas/prim_types.jsonnetis excluded by!tests/**tests/integration/stable/py/schemas/ret-float.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/schemas/ret-float.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/schemas/ret-float.jsonnetis excluded by!tests/**tests/integration/stable/py/schemas/ret-tuple.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/schemas/ret-tuple.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/schemas/ret-tuple.jsonnetis excluded by!tests/**tests/integration/stable/py/schemas/ret.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/schemas/ret.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/schemas/ret.jsonnetis excluded by!tests/**tests/integration/stable/py/schemas/trivial.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/schemas/trivial.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/py/schemas/trivial.jsonnetis excluded by!tests/**tests/integration/stable/runners/dup-dependency.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/runners/dup-dependency.jsonnetis excluded by!tests/**tests/integration/stable/runners/env-template.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/runners/env-template.jsonnetis excluded by!tests/**tests/integration/stable/runners/lock/lock.jsonnetis excluded by!tests/**tests/integration/stable/runners/malformed_runner.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/runners/malformed_runner.jsonnetis excluded by!tests/**tests/integration/stable/runners/multi-file/contract/multi-file.jsonnetis excluded by!tests/**tests/integration/stable/runners/no_runner.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/runners/no_runner.jsonnetis excluded by!tests/**tests/integration/stable/runners/zip/no-zip.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/runners/zip/no-zip.jsonnetis excluded by!tests/**tests/integration/stable/runners/zip/zip.jsonnetis excluded by!tests/**tests/integration/stable/self-run/datetime.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/self-run/datetime.jsonnetis excluded by!tests/**tests/integration/stable/self-run/floats.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/self-run/floats.jsonnetis excluded by!tests/**tests/integration/stable/self-run/formats.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/self-run/formats.jsonnetis excluded by!tests/**tests/integration/stable/self-run/issue_163.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/self-run/issue_163.jsonnetis excluded by!tests/**tests/integration/stable/self-run/module/np.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/self-run/module/np.jsonnetis excluded by!tests/**tests/integration/stable/self-run/module/pil.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/self-run/module/pil.jsonnetis excluded by!tests/**tests/integration/stable/self-run/re.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/self-run/re.jsonnetis excluded by!tests/**tests/integration/stable/self-run/typing_is_ok.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/self-run/typing_is_ok.jsonnetis excluded by!tests/**tests/integration/stable/storage/alloc_generic.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/alloc_generic.jsonnetis excluded by!tests/**tests/integration/stable/storage/alloc_generic_err.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/alloc_generic_err.jsonnetis excluded by!tests/**tests/integration/stable/storage/base.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/base.jsonnetis excluded by!tests/**tests/integration/stable/storage/floats.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/floats.jsonnetis excluded by!tests/**tests/integration/stable/storage/gvm-89.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/gvm-89.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/gvm-89.jsonnetis excluded by!tests/**tests/integration/stable/storage/locking/default-frozen.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/locking/default-frozen.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/locking/default-frozen.0_0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/locking/default-frozen.jsonnetis excluded by!tests/**tests/integration/stable/storage/locking/modify_ctor.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/locking/modify_ctor.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/locking/modify_ctor.jsonnetis excluded by!tests/**tests/integration/stable/storage/locking/modify_later.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/locking/modify_later.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/locking/modify_later.0_0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/locking/modify_later.jsonnetis excluded by!tests/**tests/integration/stable/storage/np.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/np.jsonnetis excluded by!tests/**tests/integration/stable/storage/persists.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/persists.0_0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/persists.jsonnetis excluded by!tests/**tests/integration/stable/storage/read_nondet.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/read_nondet.jsonnetis excluded by!tests/**tests/integration/stable/storage/storage_tree_map.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/storage_tree_map.jsonnetis excluded by!tests/**tests/integration/stable/storage/to_str.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/to_str.jsonnetis excluded by!tests/**tests/integration/stable/storage/tree_map_nested.0.hashis excluded by!**/*.hash,!tests/**tests/integration/stable/storage/tree_map_nested.jsonnetis excluded by!tests/**
📒 Files selected for processing (13)
executor/codegen/data/host-fns.jsonexecutor/crates/common/Cargo.tomlexecutor/crates/common/src/host_fns.rsexecutor/crates/common/src/lib.rsexecutor/crates/modules-interfaces/Cargo.tomlexecutor/src/exe/run.rsexecutor/src/host/mod.rsexecutor/src/lib.rsexecutor/src/rt/memlimiter.rsexecutor/src/rt/mod.rsexecutor/src/rt/supervisor/mod.rsexecutor/src/rt/vm/mod.rsexecutor/src/wasi/genlayer_sdk.rs
💤 Files with no reviewable changes (2)
- executor/crates/common/src/host_fns.rs
- executor/codegen/data/host-fns.json
| topmost_runner_id: NestedRunnerId("contract".to_owned()), | ||
| remaining_recursion: vm_data.remaining_recursion, | ||
| remaining_det_fuel, | ||
| memory_limit: supervisor.limiter.get(true).get_remaining_memory(), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
The nested memory budget crosses the executor boundary with no shared or clamped ceiling. The producer sends its full remaining deterministic memory, and the consumer installs that value as an independent limiter, so a routed chain is no longer bounded by one budget the way the in-process Limiter::derived() path is.
executor/src/wasi/genlayer_sdk.rs#L986-L986: send a reduced share instead ofsupervisor.limiter.get(true).get_remaining_memory(), or document the manager-side aggregate ceiling that makes the full value safe.executor/src/exe/run.rs#L316-L317: clampn.memory_limitagainst a local maximum before passing it tocreate_supervisor, matching howremaining_recursionis clamped with.min(public_abi::top_limits::VM_RECURSION)inexecutor/src/lib.rs.
📍 Affects 2 files
executor/src/wasi/genlayer_sdk.rs#L986-L986(this comment)executor/src/exe/run.rs#L316-L317
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@executor/src/wasi/genlayer_sdk.rs` at line 986, The nested memory limit
currently crosses the executor boundary without a shared ceiling. In
executor/src/wasi/genlayer_sdk.rs:986, send a reduced memory share or document
the manager-side aggregate ceiling that makes the full remaining value safe; in
executor/src/exe/run.rs:316-317, clamp n.memory_limit to a local maximum before
passing it to create_supervisor, following the existing remaining_recursion
clamp pattern.
27ef6a0 to
4eb17a2
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
executor/src/rt/mod.rs (1)
145-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the remaining fuel-budget branches.
The new test covers an imported budget that caps host fuel and decreases after consumption. Add cases for no imported budget, host fuel below the imported budget, and consumption beyond the remaining budget. These cases protect the
None,min, andsaturating_subbehavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@executor/src/rt/mod.rs` around lines 145 - 163, Extend the tests around DetFuelBudget::new, remaining, and consume to cover no imported budget, host fuel lower than the imported budget, and consumption exceeding the remaining budget. Assert that None uses the host fuel, remaining applies the minimum of host and imported fuel, and over-consumption saturates at zero while preserving the existing imported-budget assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@executor/crates/common/src/expr/value.rs`:
- Around line 213-217: Add a terminal failed variant to ThunkState that stores
the original EvalError, and update the force logic around deferred() to cache
ordinary evaluation errors in that variant instead of leaving the thunk
InProgress. Ensure subsequent force calls return the cached error directly,
while preserving Forced behavior for successful evaluations and reserving
recursion reporting for an actually re-entered InProgress thunk.
In `@executor/crates/common/src/io.rs`:
- Around line 224-226: Update the safety contract for FdPairStream::from_raw_fds
to require source_fd and sink_fd are distinct valid open descriptors, and
document that successful construction transfers ownership of both descriptors to
the returned wrapper.
In `@executor/crates/sdk-rs/src/abi/consts.rs`:
- Line 3: Update the module-level lint attribute in consts.rs to remove
clippy::all and restore the narrower clippy::redundant_static_lifetimes
allowance, keeping dead_code suppression unchanged for the generated module.
---
Nitpick comments:
In `@executor/src/rt/mod.rs`:
- Around line 145-163: Extend the tests around DetFuelBudget::new, remaining,
and consume to cover no imported budget, host fuel lower than the imported
budget, and consumption exceeding the remaining budget. Assert that None uses
the host fuel, remaining applies the minimum of host and imported fuel, and
over-consumption saturates at zero while preserving the existing imported-budget
assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b2a5bee-28b7-4864-98ab-e92c9914a1a2
📒 Files selected for processing (10)
executor/crates/common/src/expr/evaluator.rsexecutor/crates/common/src/expr/value.rsexecutor/crates/common/src/io.rsexecutor/crates/sdk-rs/src/abi/consts.rsexecutor/src/domain/fees.rsexecutor/src/rt/mod.rsexecutor/src/rt/supervisor/actions.rsexecutor/src/rt/supervisor/mod.rsexecutor/src/rt/vm/mod.rsexecutor/src/wasi/genlayer_sdk.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- executor/src/rt/supervisor/mod.rs
- executor/src/rt/vm/mod.rs
- executor/src/wasi/genlayer_sdk.rs
| // This file is auto-generated. Do not edit! | ||
|
|
||
| #![allow(dead_code, clippy::redundant_static_lifetimes)] | ||
| #![allow(dead_code, clippy::all)] |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 2 'clippy::all|clippy::redundant_static_lifetimes' executor/crates/sdk-rs --glob '*.rs'Repository: genlayerlabs/genvm-executor
Length of output: 498
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf 'Tracked files under abi:\n'
git ls-files executor/crates/sdk-rs/src/abi | sed -n '1,80p'
printf '\nconsts.rs first 40 lines:\n'
cat -n executor/crates/sdk-rs/src/abi/consts.rs | sed -n '1,40p'
printf '\nGenerator references to abi/consts.rs or consts generation:\n'
rg -n 'consts\.rs|abi::consts|abi/consts|generator|generate' executor/crates/sdk-rs --glob '*.rs' --glob '*.rs.in' --glob '*.toml' | sed -n '1,160p'Repository: genlayerlabs/genvm-executor
Length of output: 2135
Keep clippy::all off this generated module.
clippy::all suppresses the full default Clippy lint group for generated Rust code. Restore the narrower clippy:redundant_static_lifetimes allowance, or use a target-specific Clippy setting for this generated module.
Proposed fix
-#![allow(dead_code, clippy::all)]
+#![allow(dead_code, clippy::redundant_static_lifetimes)]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #![allow(dead_code, clippy::all)] | |
| #![allow(dead_code, clippy::redundant_static_lifetimes)] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@executor/crates/sdk-rs/src/abi/consts.rs` at line 3, Update the module-level
lint attribute in consts.rs to remove clippy::all and restore the narrower
clippy::redundant_static_lifetimes allowance, keeping dead_code suppression
unchanged for the generated module.
cdf4e28 to
bbad967
Compare
* chore(fuzz): refresh executor corpora 🎨 `latest_non_final` and `accepted` named an implementation queue rather than the state-view contract: the view is the latest state-changing decided transaction, with finalized state as the fallback. Wire values change with no back-compat alias, so hosts and SDKs must move in lockstep.
bbad967 to
52b4153
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Auto-opened executor mirror of genlayerlabs/genvm-manager#24.
Carries the executor-side work for that manager PR. Auto-closed as merged when the manager PR lands (its
pr/v0.2/fix/vm-fatal-errorsbranch is moved ontov0.2-dev).Summary by CodeRabbit
New Features
Bug Fixes