Skip to content

refactor!(workspace): add global separation of syscall kind#5401

Open
StackOverflowExcept1on wants to merge 16 commits intomasterfrom
av/ethexe-check-syscalls
Open

refactor!(workspace): add global separation of syscall kind#5401
StackOverflowExcept1on wants to merge 16 commits intomasterfrom
av/ethexe-check-syscalls

Conversation

@StackOverflowExcept1on
Copy link
Copy Markdown
Member

@StackOverflowExcept1on StackOverflowExcept1on commented May 1, 2026

Resolves #4138

TLDR:

  • list of forbidden syscalls has been synchronized in utils/wasm-instrument/src/syscalls.rs, gsys/src/lib.rs, ethexe/runtime/common/src/lib.rs
  • gr_create_program, gr_random, gr_wait have been disabled in ethexe and in the ecosystem to avoid wasm files with unimplemented syscalls
  • ecosystem crates gtest, gear-wasm-builder now automatically throw error if some import is missing (depending on wasm runtime)

@semanticdiff-com
Copy link
Copy Markdown

semanticdiff-com Bot commented May 1, 2026

Review changes with  SemanticDiff

Changed Files
File Status
  pallets/gear/src/queue.rs  96% smaller
  core/backend/src/lib.rs  95% smaller
  utils/wasm-builder/src/code_validator.rs  81% smaller
  pallets/gear/src/benchmarking/mod.rs  69% smaller
  ethexe/runtime/common/src/lib.rs  66% smaller
  pallets/gear/src/lib.rs  58% smaller
  utils/wasm-gen/src/generator.rs  53% smaller
  utils/wasm-gen/src/generator/syscalls/imports.rs  31% smaller
  core/backend/src/env.rs  25% smaller
  utils/wasm-instrument/src/syscalls.rs  11% smaller
  utils/wasm-proc/src/main.rs  4% smaller
  utils/calc-stack-height/src/main.rs  2% smaller
  utils/wasm-builder/src/wasm_project.rs  2% smaller
  gstd/src/msg/async.rs  1% smaller
  pallets/gear/src/runtime_api.rs  1% smaller
  utils/wasm-builder/src/crate_info.rs  1% smaller
  utils/wasm-builder/src/lib.rs  1% smaller
  core/src/code/mod.rs  1% smaller
  core/processor/src/common.rs  0% smaller
  core/processor/src/context.rs  0% smaller
  core/processor/src/executor.rs  0% smaller
  core/processor/src/processing.rs  0% smaller
  core/src/code/utils.rs  0% smaller
  ethexe/processor/src/tests.rs  0% smaller
  ethexe/runtime/src/wasm/api/instrument.rs  0% smaller
  examples/delayed-sender-ethexe/src/wasm.rs  0% smaller
  examples/syscalls-ethexe/src/lib.rs  0% smaller
  examples/syscalls-ethexe/src/wasm.rs  0% smaller
  gcore/src/exec.rs  0% smaller
  gcore/src/lib.rs  0% smaller
  gcore/src/prog.rs  0% smaller
  gstd/src/lib.rs  0% smaller
  gstd/src/prog/basic.rs  0% smaller
  gstd/src/prog/encoded.rs  0% smaller
  gstd/src/prog/generator.rs  0% smaller
  gsys/src/lib.rs  0% smaller
  gtest/src/manager/block_exec.rs  0% smaller
  gtest/src/manager/memory.rs  0% smaller
  gtest/src/program.rs  0% smaller
  pallets/gear/src/benchmarking/utils.rs  0% smaller
  pallets/gear/src/tests.rs  0% smaller
  utils/wasm-builder/Cargo.toml Unsupported file format
  utils/wasm-gen/src/config/syscalls/injection.rs  0% smaller
  utils/wasm-gen/src/tests.rs  0% smaller
  utils/wasm-instrument/src/tests.rs  0% smaller

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request implements a global separation of syscall kinds within the workspace. By introducing SyscallKind, the system can now explicitly differentiate between standard Vara syscalls and Ethereum-compatible syscalls. This change impacts the entire execution stack, requiring updates to code instrumentation, environment initialization, and validation logic to ensure that programs are correctly configured for their intended execution environment.

Highlights

  • Syscall Kind Separation: Introduced a global SyscallKind enum to distinguish between different syscall environments (Vara and Eth), allowing for more granular control over syscall availability and instrumentation.
  • Infrastructure Updates: Updated core components, including the processor, environment, and code validation logic, to propagate and utilize the SyscallKind throughout the execution pipeline.
  • Test and Example Adjustments: Updated numerous tests, benchmarks, and examples to accommodate the new SyscallKind parameter, ensuring compatibility with the updated architecture.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces SyscallKind to distinguish between Vara and Ethereum (ethexe) execution environments across the workspace. It propagates this kind through the core backend, processor, and WASM instrumentation layers to ensure environment-specific syscall validation and binding. Additionally, it applies conditional compilation to disable unsupported syscalls like wait, random, and program creation in the ethexe context, while updating tests and examples to support both execution modes. I have no feedback to provide.

SyscallName::SendWGas, // Deprecated
SyscallName::SystemReserveGas, // Deprecated
SyscallName::UnreserveGas, // Deprecated
SyscallName::Wait, // Deprecated
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

same order as gsys and gear-wasm-instrument

@StackOverflowExcept1on
Copy link
Copy Markdown
Member Author

@claude opus review

@claude
Copy link
Copy Markdown

claude Bot commented May 4, 2026

Claude finished @StackOverflowExcept1on's task in 7m 43s —— View job


Claude Opus PR Review

This PR introduces SyscallKind (Vara / Eth) and threads it through code instrumentation, the WASM environment binding, and the wasm-builder/gtest validation pipelines so that programs targeting ethexe are checked against an explicit, smaller syscall set. It also adds #[cfg(not(feature = "ethexe"))] gates on the gr_wait, gr_random, and gr_create_program raw bindings (and on the corresponding gcore/gstd wrappers) so ethexe-targeted WASM cannot import unimplemented host functions.

Overall the change is well-structured. I found one critical issue, one design concern around drift, and a couple of small things.


severity: criticalbind_funcs and the post-bind assertion in Environment::new are inconsistent; the assertion will panic in both modes

File: core/backend/src/env.rs lines 164–233 and 300–307

The new macro in bind_funcs skips a syscall when:

  • SyscallKind::Vara and !syscall.is_vara() — true only for SystemBreak (is_vara is self != Self::SystemBreak)
  • SyscallKind::Eth and !syscall.is_eth() — true for the 20 syscalls excluded by is_eth() (e.g. Wait, Random, CreateProgram*, Reservation*, all *WGas, etc.)

So builder.funcs_count after bind_funcs(syscall_kind) is:

  • Vara: SyscallName::all().count() - 1 (SystemBreak skipped)
  • Eth: SyscallName::all().count() - 20 + 1 (eth-excluded skipped, SystemBreak still added because SystemBreak.is_eth() == true)

But the assertion immediately after still compares against the unconditional total:

assert_eq!(
    builder.funcs_count,
    SyscallName::all().count(),
    "Not all existing syscalls were added to the module's env."
);

This means every call to Environment::new will panic with "Not all existing syscalls were added to the module's env.", including the production Vara queue path (pallets/gear/src/queue.rs:287, which passes SyscallKind::Vara) and the test_syscalls_table loop in core/backend/src/lib.rs:77 that exercises both kinds. The current PR-CI failures look infra-related ("Setup linker") so this didn't get caught by tests. Please run cargo nextest run -p gear-core-backend test_syscalls_table locally to confirm.

The fix is to make the assertion match the new semantics — e.g. compare against SyscallName::instrumentable(syscall_kind).count() plus 1 for SystemBreak when it's actually bound, or rework bind_funcs so SystemBreak is bound unconditionally and only the user-facing syscalls are filtered (which seems closer to the intent, since SystemBreak is a runtime-injected host function and isn't listed in instrumentable_all).


severity: medium — three sources of truth for the ethexe-disabled syscall list with no enforced consistency

There are now three places that must agree on which syscalls are unavailable in ethexe:

  1. utils/wasm-instrument/src/syscalls.rs:230-254SyscallName::is_eth() exclusion list (drives instrumentation/import check)
  2. gsys/src/lib.rs#[cfg(not(feature = "ethexe"))] attributes on the FFI declarations (drives compile-time gating in WASM programs)
  3. ethexe/runtime/common/src/lib.rs:190-211forbidden_funcs in the runtime's BlockConfig (drives runtime rejection)

The PR description and the author's own comment on line 210 acknowledge this: "same order as gsys and gear-wasm-instrument". But there's no compile-time check or test asserting that the three lists are identical. A future PR that adds/removes an ethexe-restricted syscall in only one or two places will silently de-sync them — and the failure mode (ethexe runtime accepting a wasm whose syscall has no host impl, or rejecting one that gsys allows) is a consensus-relevant one.

Suggestion: derive forbidden_funcs directly from SyscallName::all().filter(|s| !s.is_eth()), or add a unit test in ethexe-runtime-common that asserts the forbidden_funcs set equals SyscallName::all().filter(|s| !s.is_eth()).collect(). That removes an entire class of drift bugs.


severity: low — misleading grouping comment in generate_precise_syscalls

File: utils/wasm-gen/src/generator/syscalls/imports.rs lines 187–197

let precise_syscalls: [(...); 5] = [
    (ReservationSend, Self::generate_send_from_reservation),
    (ReservationReply, Self::generate_reply_from_reservation),
    // Available for both `SyscallKind::Vara` and `SyscallKind::Eth`
    (SendCommit, Self::generate_send_commit),
    (SendCommitWGas, Self::generate_send_commit_with_gas),
    (ReplyDeposit, Self::generate_reply_deposit),
];

The "Available for both" comment is placed before three entries but only SendCommit is actually eth-available. SendCommitWGas and ReplyDeposit are both in the is_eth() exclusion list and will be skipped by the eth branch immediately below. The runtime check below handles this correctly, so this is purely a comment placement issue — but it's misleading as written. Either move the comment to apply only to SendCommit, or drop it.


severity: low — intra-doc links to gated wait/random will be broken under the ethexe feature

File: gcore/src/exec.rs

wait and random are now gated by #[cfg(not(feature = "ethexe"))], but other items still doc-link to them:

  • wait_for (line 359) and wait_up_to (line 368) — Same as [wait], but...
  • wake (line 376) — Suppose a message has been paused using the [wait] function...

When docs are built with --features ethexe, those [wait] references will become broken intra-doc links, and cargo doc with -Dwarnings will fail. Replace with prose ("the wait syscall") or gate the docs accordingly.


Things that look good

  • Threading SyscallKind through ProcessExecutionContext / WasmExecutionContext / ContextCharged is consistent across the processor crates.
  • wasm-builder and gtest correctly select SyscallKind via a private SYSCALL_KIND const driven by their own ethexe feature, and propagate gsys/ethexe so the wasm side stays consistent.
  • The test_syscalls_table test now exercises both kinds — once the assertion above is fixed, this is good coverage.
  • Pallet-side Code::try_new call sites all pass SyscallKind::Vara correctly (pallets/gear/src/lib.rs, runtime_api.rs, tests.rs, benchmarking/).
    · branch: av/ethexe-check-syscalls

@StackOverflowExcept1on StackOverflowExcept1on added the ci: full Run full CI matrix (all platforms and profiles) label May 5, 2026
@StackOverflowExcept1on StackOverflowExcept1on changed the title feat(workspace): add global separation of syscall kind refactor!(workspace): add global separation of syscall kind May 5, 2026
@StackOverflowExcept1on StackOverflowExcept1on added the type: refactor Internal improvements without changing behavior label May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci: full Run full CI matrix (all platforms and profiles) type: refactor Internal improvements without changing behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(gear-wasm-builder): Consider adding syscall (validity) checking into gear-wasm-builder

2 participants