Skip to content

fix(f02-mocks): match encoding format against f02's own vectors - #20

Merged
wjmelements merged 17 commits into
mainfrom
rvagg/f02-mock-wireformat
Aug 11, 2026
Merged

fix(f02-mocks): match encoding format against f02's own vectors#20
wjmelements merged 17 commits into
mainfrom
rvagg/f02-mock-wireformat

Conversation

@rvagg

@rvagg rvagg commented Aug 10, 2026

Copy link
Copy Markdown
Member

Also:

  • one set of CBOR primitives
  • addresses encode in both forms a contract can hold
  • add tryStepWeightRecords over a shared encoder, and guard each field against the width f02 declares
  • FVMRewardWire.t.sol mirrors actors/reward/tests/types_test.rs, asserting the bytes the mock received

@FilOzzy FilOzzy added this to FOC Aug 10, 2026
@github-project-automation github-project-automation Bot moved this to 📌 Triage in FOC Aug 10, 2026
@rvagg
rvagg requested review from wjmelements and a lite review from Copilot August 10, 2026 10:40

Copilot AI left a comment

Copy link
Copy Markdown

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 aligns the Solidity f02 (Reward actor) wire encoding/decoding with f02’s own DAG-CBOR serialization vectors, so the production encoder can be validated against canonical on-chain expectations instead of only matching the local mock.

Changes:

  • Reworks FVMRewards to use shared CBOR primitives, shortest-form integer encoding, explicit width-guards, and adds tryStepWeightRecords.
  • Updates the mock Reward actor and its tests to match the new parameter tuple shapes (single-field tuple wrappers, distribution encoding, cancel-pending null-id semantics).
  • Adds FVMRewardWire.t.sol vector tests that assert the exact bytes dispatched to the mock, mirroring f02’s upstream vectors.

Reviewed changes

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

File Description
src/lib/FVMRewards.sol Replaces ad-hoc encoders with shared DAG-CBOR primitives, adds width guards and StepWeightRecords support.
test/mocks/FVMRewardActor.sol Updates mock decoding/behavior to match updated wire format; adds param-recording and claim-return override hooks.
test/mocks/FVMRewardActor.t.sol Refactors tests to use new library wrappers and new registration/distribution semantics.
test/mocks/FVMRewardWire.t.sol New “golden vector” tests pinning exact bytes emitted by the production encoder.
Suppressed comments (2)

test/mocks/FVMRewardActor.sol:818

  • _decodeAddress accepts any protocol-4 byte string of length 22 as an EVM address, but the comment specifies protocol-4 with the EAM namespace (0x0a). Without checking the namespace byte, non-EAM delegated addresses would be mis-decoded as valid EVM addresses.
        } else if (protocol == 4 && len == 22) {
            assembly ("memory-safe") {
                addr := shr(96, calldataload(add(pos, 2)))
            }
        }

test/mocks/FVMRewardActor.sol:787

  • This doc comment still describes decoding an f410 address and/or CBOR null, but _decodeAddress no longer handles null and now supports both protocol-0 ID addresses and protocol-4 delegated addresses. Updating the comment avoids misleading future edits.
    /// @dev Decodes an f410 delegated address wrapped in a CBOR byte string (0x04, 0x0a, 20
    /// bytes) at absolute calldata position `pos`, or CBOR null (address(0)) for an IMPLICIT
    /// stream's absent writer. The address bytes are big-endian, so one `calldataload` shifted
    /// into place reads all 20 at once.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/lib/FVMRewards.sol
Comment thread src/lib/FVMRewards.sol
Comment thread test/mocks/FVMRewardActor.sol
@rvagg
rvagg force-pushed the rvagg/f02-mock-wireformat branch from cfba718 to 079cf15 Compare August 10, 2026 10:50
@rvagg

rvagg commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

@wjmelements anticipating one of your objections: I did replace your optimised, inlined cbor with a more compact form that could share across the implementations. forge snapshot suggests it's only a ~4k gas regression for most calls (EVM gas, but probably not a terrible proxy for scale in FEVM, and it's easily dwarfed by the call_actor either side of it anyway). IMO that's worth it for the clarity and maintainability, but it's up to you: you can unwind it if you want to - the tests in here assert against hex generated by f02 itself (actors/reward/tests/types_test.rs, and we'll mirror those in go-state-types too), so if they pass we agree with the chain.

@rvagg
rvagg force-pushed the rvagg/f02-mock-wireformat branch from 079cf15 to 3473400 Compare August 10, 2026 11:19
Base automatically changed from rvagg/f02-mockfixes to main August 10, 2026 15:21
Also:

- one set of CBOR primitives
- addresses encode in both forms a contract can hold
- add tryStepWeightRecords over a shared encoder, and guard each field against
  the width f02 declares
- FVMRewardWire.t.sol mirrors actors/reward/tests/types_test.rs, asserting the
  bytes the mock received
@wjmelements
wjmelements force-pushed the rvagg/f02-mock-wireformat branch from 3473400 to 904a142 Compare August 10, 2026 15:21
Comment thread src/lib/FVMRewards.sol Outdated
Comment thread test/mocks/FVMRewardActor.sol Outdated
Comment thread test/mocks/FVMRewardWire.t.sol Outdated
Comment thread src/lib/FVMRewards.sol Outdated
Comment thread src/lib/FVMRewards.sol Outdated
Comment thread test/mocks/FVMRewardActor.t.sol Outdated
Comment thread src/lib/FVMRewards.sol Outdated
Comment thread src/lib/FVMRewards.sol Outdated
Comment thread src/lib/FVMRewards.sol
Comment thread src/lib/FVMRewards.sol Outdated
Comment thread src/lib/FVMRewards.sol Outdated
Assisted-by: Claude:claude-sonnet-4-6
Comment thread src/lib/FVMRewards.sol
Comment thread src/lib/FVMRewards.sol Outdated
Comment thread src/lib/FVMRewards.sol Outdated
Comment thread src/lib/FVMRewards.sol
Comment thread src/lib/FVMRewards.sol Outdated
Comment thread src/lib/FVMRewards.sol Outdated
Comment thread src/lib/FVMRewards.sol Outdated
@github-project-automation github-project-automation Bot moved this from 📌 Triage to ✔️ Approved by reviewer in FOC Aug 11, 2026
@rvagg
rvagg requested a review from wjmelements August 11, 2026 07:51
Comment thread src/lib/FVMRewards.sol Outdated
Comment on lines -43 to -44
/// @dev An implicit stream (null distribution) carries no share map.
error ImplicitStreamWithShares();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nice

@wjmelements wjmelements left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nice

@wjmelements
wjmelements merged commit 9932d3d into main Aug 11, 2026
3 checks passed
@github-project-automation github-project-automation Bot moved this from ✔️ Approved by reviewer to 🎉 Done in FOC Aug 11, 2026
@wjmelements
wjmelements deleted the rvagg/f02-mock-wireformat branch August 11, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants