Skip to content

feat(executor): expose return codec in ApplyRet#2264

Merged
rvagg merged 2 commits intomasterfrom
rvagg/return_codec
Mar 6, 2026
Merged

feat(executor): expose return codec in ApplyRet#2264
rvagg merged 2 commits intomasterfrom
rvagg/return_codec

Conversation

@rvagg
Copy link
Member

@rvagg rvagg commented Mar 4, 2026

This is part of FIP-0107 which may or may not be accepted, but it's also independent because this is a long-standing TODO. I attempted to find a good way to test this but currently we have no infra that runs a test actor returning a result with a known codec. I judged this trivial enough to just go ahead with without the test.

This is a breaking API change though because of the addition to ApplyRet. I have a follow-up for filecoin-ffi but Forest will need to deal with it separately.

@rvagg rvagg force-pushed the rvagg/return_codec branch from 68c5bce to 0ca9e00 Compare March 4, 2026 08:13
@codecov-commenter
Copy link

codecov-commenter commented Mar 4, 2026

Codecov Report

❌ Patch coverage is 73.80952% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.58%. Comparing base (8e2f86b) to head (b6052ae).

Files with missing lines Patch % Lines
fvm/src/executor/default.rs 75.60% 10 Missing ⚠️
fvm/src/executor/mod.rs 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2264      +/-   ##
==========================================
- Coverage   77.59%   77.58%   -0.02%     
==========================================
  Files         147      147              
  Lines       15851    15865      +14     
==========================================
+ Hits        12300    12309       +9     
- Misses       3551     3556       +5     
Files with missing lines Coverage Δ
fvm/src/executor/mod.rs 20.58% <0.00%> (-0.63%) ⬇️
fvm/src/executor/default.rs 77.44% <75.60%> (-0.28%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the FVM executor API to expose the IPLD codec associated with top-level return data, addressing a long-standing TODO and aligning ApplyRet with existing syscall-level return metadata.

Changes:

  • Add a new return_codec: u64 field to ApplyRet (with 0 indicating no return data).
  • Plumb the return codec through the default executor by extracting it from the invocation return block and returning it alongside the receipt.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
fvm/src/executor/mod.rs Extends ApplyRet with a return_codec field and initializes it in prevalidation_fail.
fvm/src/executor/default.rs Extracts the codec from the invocation return block and propagates it into the produced ApplyRet.

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

rvagg added a commit to filecoin-project/filecoin-ffi that referenced this pull request Mar 4, 2026
Ref: filecoin-project/ref-fvm#2264

Add return_codec to FvmMachineExecuteResponse and populate it from
the FVM ApplyRet. The v2/v3 engine adapters default to 0. The Go
bindings read it into the ApplyRet.ReturnCodec field.
@github-project-automation github-project-automation bot moved this from 📌 Triage to ✔️ Approved by reviewer in FilOz Mar 6, 2026
@rvagg rvagg merged commit 37457a9 into master Mar 6, 2026
17 checks passed
@rvagg rvagg deleted the rvagg/return_codec branch March 6, 2026 07:48
@github-project-automation github-project-automation bot moved this from ✔️ Approved by reviewer to 🎉 Done in FilOz Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🎉 Done

Development

Successfully merging this pull request may close these issues.

4 participants