Skip to content

feat!: add structured poll interface for watch-towers - #23

Merged
mfw78 merged 1 commit into
developfrom
feat/poll-interface
Jul 30, 2026
Merged

feat!: add structured poll interface for watch-towers#23
mfw78 merged 1 commit into
developfrom
feat/poll-interface

Conversation

@mfw78

@mfw78 mfw78 commented Jul 10, 2026

Copy link
Copy Markdown

The polling half of the dual-path architecture:

  • generateOrder moves into IConditionalOrder as the shared source of truth
  • GeneratorResult/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 / describeOrder hooks with base defaults
  • revert decoding: only OrderNotValid is terminal; Panic, Error(string) and unknown reverts map to TRY_NEXT_BLOCK carrying the caught selector, so transient faults reschedule instead of permanently killing an order (closes polling: unknown reverts and Panic decode to permanent INVALID #3)
  • tryGenerateOrder probe returns (success, order, revertData) as ordinary return data — full Panic sub-codes and inner-error arguments retrievable without RPC revert handling

BREAKING: the IConditionalOrderGenerator ERC-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

generateOrder stays on IConditionalOrderGenerator rather than moving up to IConditionalOrder.

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 verify and poll". That reasoning does not survive scrutiny: verify() calling generateOrder() is how BaseConditionalOrder implements verify, not something the interface requires. generateOrder is public on the base, so verify reaches it as an ordinary member call, and nothing casts a handler to IConditionalOrder to call it -- the registry gates on type(IConditionalOrderGenerator).interfaceId before 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 generateOrder to 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 generateOrder NatSpec now states that a generator's verify MUST 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's MirrorConditionalOrder is the concrete non-generator case and keeps the plain IConditionalOrder shape covered.

Error placement

Only OrderNotValid stays on IConditionalOrder: verify raises it directly (BaseConditionalOrder.sol on hash mismatch), so it is not generator-specific.

The polling errors (PollTryNextBlock, PollTryAtBlock, PollTryAtTimestamp) move down to the generator. They are raised only from generateOrder implementations and decoded only by _decodeErrorToPollResult; a plain IConditionalOrder handler is never polled, so a reschedule hint from its verify would 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.

@mfw78
mfw78 force-pushed the feat/typed-error-reasons branch from 06f7128 to 9635724 Compare July 30, 2026 03:50
@mfw78
mfw78 force-pushed the feat/poll-interface branch 2 times, most recently from 4865c88 to 65e5073 Compare July 30, 2026 04:01
@mfw78
mfw78 force-pushed the feat/typed-error-reasons branch 2 times, most recently from 23a005d to d307d23 Compare July 30, 2026 06:59
@mfw78
mfw78 force-pushed the feat/poll-interface branch from 65e5073 to 5e837ba Compare July 30, 2026 06:59
@mfw78
mfw78 force-pushed the feat/typed-error-reasons branch from d307d23 to 54400e6 Compare July 30, 2026 07:02
@mfw78
mfw78 force-pushed the feat/poll-interface branch from 5e837ba to 02f9536 Compare July 30, 2026 07:02
@mfw78
mfw78 force-pushed the feat/typed-error-reasons branch from 54400e6 to 87c7ffe Compare July 30, 2026 07:07
@mfw78
mfw78 force-pushed the feat/poll-interface branch from 02f9536 to af470ad Compare July 30, 2026 07:07
@mfw78
mfw78 force-pushed the feat/typed-error-reasons branch from 87c7ffe to d912654 Compare July 30, 2026 07:09
@mfw78
mfw78 force-pushed the feat/poll-interface branch 2 times, most recently from 2c8760b to da512da Compare July 30, 2026 07:14
@mfw78
mfw78 force-pushed the feat/typed-error-reasons branch 2 times, most recently from 2b43dc5 to a439033 Compare July 30, 2026 07:18
@mfw78
mfw78 force-pushed the feat/poll-interface branch from da512da to a81e4a1 Compare July 30, 2026 07:18
@mfw78
mfw78 force-pushed the feat/typed-error-reasons branch from a439033 to 9cd3b1b Compare July 30, 2026 07:22
@mfw78
mfw78 force-pushed the feat/poll-interface branch from a81e4a1 to 3997bec Compare July 30, 2026 07:22
@mfw78
mfw78 force-pushed the feat/typed-error-reasons branch from 9cd3b1b to 0d8d9a3 Compare July 30, 2026 07:26
@mfw78
mfw78 force-pushed the feat/poll-interface branch from 3997bec to 12f5b7d Compare July 30, 2026 07:26
Base automatically changed from feat/typed-error-reasons to develop July 30, 2026 07:32
@mfw78
mfw78 force-pushed the feat/poll-interface branch from 12f5b7d to fafabe0 Compare July 30, 2026 07:33
@mfw78
mfw78 force-pushed the feat/poll-interface branch 2 times, most recently from db7773d to 89871fe Compare July 30, 2026 08:42
Comment thread test/ComposableCow.gat.t.sol Outdated
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
mfw78 force-pushed the feat/poll-interface branch from 89871fe to f7a0b44 Compare July 30, 2026 09:51
@mfw78
mfw78 merged commit bcc2498 into develop Jul 30, 2026
@mfw78
mfw78 deleted the feat/poll-interface branch July 30, 2026 09:53
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.
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.

polling: unknown reverts and Panic decode to permanent INVALID

1 participant