-
Notifications
You must be signed in to change notification settings - Fork 137
Multistage execution: Add reservation Begin/End FFI bindings #554
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.
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
snissn
wants to merge
19
commits into
filecoin-project:master
Choose a base branch
from
snissn:multistage-execution
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 17 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
89190ab
feat: add FVM reservation Begin/End FFI with error messages
snissn 88f0ff0
docs: add PR description for reservation FFI
snissn b503fd1
Add reservation FFI bindings
snissn 39e29cb
rust fmt
snissn fe1f3de
Fix reservation FFI build with FVM4
snissn 2e40d10
Guard reservation cgo test behind cgo build tag
snissn 386cbb2
Fix reservation error message test without cgo import
snissn 8a59793
chore: patch fvm dependencies to local path for lotus build
snissn 9416272
Merge remote-tracking branch 'upstream/master' into multistage-execution
4e2afd6
Refactor FVM reservations to use explicit machine instance
4b3b977
Remove local path patches from Cargo.toml to fix CI build
25499b1
Downgrade home crate to 0.5.11 to support rustc 1.86.0
bcfd395
Validate executor existence in FVM_BeginReservations even for empty p…
f7b89ce
Fix: Replace deprecated TempDir::into_path() with TempDir::keep().unw…
ab095bd
Fix missing vars in test_faulty_sectors_inner and update TempDir::kee…
4419540
Reduce MAX_PLAN_BYTES to 256 KiB to prevent potential DoS
ad0973e
Fix TempDir keep usage for tempfile 3.23
9bdbcf1
docs: fix PR description to match implementation
f1c96b2
docs: refine test coverage description
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| # feat: add FVM reservation Begin/End FFI with error messages | ||
|
|
||
| This PR wires the ref‑fvm reservation session API through filecoin‑ffi and extends the Begin/End FFI to return short human‑readable error messages alongside status codes. Lotus uses the status codes for gating and the messages for logging; ref‑fvm remains network‑version agnostic. | ||
|
|
||
| ## Summary | ||
|
|
||
| - Add C ABI exports for `FVM_BeginReservations` / `FVM_EndReservations` with status codes plus optional error strings. | ||
| - Provide Go wrappers that map status codes to typed errors and attach the engine‑provided message when present. | ||
| - Keep the status‑code contract as the only semantic signal for hosts (messages are logging only). | ||
|
|
||
| ## Changes | ||
|
|
||
| ### Cgo surface and Go bindings | ||
|
|
||
| - **`cgo/fvm.go`** | ||
| - Extend C declarations: | ||
| - `int32_t FVM_BeginReservations(const uint8_t *cbor_plan_ptr, size_t cbor_plan_len, const uint8_t **error_msg_ptr, size_t *error_msg_len);` | ||
| - `int32_t FVM_EndReservations(const uint8_t **error_msg_ptr, size_t *error_msg_len);` | ||
|
snissn marked this conversation as resolved.
Outdated
|
||
| - `void FVM_DestroyReservationErrorMessage(uint8_t *error_msg_ptr, size_t error_msg_len);` | ||
| - Add Go wrappers: | ||
| - `func FvmBeginReservations(plan SliceRefUint8) (int32, string)` | ||
|
snissn marked this conversation as resolved.
Outdated
|
||
| - `func FvmEndReservations() (int32, string)` | ||
|
snissn marked this conversation as resolved.
Outdated
|
||
| - Behaviour: | ||
| - Call the C ABI, copy the returned message (if any) into Go memory, and free the FFI allocation via `FVM_DestroyReservationErrorMessage`. | ||
|
|
||
| - **`fvm.go`** | ||
| - Define typed reservation errors: | ||
| - `ErrReservationsNotImplemented` | ||
| - `ErrReservationsInsufficientFunds` | ||
| - `ErrReservationsSessionOpen` | ||
| - `ErrReservationsSessionClosed` | ||
| - `ErrReservationsNonZeroRemainder` | ||
| - `ErrReservationsPlanTooLarge` | ||
| - `ErrReservationsOverflow` | ||
| - `ErrReservationsInvariantViolation` | ||
| - Implement: | ||
| - `ReservationStatusToError(code int32) error` mapping raw status codes to these sentinels. | ||
| - `(*FVM).BeginReservations(plan []byte) error`: | ||
| - Calls `cgo.FvmBeginReservations`, maps status to a typed error, and if a non‑empty message is present, wraps the error as `fmt.Errorf("%w: %s", baseErr, msg)`. | ||
| - `(*FVM).EndReservations() error` with analogous behaviour. | ||
| - These APIs are called by Lotus’ FVM wrapper (`chain/vm/fvm.go`) when orchestrating Begin/End reservations. | ||
|
|
||
| ### Rust FFI glue | ||
|
|
||
| - **`rust/src/fvm/machine.rs`** | ||
| - Introduce helper: | ||
| - `fn set_reservation_error_message_out(error_msg_ptr_out: *mut *const u8, error_msg_len_out: *mut usize, msg: &str)` to allocate and expose a short message over the FFI. | ||
| - Add `fn map_reservation_error_to_status(err: ReservationError, error_msg_ptr_out: *mut *const u8, error_msg_len_out: *mut usize) -> FvmReservationStatus` that: | ||
| - Maps `fvm4::executor::ReservationError` variants to `FvmReservationStatus`. | ||
| - Sets a short message string for non‑OK statuses, e.g.: | ||
| - `ErrInsufficientFundsAtBegin { sender }` → message including sender ID. | ||
| - `ErrReservationInvariant(reason)` → message including the invariant description. | ||
| - Lock poisoning or missing executor → `ErrReservationInvariant` with a descriptive message. | ||
| - Update: | ||
| - `FVM_BeginReservations(...) -> FvmReservationStatus` to: | ||
| - Decode the plan, look up the current `InnerFvmMachine`, and call `executor.begin_reservations(plan)`. | ||
| - On error, call `map_reservation_error_to_status` to populate both status and message. | ||
| - `FVM_EndReservations() -> FvmReservationStatus` similarly calls `executor.end_reservations()` and uses the mapping helper on error. | ||
| - Add tests that: | ||
| - Exercise `map_reservation_error_to_status` for representative errors (`InsufficientFundsAtBegin`, `ReservationInvariant`) and verify that: | ||
| - Status values are as expected. | ||
| - Returned messages are non‑empty and contain the relevant context. | ||
| - `FVM_DestroyReservationErrorMessage` can free the allocation safely. | ||
|
|
||
| ### Tests | ||
|
|
||
| - **`cgo/fvm_reservations_test.go`** | ||
| - `TestFvmBeginReservationsErrorMessage`: | ||
| - Calls `FvmBeginReservations` with a deliberately invalid plan pointer/length pair to trigger a Reservation invariant error. | ||
| - Asserts that the status code is non‑zero and the message is non‑empty. | ||
| - `TestFvmEndReservationsErrorMessage`: | ||
| - Calls `FvmEndReservations` without an active session. | ||
| - Asserts non‑zero status and non‑empty message. | ||
|
snissn marked this conversation as resolved.
Outdated
|
||
|
|
||
| ## Notes | ||
|
|
||
| - Hosts must continue to base consensus decisions purely on status codes (mapped to typed errors on the Go side). The new messages are for logging and operator diagnostics only. | ||
| - The FFI remains agnostic to network version; activation and gating are handled entirely in Lotus using the tipset network version and feature flags. | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.