Multistage execution: Add reservation Begin/End FFI bindings #554

snissn · 2025-11-18T19:56:34Z

Summary

This PR adds FFI bindings for the new tipset reservation session API required by the Multi‑Stage Tipset Gas Reservations work.

Specifically, it:

Extends the C FFI with Begin/End reservation session entrypoints and associated status codes.
Wires new Go wrappers in cgo/fvm.go to call the reservation API and surface structured errors to Lotus.
Introduces a new Rust reservations module in rust/src/fvm and hooks it into mod.rs.

These bindings are used by Lotus to start and end a reservation session around tipset execution while keeping the reservation implementation inside ref‑fvm.

Motivation

Lotus and ref‑fvm are adding tipset‑scope gas reservations to eliminate miner‑penalty exposure from intra‑tipset self‑drain scenarios, as described in the draft FIP
“Multi‑Stage Tipset Gas Reservations”.

filecoin‑ffi needs to expose the engine’s reservation session API (Begin/End and error reporting) to Go so that Lotus can orchestrate reservations at the tipset boundary
without introducing new on‑chain actors or migrations.

Changes

cgo/fvm.go:
- Adds FFI wrappers that call the engine’s reservation Begin/End API.
- Propagates reservation error codes and messages into Go.
cgo/fvm_reservations.go:
- New Go helpers for reservation management, used by Lotus’ FVM VM wrapper.
rust/src/fvm/mod.rs:
- Registers the new reservations submodule.
rust/src/fvm/reservations.rs:
- Defines the Rust side of the reservation FFI surface.

Testing

go test ./... in this repo.
Rebuilt Lotus with this branch as the extern/filecoin-ffi submodule and ran the existing FVM/consensus test suites that exercise FFI calls.

snissn · 2025-11-18T19:57:34Z

relevant PRs:

filecoin-project/FIPs#1218
filecoin-project/ref-fvm#2236
filecoin-project/lotus#13427
#554

Copilot

Pull Request Overview

This PR adds FFI bindings for FVM reservation session management, enabling Lotus to orchestrate gas reservations at tipset boundaries. The implementation spans Rust FFI exports, CGO wrappers, and Go public APIs.

Key Changes:

Adds FVM_BeginReservations and FVM_EndReservations FFI exports with status codes and error messages
Introduces reservation error types and status-to-error mapping in both Rust and Go
Implements error message allocation/deallocation across the FFI boundary

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
rust/src/fvm/machine.rs	Implements FFI exports for Begin/End reservation sessions, error message handling, and tracks current machine in global state
rust/src/fvm/engine.rs	Defines `ReservationError` enum and default trait methods for reservation operations
cgo/fvm.go	Provides CGO wrappers that invoke C ABI and manage error message memory
fvm.go	Exposes typed reservation errors and high-level `BeginReservations`/`EndReservations` methods on FVM instances
cgo/fvm_reservations_test.go	Tests error message handling for invalid reservation calls

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

rust/src/fvm/machine.rs

fvm.go

rust/src/fvm/machine.rs

Removes the CURRENT_MACHINE global in Rust and passes the executor pointer explicitly through CGO to Rust. This prevents safety issues with concurrent FVM instances.

…lans

…rap()

…p usage

Copilot

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

rust/src/fvm/machine.rs

Copilot

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

PR_tipset_reservations_ffi.md

Copilot

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-09T03:34:42Z

rust/src/fvm/machine.rs

 const LOTUS_FVM_CONCURRENCY_ENV_NAME: &str = "LOTUS_FVM_CONCURRENCY";
 const VALID_CONCURRENCY_RANGE: RangeInclusive<u32> = 1..=256;

+#[derive_ReprC]


The derive_ReprC attribute appears to be used here, but it's not a standard Rust derive macro. This seems to be from the safer_ffi crate or similar. Please verify that this crate is properly included in dependencies and that this is the intended FFI representation mechanism.

Copilot · 2025-12-09T03:34:42Z

rust/src/fvm/machine.rs

+    // Enforce a maximum encoded plan size (256 KiB).
+    // This is based on a maximum of 4096 entries at 64 bytes per entry.
+    const MAX_PLAN_BYTES: usize = 256 * 1024;


The MAX_PLAN_BYTES constant of 256 KiB (based on 4096 entries × 64 bytes) should be validated against the actual CBOR encoding size of a Vec<(Address, TokenAmount)>. The comment suggests 64 bytes per entry, but a CBOR-encoded Address (41+ bytes) plus TokenAmount (variable) plus CBOR overhead may exceed this estimate, especially for addresses with longer protocols or large token amounts. Consider adding a test that verifies a plan with 4096 entries actually fits within this limit.

Copilot · 2025-12-09T03:34:42Z

rust/src/fvm/machine.rs

+    let plan: Vec<(Address, TokenAmount)> = match fvm_ipld_encoding::from_slice(bytes) {
+        Ok(p) => p,
+        Err(_) => {
+            set_reservation_error_message_out(
+                error_msg_ptr_out,
+                error_msg_len_out,
+                "reservation invariant violated: invalid plan CBOR encoding",
+            );
+            return FvmReservationStatus::ErrReservationInvariant;
+        }


The CBOR decoding error at line 643 is classified as ErrReservationInvariant with message "invalid plan CBOR encoding". However, this is not an invariant violation—it's invalid input from the caller. Consider either: (1) Adding a new error variant like InvalidPlanEncoding to distinguish client errors from internal invariant violations, or (2) Accepting any decoding error as OK with an empty-plan semantics if the intent is to be lenient. The current classification may make debugging harder by conflating different error categories.

Copilot · 2025-12-09T03:34:42Z

rust/src/fvm/machine.rs

+    fn reservation_error_mapping_sets_message_for_insufficient_funds() {
+        let mut msg_ptr: *const u8 = std::ptr::null();
+        let mut msg_len: usize = 0;
+
+        let status = map_reservation_error_to_status(
+            ReservationError::InsufficientFundsAtBegin { sender: 42 },
+            &mut msg_ptr,
+            &mut msg_len,
+        );
+
+        assert_eq!(status, FvmReservationStatus::ErrInsufficientFundsAtBegin);
+        assert!(!msg_ptr.is_null());
+        assert!(msg_len > 0);
+
+        let msg = unsafe {
+            let slice = std::slice::from_raw_parts(msg_ptr, msg_len);
+            std::str::from_utf8(slice).expect("reservation error message is valid UTF-8")
+        };
+        assert!(
+            msg.contains("sender 42"),
+            "expected message to contain sender id, got {msg}"
+        );
+
+        FVM_DestroyReservationErrorMessage(msg_ptr as *mut u8, msg_len);
+    }


[nitpick] The map_reservation_error_to_status function at line 972-976 tests InsufficientFundsAtBegin with a hardcoded sender ID of 42, but the test doesn't verify that this sender ID actually appears in the returned error message. The assertion at line 987 uses contains("sender 42") which could pass even if the formatting is incorrect. Consider adding an assertion that checks for the exact expected format or at minimum verifies the sender ID is present as a number.

snissn added 3 commits November 14, 2025 12:03

feat: add FVM reservation Begin/End FFI with error messages

89190ab

docs: add PR description for reservation FFI

88f0ff0

Add reservation FFI bindings

b503fd1

github-project-automation bot added this to FilOz Nov 18, 2025

github-project-automation bot moved this to 📌 Triage in FilOz Nov 18, 2025

snissn added 4 commits November 20, 2025 12:31

rust fmt

39e29cb

Fix reservation FFI build with FVM4

fe1f3de

Guard reservation cgo test behind cgo build tag

2e40d10

Fix reservation error message test without cgo import

386cbb2

snissn marked this pull request as ready for review November 21, 2025 00:47

Copilot AI review requested due to automatic review settings November 21, 2025 00:47

Copilot started reviewing on behalf of snissn November 21, 2025 00:48 View session

Copilot finished reviewing on behalf of snissn November 21, 2025 00:50

Copilot AI reviewed Nov 21, 2025

View reviewed changes

rust/src/fvm/machine.rs Outdated Show resolved Hide resolved

rust/src/fvm/machine.rs Outdated Show resolved Hide resolved

fvm.go Show resolved Hide resolved

fvm.go Show resolved Hide resolved

rust/src/fvm/machine.rs Show resolved Hide resolved

BigLep requested a review from ZenGround0 December 1, 2025 19:32

BigLep assigned snissn Dec 1, 2025

BigLep moved this from 📌 Triage to 🔎 Awaiting Review in FilOz Dec 1, 2025

snissn and others added 8 commits December 3, 2025 00:09

chore: patch fvm dependencies to local path for lotus build

8a59793

Merge remote-tracking branch 'upstream/master' into multistage-execution

9416272

Refactor FVM reservations to use explicit machine instance

4e2afd6

Removes the CURRENT_MACHINE global in Rust and passes the executor pointer explicitly through CGO to Rust. This prevents safety issues with concurrent FVM instances.

Remove local path patches from Cargo.toml to fix CI build

4b3b977

Downgrade home crate to 0.5.11 to support rustc 1.86.0

25499b1

Validate executor existence in FVM_BeginReservations even for empty p…

bcfd395

…lans

Fix: Replace deprecated TempDir::into_path() with TempDir::keep().unw…

f7b89ce

…rap()

Fix missing vars in test_faulty_sectors_inner and update TempDir::kee…

ab095bd

…p usage

snissn requested a review from Copilot December 6, 2025 00:25

Copilot started reviewing on behalf of snissn December 6, 2025 00:26 View session

Copilot AI reviewed Dec 6, 2025

View reviewed changes

rust/src/fvm/machine.rs Outdated Show resolved Hide resolved

Gemini Agent added 2 commits December 5, 2025 14:56

Reduce MAX_PLAN_BYTES to 256 KiB to prevent potential DoS

4419540

Fix TempDir keep usage for tempfile 3.23

ad0973e

snissn requested a review from Copilot December 6, 2025 02:22

Copilot started reviewing on behalf of snissn December 6, 2025 02:22 View session

Copilot AI reviewed Dec 6, 2025

View reviewed changes

Gemini Agent added 2 commits December 8, 2025 17:21

docs: fix PR description to match implementation

9bdbcf1

docs: refine test coverage description

f1c96b2

snissn requested a review from Copilot December 9, 2025 03:28

Copilot started reviewing on behalf of snissn December 9, 2025 03:29 View session

Copilot AI reviewed Dec 9, 2025

View reviewed changes

Multistage execution: Add reservation Begin/End FFI bindings #554

Are you sure you want to change the base?

Multistage execution: Add reservation Begin/End FFI bindings #554

Conversation

snissn commented Nov 18, 2025

Summary

Motivation

Changes

Testing

Uh oh!

snissn commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

snissn commented Nov 18, 2025 •

edited

Loading