Skip to content

feat!: carry the generation verdict on a manifest page - #52

Merged
mfw78 merged 1 commit into
developfrom
feat/manifest-status
Aug 1, 2026
Merged

feat!: carry the generation verdict on a manifest page#52
mfw78 merged 1 commit into
developfrom
feat/manifest-status

Conversation

@mfw78

@mfw78 mfw78 commented Aug 1, 2026

Copy link
Copy Markdown

Closes #5.

An empty manifest page already carried reasonCode, so the issue is not entirely as written. But reasonCode is the selector of an error the handler declares (TooEarly.selector, OrderNotInitialized.selector), so a consumer that has not been taught a particular handler's error names still cannot tell a not-yet-active order from a permanently invalid one. Both render as "no orders". The verdict and the wait timestamp were both decoded by _decodeErrorToGeneratorResult and then discarded on the way out.

The change

getManifestPage returns a ManifestStatus in place of the bare selector:

struct ManifestStatus {
    IConditionalOrderGenerator.GeneratorResultCode code;
    uint256 waitUntil;
    bytes4 reasonCode;
}

code is the same verdict poll would return for the same conditions, decoded through the same _decodeErrorToGeneratorResult. The manifest mirrors the poll path rather than becoming a second source of truth for whether an order is live. reasonCode is unchanged and remains the handler-declared detail, useful once a consumer does know the handler.

An ordinary page reports POST with zeroes, including a page that is empty only because offset is past the end. That is what separates "nothing here" from "nothing yet": both return no entries, and only code distinguishes them.

All three implementations move together

Two helpers on BaseConditionalOrder, _manifestOk() and _manifestStatus(errorData), so the error-to-status mapping is stated once. TWAP and PerpetualStableSwap call them rather than restating it, which is what keeps the three from drifting apart later.

TWAP's uninitialised-context page now reports TRY_NEXT_BLOCK rather than an unqualified empty page. A context that has not been written yet is transient, and reporting it as an ordinary empty page is the same conflation the issue is about.

Breaking

getManifestPage's third return value changes type. #46 established there are no deployments, so no consumer is affected; that is the reason for doing this now rather than adding a parallel accessor.

Verification

166 tests pass, forge fmt --check clean, forge build clean.

The two assertions that carry this change were checked against an implementation that decodes the verdict and then discards it, keeping only reasonCode (the old behaviour):

mutation applied: verdict discarded, reasonCode kept
  test_manifestPage_EmptyPageCarriesWaitReason:    FAIL
  test_manifestPage_EmptyPageCarriesInvalidReason: FAIL
restored: 166 tests passed

Both fail without the fix, so they are testing the behaviour rather than passing either way.

How to test

export FOUNDRY_PROFILE=ci
forge fmt --check && forge build && forge test --no-match-test fork --fuzz-seed 672679878

An empty manifest page already carried `reasonCode`, but that is the selector
of an error the *handler* declares, so a consumer that has not been taught a
particular handler's error names cannot tell a not-yet-active order from a
permanently invalid one. Both still render as "no orders". The verdict and the
wait timestamp were decoded and then discarded.

`getManifestPage` now returns a `ManifestStatus` in place of the bare selector:

    struct ManifestStatus {
        GeneratorResultCode code;
        uint256 waitUntil;
        bytes4 reasonCode;
    }

`code` is the same verdict `poll` would return for the same conditions, so the
manifest mirrors the poll path rather than becoming a second source of truth.
It is decoded through `_decodeErrorToGeneratorResult`, the function `poll`
itself uses, so the two cannot disagree. `reasonCode` is unchanged and remains
the handler-declared detail.

An ordinary page reports `POST` with zeroes, including one that is empty only
because `offset` is past the end. That is what distinguishes "nothing here" from
"nothing yet": both return no entries, and only `code` separates them.

All three implementations move together, so behaviour does not depend on which
handler a consumer happens to hit. `TWAP` and `PerpetualStableSwap` share the
two helpers added to `BaseConditionalOrder` rather than restating the mapping.
`TWAP`'s uninitialised-context page now reports `TRY_NEXT_BLOCK` rather than an
unqualified empty page, since a context that has not been written yet is a
transient condition.

The existing not-yet-active and permanently-invalid tests now assert on `code`
and `waitUntil`, and were checked against an implementation that discards the
verdict: both fail without the change.

BREAKING CHANGE: `getManifestPage` returns `ManifestStatus` rather than
`bytes4`. No deployments exist, so no consumer is affected.

Closes #5.
@mfw78
mfw78 merged commit 7108907 into develop Aug 1, 2026
1 check passed
@mfw78
mfw78 deleted the feat/manifest-status branch August 1, 2026 00:51
mfw78 added a commit that referenced this pull request Aug 1, 2026
`generateOrder` is reached three ways and `msg.sender` differs between them:
the registry under `verify`, which calls it internally, and the handler itself
under `poll` and `getManifestPage`, which reach it via `this.generateOrder` for
try/catch. A handler branching on `msg.sender` therefore derives one order
under settlement and another under polling.

Design principle 6 already forbade branching on `msg.sender`, but described
handlers as pure functions of their explicit arguments including `sender`. That
permits what `getManifestPage` cannot support: it passes `address(0)`, so a
handler whose output varied with `sender` could not produce a manifest matching
settlement. The stated mitigation for the `msg.sender` trap was itself
path-dependent.

Principle 6 now states the rule over the generated order rather than the
argument list, and a Caller Identity section records the three paths, the two
rules that follow, and that neither is enforced on-chain. `sender` stays in the
signature because gating admissibility by reverting is a legitimate use; it may
not alter the order's contents.

No handler in `src/types/` reads `msg.sender` or `sender`, so this constrains
future handlers rather than describing current ones.

Also corrects the manifest documentation, which still described the interface
as it stood before #52:

- The pagination contract had `getManifestPage` returning
  `(entries, hasMore, reasonCode)` and described the third value as a bare
  selector. It is a `ManifestStatus`, and `ManifestStatus` appeared nowhere in
  the docs.
- The `IOrderManifest` sketch declared the third return as `string memory
  status`, which predates even the typed-error work.

Closes #8.
mfw78 added a commit that referenced this pull request Aug 1, 2026
Documentation only, and independent of the caller-identity work now in
#56. Split out so each PR is written once rather than one revising the
other.

The manifest documentation still described the interface as it stood
before #52:

- **Pagination contract.** `getManifestPage` was described as returning
`(entries, hasMore, reasonCode)`, with the third value a bare selector.
It has been a `ManifestStatus` since #52, and `ManifestStatus` appeared
nowhere in the docs.
- **`IOrderManifest` sketch.** The third return was declared `string
memory status`, which predates even the typed-error work in #22.

Also records that an ordinary page reports `POST` with zero `waitUntil`
and `reasonCode`, including a page that is empty only because `offset`
is past the end. Both return no entries, and only `code` separates
"nothing here" from "nothing yet", which is the distinction the struct
exists to carry.

`forge fmt --check` clean, 166 tests pass. No source change.
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.

manifest: default getManifestPage hides the WAIT reason as an empty page

1 participant