Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ velcro = "0.5.4"
wasmer = "6.1.0"
wasmer-types = "6.1.0"
wasmer-compiler-cranelift = "6.1.0"
wasmer-middlewares = "6.1.0"
wat = "1.243.0"
web3 = "0.19.0"
webbrowser = "1.0.4"
Expand Down
9 changes: 7 additions & 2 deletions crates/runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@ tokio-stream.workspace = true
tracing.workspace = true
ureq.workspace = true
wasmer.workspace = true
wasmer-compiler-cranelift.workspace = true
Copy link

Choose a reason for hiding this comment

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

💡 Cranelift compiler changed from optional to required dependency

The wasmer-compiler-cranelift dependency was previously optional (gated by the profiling feature) but is now always included. This increases binary size for all users, even those who don't need profiling or could use precompiled modules. The feature comment acknowledges this but the tradeoff may not be ideal for all use cases.

Suggested fix:

Consider making the metering/Cranelift dependency optional with a separate feature flag (e.g., `execution-limits`), allowing users who only work with trusted precompiled modules to opt out.

Copy link

Choose a reason for hiding this comment

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

Resolved - This issue has been addressed in the latest changes.

Copy link

Choose a reason for hiding this comment

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

Resolved - This issue has been addressed in the latest changes.

Copy link

Choose a reason for hiding this comment

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

Resolved - This issue has been addressed in the latest changes.

wasmer-middlewares.workspace = true
wasmer-types.workspace = true
wasmer-compiler-cranelift = { workspace = true, optional = true }

calimero-primitives.workspace = true
calimero-node-primitives.workspace = true
Expand All @@ -46,4 +47,8 @@ wat.workspace = true

[features]
host-traces = ["owo-colors"]
profiling = ["wasmer-compiler-cranelift"]
# Enables PerfMap profiling support for WASM stack traces.
# When enabled, set ENABLE_WASMER_PROFILING=true at runtime to generate perf.map files.
# Note: This feature only controls the runtime env var check behavior - it does NOT
# gate the Cranelift compiler dependency, which is always included for operation limit metering.
profiling = []
2 changes: 2 additions & 0 deletions crates/runtime/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@ pub enum WasmTrap {
Unreachable,
#[error("unaligned atomic operation")]
UnalignedAtomic,
#[error("execution exceeded timeout")]
Copy link

Choose a reason for hiding this comment

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

🟡 Misleading error name and message for operation limit

The error variant ExecutionTimeout with message "execution exceeded timeout" suggests a time-based timeout, but this is actually an operation count limit. Users debugging issues may be confused expecting a wall-clock timeout mechanism when it's really instruction counting.

Suggested fix:

Consider renaming to `OperationLimitExceeded` or `ExecutionLimitExceeded` with message "execution exceeded operation limit" to accurately describe the behavior.

Copy link

Choose a reason for hiding this comment

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

Resolved - This issue has been addressed in the latest changes.

Copy link

Choose a reason for hiding this comment

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

Resolved - This issue has been addressed in the latest changes.

Copy link

Choose a reason for hiding this comment

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

Resolved - This issue has been addressed in the latest changes.

Copy link

Choose a reason for hiding this comment

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

💡 Misleading error variant name - 'ExecutionTimeout' implies time-based limit

The error variant ExecutionTimeout with message 'execution exceeded timeout' suggests a time-based limit, but this is actually triggered by exceeding an operation count limit. This naming inconsistency could confuse users debugging WASM execution failures.

Suggested fix:

Consider renaming to `OperationLimitExceeded` or `ExecutionLimitExceeded` with a message like 'execution exceeded operation limit' to accurately reflect the metering-based nature of this limit.

Copy link

Choose a reason for hiding this comment

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

Resolved - This issue has been addressed in the latest changes.

ExecutionTimeout,
#[error("indeterminate trap")]
Indeterminate,
}
Expand Down
Loading
Loading