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.

Copilot · 2025-11-21T00:51:50Z

rust/src/fvm/machine.rs

+            // Track the most recently created machine for reservation sessions.
+            {
+                let mut current = CURRENT_MACHINE
+                    .lock()
+                    .map_err(|e| anyhow!("current executor lock poisoned: {e}"))?;
+                *current = MachinePtr(&*boxed as *const InnerFvmMachine);
+            }


The global CURRENT_MACHINE stores a raw pointer to the most recently created InnerFvmMachine, but this pointer is never cleared when the machine is destroyed via drop_fvm_machine. This creates a use-after-free vulnerability: if a machine is destroyed and then FVM_BeginReservations or FVM_EndReservations is called, the code will dereference a dangling pointer at lines 685 and 747.

Recommendation: Either:

Clear CURRENT_MACHINE in the drop_fvm_machine destructor, or

Use a different design such as passing the machine pointer as a parameter to the reservation functions, or

Use a weak reference or reference counting scheme to detect when the machine has been destroyed.

Copilot · 2025-11-21T00:51:51Z

rust/src/fvm/machine.rs

+    let machine_ptr = match CURRENT_MACHINE.lock() {
+        Ok(guard) => guard.0,
+        Err(_) => {
+            set_reservation_error_message_out(
+                error_msg_ptr_out,
+                error_msg_len_out,
+                "reservation invariant violated: current executor lock poisoned",
+            );
+            return FvmReservationStatus::ErrReservationInvariant;
+        }
+    };
+
+    if machine_ptr.is_null() {
+        set_reservation_error_message_out(
+            error_msg_ptr_out,
+            error_msg_len_out,
+            "reservations not implemented for current machine",
+        );
+        return FvmReservationStatus::ErrNotImplemented;
+    }
+
+    // SAFETY: the pointer is set when the machine is created and
+    // is expected to remain valid while reservations are used.
+    let inner = unsafe { &*machine_ptr };
+    let machine_mutex = match &inner.machine {


The safety comment states "the pointer is set when the machine is created and is expected to remain valid while reservations are used," but there's no mechanism to enforce this. If multiple machines are created (which is allowed), CURRENT_MACHINE will point to the most recent one. If that machine is destroyed while an older machine is still in use, this pointer becomes dangling. Additionally, if reservations are used across multiple machines concurrently, this global state will cause race conditions where the wrong machine's reservations are accessed.

Recommendation: The reservation functions should take the machine pointer as an explicit parameter rather than relying on global state, ensuring type-safe lifetime management.

Suggested change

let machine_ptr = match CURRENT_MACHINE.lock() {

Ok(guard) => guard.0,

Err(_) => {

set_reservation_error_message_out(

error_msg_ptr_out,

error_msg_len_out,

"reservation invariant violated: current executor lock poisoned",

);

return FvmReservationStatus::ErrReservationInvariant;

}

};

if machine_ptr.is_null() {

set_reservation_error_message_out(

error_msg_ptr_out,

error_msg_len_out,

"reservations not implemented for current machine",

);

return FvmReservationStatus::ErrNotImplemented;

}

// SAFETY: the pointer is set when the machine is created and

// is expected to remain valid while reservations are used.

let inner = unsafe { &*machine_ptr };

let machine_mutex = match &inner.machine {

// The machine pointer/reference should be passed as an explicit parameter to this function.

// Remove reliance on CURRENT_MACHINE global state.

//

// Example: fn reservation_function(machine: &Machine, ...)

//

// Use the passed-in machine reference directly:

let machine_mutex = match &machine.machine {

Copilot · 2025-11-21T00:51:51Z

fvm.go

+func (f *FVM) BeginReservations(plan []byte) error {
+	defer runtime.KeepAlive(f)
+	status, msg := cgo.FvmBeginReservations(cgo.AsSliceRefUint8(plan))
+	baseErr := ReservationStatusToError(status)
+	if baseErr == nil {
+		return nil
+	}
+	if msg == "" {
+		return baseErr
+	}
+	return fmt.Errorf("%w: %s", baseErr, msg)
+}


The BeginReservations and EndReservations methods are called on an FVM instance but don't actually pass f.executor to the underlying CGO functions. This means these methods operate on whatever machine is stored in the global CURRENT_MACHINE rather than the machine associated with this FVM instance. This can lead to incorrect behavior if multiple FVM instances exist concurrently - calling BeginReservations on one instance could affect a different instance's machine.

Recommendation: Pass f.executor to the CGO functions, and update the CGO and Rust FFI signatures to accept a machine pointer parameter.

Copilot · 2025-11-21T00:51:51Z

fvm.go

+func (f *FVM) EndReservations() error {
+	defer runtime.KeepAlive(f)
+	status, msg := cgo.FvmEndReservations()
+	baseErr := ReservationStatusToError(status)
+	if baseErr == nil {
+		return nil
+	}
+	if msg == "" {
+		return baseErr
+	}
+	return fmt.Errorf("%w: %s", baseErr, msg)
+}


Same issue as in BeginReservations: the method doesn't pass f.executor to the underlying CGO function, operating on global state instead of the instance's machine.

Copilot · 2025-11-21T00:51:52Z

rust/src/fvm/machine.rs

+    // Empty plan is a no-op and must not enter reservation mode.
+    if cbor_plan_len == 0 {
+        return FvmReservationStatus::Ok;
+    }


[nitpick] The early return for an empty plan (lines 622-625) assumes that an empty plan should be a no-op. However, this means that calling BeginReservations with an empty plan will return Ok without actually entering reservation mode or validating that the machine supports reservations. If the caller then calls EndReservations, it may get an ErrSessionClosed error because no session was opened. This asymmetry could be confusing.

Recommendation: Consider documenting this behavior clearly, or alternatively, still validate that the machine supports reservations even for empty plans (though this may be intentional to avoid overhead).

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

snissn commented Nov 18, 2025

Uh oh!

snissn commented Nov 18, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Copilot AI Nov 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

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

Uh oh!

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!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 21, 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