feat!: add structured poll interface for watch-towers - #23
Merged
Conversation
25 tasks
mfw78
force-pushed
the
feat/typed-error-reasons
branch
from
July 30, 2026 03:50
06f7128 to
9635724
Compare
mfw78
force-pushed
the
feat/poll-interface
branch
2 times, most recently
from
July 30, 2026 04:01
4865c88 to
65e5073
Compare
mfw78
force-pushed
the
feat/typed-error-reasons
branch
2 times, most recently
from
July 30, 2026 06:59
23a005d to
d307d23
Compare
mfw78
force-pushed
the
feat/poll-interface
branch
from
July 30, 2026 06:59
65e5073 to
5e837ba
Compare
mfw78
force-pushed
the
feat/typed-error-reasons
branch
from
July 30, 2026 07:02
d307d23 to
54400e6
Compare
mfw78
force-pushed
the
feat/poll-interface
branch
from
July 30, 2026 07:02
5e837ba to
02f9536
Compare
mfw78
force-pushed
the
feat/typed-error-reasons
branch
from
July 30, 2026 07:07
54400e6 to
87c7ffe
Compare
mfw78
force-pushed
the
feat/poll-interface
branch
from
July 30, 2026 07:07
02f9536 to
af470ad
Compare
mfw78
force-pushed
the
feat/typed-error-reasons
branch
from
July 30, 2026 07:09
87c7ffe to
d912654
Compare
mfw78
force-pushed
the
feat/poll-interface
branch
2 times, most recently
from
July 30, 2026 07:14
2c8760b to
da512da
Compare
mfw78
force-pushed
the
feat/typed-error-reasons
branch
2 times, most recently
from
July 30, 2026 07:18
2b43dc5 to
a439033
Compare
mfw78
force-pushed
the
feat/poll-interface
branch
from
July 30, 2026 07:18
da512da to
a81e4a1
Compare
mfw78
force-pushed
the
feat/typed-error-reasons
branch
from
July 30, 2026 07:22
a439033 to
9cd3b1b
Compare
mfw78
force-pushed
the
feat/poll-interface
branch
from
July 30, 2026 07:22
a81e4a1 to
3997bec
Compare
mfw78
force-pushed
the
feat/typed-error-reasons
branch
from
July 30, 2026 07:26
9cd3b1b to
0d8d9a3
Compare
mfw78
force-pushed
the
feat/poll-interface
branch
from
July 30, 2026 07:26
3997bec to
12f5b7d
Compare
mfw78
force-pushed
the
feat/poll-interface
branch
from
July 30, 2026 07:33
12f5b7d to
fafabe0
Compare
This was referenced Jul 30, 2026
mfw78
force-pushed
the
feat/poll-interface
branch
2 times, most recently
from
July 30, 2026 08:42
db7773d to
89871fe
Compare
mfw78
commented
Jul 30, 2026
Introduce the polling half of the dual-path architecture: - move generateOrder into IConditionalOrder as the single source of truth shared by verification and polling - add GeneratorResult / GeneratorResultCode to IConditionalOrderGenerator: poll() never reverts for order conditions, returning a structured verdict instead - add getNextPollTimestamp and describeOrder scheduling/UX hooks with sensible defaults in BaseConditionalOrder - decode recognized conditional-order errors into verdicts; map unrecognized reverts (including Panic and Error(string)) to TRY_NEXT_BLOCK so transient faults reschedule instead of permanently killing an order off-chain Fill state is deliberately absent from the generator surface: it is observed by the registry, not the handler, and lands with the registry poll path. BREAKING CHANGE: the IConditionalOrderGenerator ERC-165 interface id changes; existing deployed handlers will not satisfy the new id.
mfw78
force-pushed
the
feat/poll-interface
branch
from
July 30, 2026 09:51
89871fe to
f7a0b44
Compare
mfw78
added a commit
that referenced
this pull request
Jul 30, 2026
Reshape `getTradeableOrderWithSignature` around the generator verdict plus an orthogonal, registry-observed fill overlay: - `PollResult` composes the handler `GeneratorResult` with a `FillStatus` (NONE / PARTIALLY_FILLED / FILLED / INVALIDATED) derived from `GPv2Settlement.filledAmount`; `invalidateOrder` reports as INVALIDATED, not FILLED (closes #11) - a partially filled `partiallyFillable` order keeps returning its signature, so the remainder is no longer stranded (advances #10) - `checkOrder` returns the same composed `PollResult` and applies the ERC-165 handler gate via the shared `_poll` helper (closes #9, closes #6) - `create`/`setRoot` write the cabinet before emitting and carry the resolved value in a new `bytes context` event field (closes #14) - `ConditionalOrderRemoved` emitted on removal - fill-overlay matrix covered by tests (advances #2) Together with the generator half this delivers the verdict/fill split (closes #16). **BREAKING**: `getTradeableOrderWithSignature` returns `(PollResult, bytes)`; `ConditionalOrderCreated` and `MerkleRootSet` gain a `bytes context` parameter (topic0 changes). ## Issues Closes #6. Closes #9. Closes #11. Closes #14. Closes #16 (completed jointly with #23, which landed the generator half). Advances #2 (adds the FILLED / PARTIALLY_FILLED / INVALIDATED overlay matrix, but not the two remaining checks that issue asks for: the orderUid verified against a known-good GPv2 uid - `_mockFilledAmount` mocks `filledAmount(bytes)` with no calldata argument, so any uid matches - and a KIND_BUY vs KIND_SELL total-amount case; every overlay test uses KIND_SELL). Advances #10 (fixes the stranding: a partially filled `partiallyFillable` order keeps its signature. The issue's second half is untouched - `_getFilledAmount` still keys on the per-poll orderUid, so a bounded-total intent whose orderUid moves between polls still cannot be tracked). ### `_setCabinet` helper `setRootWithContext` and `createWithContext` performed the same three steps -- resolve the factory value, write it to the caller's cabinet slot, abi-encode it for the event -- differing only in the key. That is now one internal helper taking the key; the caller is always `msg.sender`. Measured, since the motivation was gas: this is **not** a gas win. On the one fully deterministic test exercising the path, `test_setRootWithContext_e2e`, it costs **+51 gas** (13,465,019 -> 13,465,070); `createWithContext` costs roughly +29. Runtime bytecode drops **214 bytes** (10,055 -> 9,841). At `optimizer_runs = 20000` the optimizer favours runtime speed over size, so a helper called from two sites is emitted once and jumped to rather than inlined: the duplication comes back as size, and the jump plus stack and memory handling is the gas. Kept for the deduplication rather than the gas. Both are setup calls rather than settlement-path calls, so ~50 gas is immaterial, and the contract has ~14.7KB of headroom so the size saving is not load-bearing either. ### Single hash on the create path `createWithContext` hashed `params` twice: once for the cabinet key, and again inside `_create` for the `singleOrders` slot. `hash` takes `memory`, so each call copies calldata to memory, abi-encodes it again (including the dynamic `staticInput`), then keccaks. This predates the `_setCabinet` extraction. `_create` now takes the hash, and `_validateAndHash` produces it: ```solidity function _validateAndHash(IConditionalOrder.ConditionalOrderParams calldata params) internal pure returns (bytes32) { require(address(params.handler) != address(0), InvalidHandler()); return hash(params); } ``` Validating before hashing matters. Threading the hash in naively left `_create`'s null-handler check running *after* the hash, so a rejected call paid for a hash it discarded: `test_create_RevertOnInvalidHandler` regressed **+1210 gas**. Producing the hash only via `_validateAndHash` means the check cannot be skipped and the revert path never hashes. Measured against the previous revision, pinned fuzz seed: | test | delta | | --- | --- | | `test_createWithContextAndRemove_FuzzSetAndEmit` | **-964** | | `test_create_RevertOnInvalidHandler` | -31 | | `test_setRootWithContext_e2e` | -26 | | `test_setRoot_e2e` | -26 | | `test_createAndRemove_e2e` | -12 | Net against #24 as originally written, before either change: **-935 gas** on `createWithContext` and **-190 bytes** of runtime bytecode. ### Typed muxer payload `_buildSignature` built the `SignatureVerifierMuxer` payload with `abi.encodeWithSignature` and a hand-written signature string, which the compiler does not check against the arguments passed alongside it. It now uses `abi.encodeCall`. `safeSignature` is not a declared function anywhere: the muxer compares the first four bytes against a private `SAFE_SIGNATURE_MAGIC_VALUE = 0x5fd7e97d` constant and abi-decodes the remainder. `encodeCall` needs something to point at, so this adds a declaration for exactly that wire format: ```solidity interface ISafeSignaturePayload { function safeSignature(bytes32 domainSeparator, bytes32 typeHash, bytes calldata encodeData, bytes calldata payload) external; } ``` The arguments are now type-checked and the selector is derived by the compiler rather than copied as a string. That moves the failure mode rather than removing it: a drifted declaration would silently change the selector, the muxer would stop routing, and signature verification would fail somewhere unrelated. `test_safeSignaturePayload_SelectorMatchesMuxerMagicValue` pins the selector to `0x5fd7e97d` so that surfaces as a named failure. The constant is `private` upstream and so cannot be imported and compared directly. Encoding is unchanged, verified byte-for-byte against the previous form across 256 fuzzed inputs before the old call was removed. Part of #1.
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.
The polling half of the dual-path architecture:
generateOrdermoves intoIConditionalOrderas the shared source of truthGeneratorResult/GeneratorResultCode:poll()never reverts for order conditions, returning a structured verdict; fill state is deliberately absent from the generator surface (composed by the registry)getNextPollTimestamp/describeOrderhooks with base defaultsOrderNotValidis terminal;Panic,Error(string)and unknown reverts map toTRY_NEXT_BLOCKcarrying the caught selector, so transient faults reschedule instead of permanently killing an order (closes polling: unknown reverts and Panic decode to permanent INVALID #3)tryGenerateOrderprobe returns(success, order, revertData)as ordinary return data — fullPanicsub-codes and inner-error arguments retrievable without RPC revert handlingBREAKING: the
IConditionalOrderGeneratorERC-165 interface id changes; existing deployed handlers will not satisfy the new id.Issues
Closes #3.
Advances #16 (lands the generator half -
GeneratorResultCode/GeneratorResult; the orthogonal fill overlay that completes the split lands in #24).Interface placement
generateOrderstays onIConditionalOrderGeneratorrather than moving up toIConditionalOrder.An earlier revision of this PR moved it into the base on the grounds that it is "the single source of truth for order derivation, shared by
verifyandpoll". That reasoning does not survive scrutiny:verify()callinggenerateOrder()is howBaseConditionalOrderimplements verify, not something the interface requires.generateOrderispublicon the base, so verify reaches it as an ordinary member call, and nothing casts a handler toIConditionalOrderto call it -- the registry gates ontype(IConditionalOrderGenerator).interfaceIdbefore polling. The move was purely taxonomic and would have cost the distinction the split exists to express:IConditionalOrder-- verifies a discrete order. A handler whose order is fixed rather than derived implements this alone and is not pollable.IConditionalOrderGenerator-- derives discrete orders on-chain, possibly a series of them (TWAP), and is therefore pollable by a monitoring service.Promoting
generateOrderto the base would have made derivation mandatory for every handler and left the generator interface named after a capability it no longer carried.The divergence risk that motivated the move is real and is addressed directly instead: the
generateOrderNatSpec now states that a generator'sverifyMUST accept exactly the orders it produces, since the two are separate call paths and a mismatch lets a watch-tower propose an order settlement rejects. Non-generators carry no such obligation.test/ComposableCow.base.t.sol'sMirrorConditionalOrderis the concrete non-generator case and keeps the plainIConditionalOrdershape covered.Error placement
Only
OrderNotValidstays onIConditionalOrder:verifyraises it directly (BaseConditionalOrder.solon hash mismatch), so it is not generator-specific.The polling errors (
PollTryNextBlock,PollTryAtBlock,PollTryAtTimestamp) move down to the generator. They are raised only fromgenerateOrderimplementations and decoded only by_decodeErrorToPollResult; a plainIConditionalOrderhandler is never polled, so a reschedule hint from itsverifywould have no consumer.This does not change any selector: Solidity derives an error selector from its signature, not from the interface that declares it. The ABI is untouched and only the qualified Solidity reference changes.
Part of #1.