fix: vm fatal errors - #21
Open
kp2pml30 wants to merge 3 commits into
Open
Conversation
* feat(sdk): call contracts of another major through the host ✨ * refactor(rt): bound recursion with a budget minted by the chain root ♻️ * refactor(exe): carry nested execution state as one explicit group ♻️ * fix(exe): fold the callee's small hash on the nested route 🐛🔒️ * feat(exe): refuse a crossing call while custom runners are loaded ✨ * feat(host): write caller-supplied hello bytes and drop notify_finished ✨ * fix(supervisor): load precompiled modules only for registry runners 🔒️ * fix(sdk): stop exporting nondet permission across the boundary 🐛
📝 WalkthroughWalkthroughThe executor now supports cross-executor nested calls. It propagates nested permissions, recursion, memory, storage, signer, stack, fee, and deterministic-fuel state. Host connections support hello data, nested protocol requests, shared result hashes, and final flushing. ChangesNested execution and host integration
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant VM
participant CallContract
participant Host
participant NestedExecutor
VM->>CallContract: CallContract request
CallContract->>Host: resolve_callcontract_executor
Host-->>CallContract: executor address
CallContract->>Host: run_nested envelope
Host->>NestedExecutor: nested request
NestedExecutor-->>Host: NestedRunReply
Host-->>CallContract: decoded reply
CallContract-->>VM: validated nested result
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
Comment on lines
+193
to
+212
| fn convert_nested_permissions( | ||
| permissions: genvm_modules_interfaces::NestedPermissions, | ||
| state_mode: public_abi::StorageType, | ||
| topmost_runner_id: runners::Id, | ||
| ) -> wasi::base::Config { | ||
| use genvm_modules_interfaces::NestedPermissions as P; | ||
|
|
||
| wasi::base::Config { | ||
| needs_error_fingerprint: true, | ||
| is_deterministic: permissions.contains(P::DETERMINISTIC), | ||
| can_read_storage: permissions.contains(P::READ_STORAGE), | ||
| can_write_storage: permissions.contains(P::WRITE_STORAGE), | ||
| can_spawn_nondet: permissions.contains(P::SPAWN_NONDET), | ||
| can_send_messages: permissions.contains(P::SEND_MESSAGES), | ||
| can_call_others: permissions.contains(P::CALL_OTHERS), | ||
| can_register_runners: permissions.contains(P::REGISTER_RUNNERS), | ||
| state_mode, | ||
| topmost_runner_id, | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find every caller of run_with / run_with_impl / create_supervisor to see which ones
# validate nested permissions first.
set -euo pipefail
rg -nP --type=rust -C 5 '\b(run_with|run_with_impl|create_supervisor)\s*\('
rg -nP --type=rust -C 3 'check_nested_permissions'Repository: genlayerlabs/genvm-executor
Length of output: 165
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "tracked rust files:"
git ls-files '*.rs' | sed -n '1,120p'
echo
echo "try locate files:"
fd -a 'lib\.rs|run\.rs' . | sed -n '1,120p'Repository: genlayerlabs/genvm-executor
Length of output: 4807
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "executor/src/lib.rs relevant section:"
sed -n '163,220p' executor/src/lib.rs | nl -ba -v163
echo
echo "executor/src/exe/run.rs relevant section:"
sed -n '1,90p' executor/src/exe/run.rs | nl -ba -v1
echo
echo "run_with/create_supervisor references in tracked Rust files:"
rg -n -C 4 'run_with|run_with_impl|create_supervisor|check_nested_permissions|convert_nested_permissions|NestedPermissions' executor
echo
echo "candidate module outlines:"
for f in executor/src/lib.rs executor/src/exe/run.rs executor/src/rt/supervisor/mod.rs executor/src/wasi/base.rs; do
echo "----- $f"
wc -l "$f"
ast-grep outline "$f" --view condensed | sed -n '1,180p' || true
doneRepository: genlayerlabs/genvm-executor
Length of output: 243
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "executor/src/lib.rs relevant section:"
sed -n '163,220p' executor/src/lib.rs | awk '{printf "%4d %s\n", NR+162, $0}'
echo
echo "executor/src/exe/run.rs relevant section:"
sed -n '1,90p' executor/src/exe/run.rs | awk '{printf "%4d %s\n", NR, $0}'
echo
echo "run_with/create_supervisor/check_nested_permissions/relevant references in tracked Rust files:"
rg -n -C 4 'run_with|run_with_impl|create_supervisor|check_nested_permissions|convert_nested_permissions|NestedPermissions|StorageRead' executor
echo
echo "file lengths:"
wc -l executor/src/lib.rs executor/src/exe/run.rs executor/crates/sdk-rs/src/abi/entry.rs executor/src/rt/supervisor/mod.rs executor/src/wasi/base.rsRepository: genlayerlabs/genvm-executor
Length of output: 16346
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "executor/src/lib.rs create_supervisor/locked-slot/run_with sections:"
sed -n '60,145p' executor/src/lib.rs | awk '{printf "%4d %s\n", NR+59, $0}'
printf '\n---\n'
sed -n '282,470p' executor/src/lib.rs | awk '{printf "%4d %s\n", NR+281, $0}'
printf '\n---\n'
sed -n '720,960p' executor/src/wasi/genlayer_sdk.rs | awk '{printf "%4d %s\n", NR+719, $0}'
printf '\n---\n'
sed -n '285,340p' executor/src/exe/run.rs | awk '{printf "%4d %s\n", NR+284, $0}'Repository: genlayerlabs/genvm-executor
Length of output: 27412
Reject forbidden nested permission bits before converting VM config.
executor/src/lib.rs:193-206 maps WRITE_STORAGE and SEND_MESSAGES straight into wasi::base::Config, while executor/src/exe/run.rs:32-50 rejects them only in handle. A nested CallContract then passes the parent config to CallContract children, so the rejected permission can authorize message-sending/other capabilities in one direction. Add the same permission check in convert_nested_permissions.
🤖 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/lib.rs` around lines 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.
| 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.
kp2pml30
force-pushed
the
pr/v0.2/fix/vm-fatal-errors
branch
from
August 10, 2026 11:14
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
Comment on lines
+213
to
217
| // On error it stays `InProgress`: a failed computation is not retried, and | ||
| // any later force surfaces the recursion/error path consistently. | ||
| if let Ok(v) = &result { | ||
| *state = ThunkState::Forced(v.clone()); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Cache failed evaluations separately from InProgress.
When deferred() returns an ordinary EvalError, these lines leave the thunk in ThunkState::InProgress. The next force() then reports infinite recursion while forcing a lazy value, even when no recursion occurred. The original error is lost, and the binding becomes permanently unusable.
Add a terminal failed state that preserves the original error. Do not use InProgress as the error cache.
🤖 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/common/src/expr/value.rs` around lines 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.
Comment on lines
+224
to
+226
| /// | ||
| /// # Safety | ||
| /// Both fds must be valid open file descriptors that no one else closes or owns |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 'FdPairStream::from_raw_fds|from_raw_fds\s*\(' executor --glob '*.rs'Repository: genlayerlabs/genvm-executor
Length of output: 1478
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sed -n '1,280p' executor/crates/common/src/io.rsRepository: genlayerlabs/genvm-executor
Length of output: 8745
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
src = Path("executor/crates/common/src/io.rs").read_text()
needle = "unsafe fn from_raw_fds("
idx = src.index(needle)
end = src.index("pub struct FdPairStream", idx)
chunk = src[:end]
checks = {
"contains_own_raw_fds": "std::os::fd::OwnedFd::from_raw_fd(source_fd)" in chunk and "std::os::fd::OwnedFd::from_raw_fd(sink_fd)" in chunk,
"no_source_sink_inequality": "source_fd != sink_fd" not in chunk and "source_fd == sink_fd" not in chunk and "must not be the same" not in chunk.lower() and "distinct" not in chunk and "double-close" not in chunk.lower(),
"document_safety_unsafe": "# Safety" in chunk and "Both fds must be valid open file descriptors that no one else closes or owns" in chunk,
}
for k, v in checks.items():
print(f"{k}: {v}")
PYRepository: genlayerlabs/genvm-executor
Length of output: 270
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
src = Path("executor/crates/common/src/io.rs").read_text()
# Extract FdPairStream safe doc and from_raw_fds body/preceding safety section only.
body = re.search(r"(impl FdPairStream \{[\s\S]*?\n\})", src)
if not body:
raise SystemExit("FdPairStream impl not found")
sect = body.group(1)
print("contains_from_raw_fds:", "unsafe fn from_raw_fds(" in sect)
print("builds_two_owned_fds:", "OwnedFd::from_raw_fd(source_fd)" in sect and "OwnedFd::from_raw_fd(sink_fd)" in sect)
print("checks_source_sink_inequality:", bool(re.match(r".* source_fd != sink_fd .*", sect, re.S)))
print("uses_same_raw_fd_in_both_constructors:", re.search(r"OwnedFd::from_raw_fd\(source_fd\).*OwnedFd::from_raw_fd\(sink_fd\)|OwnedFd::from_raw_fd\(sink_fd\).*OwnedFd::from_raw_fd\(source_fd\)", sect))
needle = "unsafe fn from_raw_fds("
idx = sect.index(needle)
prev = sect[:idx]
after = sect[idx:sect.find("impl tokio::io::AsyncRead", idx)]
# Safety sentence(s) are immediately before pub unsafe fn; find last # Safety block in that prefix.
last_safety_start = prev.rfind("# Safety")
safety = prev[last_safety_start:] if last_safety_start >= 0 else ""
print("current_safety_text:")
print(safety.strip())
PYRepository: genlayerlabs/genvm-executor
Length of output: 424
Require distinct raw descriptors.
FdPairStream::from_raw_fds independently constructs an OwnedFd from each raw value. Passing the same raw descriptor for source_fd and sink_fd lets the wrapper drop two owned descriptors for one descriptor and can double-close it. Require source_fd != sink_fd in the safety contract and document that successful construction takes ownership of both descriptors.
🤖 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/common/src/io.rs` around lines 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.
| // 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.
Suggested change
| #![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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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