feat!: carry the generation verdict on a manifest page - #52
Merged
Conversation
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
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.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #5.
An empty manifest page already carried
reasonCode, so the issue is not entirely as written. ButreasonCodeis 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_decodeErrorToGeneratorResultand then discarded on the way out.The change
getManifestPagereturns aManifestStatusin place of the bare selector:codeis the same verdictpollwould 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.reasonCodeis unchanged and remains the handler-declared detail, useful once a consumer does know the handler.An ordinary page reports
POSTwith zeroes, including a page that is empty only becauseoffsetis past the end. That is what separates "nothing here" from "nothing yet": both return no entries, and onlycodedistinguishes them.All three implementations move together
Two helpers on
BaseConditionalOrder,_manifestOk()and_manifestStatus(errorData), so the error-to-status mapping is stated once.TWAPandPerpetualStableSwapcall them rather than restating it, which is what keeps the three from drifting apart later.TWAP's uninitialised-context page now reportsTRY_NEXT_BLOCKrather 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 --checkclean,forge buildclean.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):Both fail without the fix, so they are testing the behaviour rather than passing either way.
How to test