Skip to content

fix: vm fatal errors #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kp2pml30 wants to merge 3 commits into
base: v0.2-dev
Choose a base branch
from
Open

fix: vm fatal errors #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e3e1ffa
feat(executor): cross-major calls and host protocol rework
kp2pml30 Jul 29, 2026
f512309
fix(exe): refuse a nested fatal vm error this line cannot raise 🔒️
kp2pml30 Aug 5, 2026
4eb17a2
test(integration): adopt the shared tag registry vocabulary ✅
kp2pml30 Aug 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
2 changes: 2 additions & 0 deletions executor/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 0 additions & 40 deletions executor/codegen/data/host-fns.json
View file
Open in desktop

This file was deleted.

25 changes: 23 additions & 2 deletions executor/crates/common/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions executor/crates/common/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ genlayer_calldata = { path = "../calldata" }
genlayer_sdk = { path = "../sdk-rs", default-features = false, features = [
"arbitrary",
] }
genvm_modules_interfaces = { path = "../modules-interfaces" }

anyhow = { version = "1.0.97", features = ["backtrace"] }
arbitrary = "1.4.2"
Expand Down
4 changes: 3 additions & 1 deletion executor/crates/common/src/expr/evaluator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ use super::value::{BinOp, EvalError, Expr, StrSeg, Thunk, Value};
// It is only used for trusted, operator-supplied fee config expressions,
// never for contract-supplied or user-supplied input.

type GetVarFn = dyn Fn(&str) -> Result<Value, EvalError> + Send + Sync;

#[derive(Clone)]
struct EvalContext {
get_var: Arc<dyn Fn(&str) -> Result<Value, EvalError> + Send + Sync>,
get_var: Arc<GetVarFn>,
let_bindings: rpds::RedBlackTreeMap<String, Thunk, archery::ArcK>,
}

Expand Down
10 changes: 5 additions & 5 deletions executor/crates/common/src/expr/value.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,12 +210,12 @@ impl Thunk {
let result = deferred();

let mut state = self.0.lock().expect("thunk mutex poisoned");
match &result {
Ok(v) => *state = ThunkState::Forced(v.clone()),
// Leave it `InProgress`: a failed computation is not retried, and
// any later force surfaces the recursion/error path consistently.
Err(_) => {}
// 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());
}
Comment on lines +213 to 217

@coderabbitai coderabbitai Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 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.


result
}
}
Expand Down
128 changes: 0 additions & 128 deletions executor/crates/common/src/host_fns.rs
View file
Open in desktop

This file was deleted.

6 changes: 6 additions & 0 deletions executor/crates/common/src/io.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ impl AsyncCustomFD {

/// Create a new AsyncCustomFD from a raw fd, taking ownership
/// Sets the fd to non-blocking mode automatically
///
/// # Safety
/// `fd` must be a valid open file descriptor that no one else closes or owns
pub unsafe fn from_raw_fd(fd: std::os::fd::RawFd) -> std::io::Result<Self> {
set_fd_nonblocking(fd)?;
let owned = std::os::fd::OwnedFd::from_raw_fd(fd);
Expand Down Expand Up @@ -218,6 +221,9 @@ impl FdPairStream {
/// Create a new FdPairStream from raw file descriptors
/// source_fd is used for reading, sink_fd is used for writing
/// Takes ownership and sets both to non-blocking mode
///
/// # Safety
/// Both fds must be valid open file descriptors that no one else closes or owns
Comment on lines +224 to +226

@coderabbitai coderabbitai Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 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.rs

Repository: 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}")
PY

Repository: 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())
PY

Repository: 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.

pub unsafe fn from_raw_fds(
source_fd: std::os::fd::RawFd,
sink_fd: std::os::fd::RawFd,
Expand Down
2 changes: 1 addition & 1 deletion executor/crates/common/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub mod templater;
pub mod version;

pub mod expr;
pub mod host_fns;
pub use genvm_modules_interfaces::host_fns;
pub mod util;

#[cfg(not(debug_assertions))]
Expand Down
1 change: 1 addition & 0 deletions executor/crates/modules-interfaces/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions executor/crates/modules-interfaces/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ serde = { version = "1.0.219", features = ["rc", "derive"] }
serde_bytes = "0.11.17"
serde_derive = "1.0.219"
serde_json = "1.0.140"
sha3 = "0.10"
tokio = { version = "1.44.1", features = [
"rt",
"rt-multi-thread",
Expand Down
2 changes: 1 addition & 1 deletion executor/crates/sdk-rs/src/abi/consts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// This file is auto-generated. Do not edit!

#![allow(dead_code, clippy::redundant_static_lifetimes)]
#![allow(dead_code, clippy::all)]

@coderabbitai coderabbitai Bot Aug 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 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.


use serde::{Deserialize, Serialize};

Expand Down
2 changes: 2 additions & 0 deletions executor/src/domain/fees.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ impl MessageAllocationNode {
abi::encode(roots)
}

#[allow(clippy::if_same_then_else)]
pub fn matches_internal(
&self,
on: On,
Expand All @@ -121,6 +122,7 @@ impl MessageAllocationNode {
}
}

#[allow(clippy::if_same_then_else)]
pub fn matches_external(
&self,
recipient: genlayer_sdk::calldata::Address,
Expand Down
Loading