Skip to content

feat: dual-path architecture with order manifest support [tracking] - #1

Draft
mfw78 wants to merge 25 commits into
developfrom
feat/dual-path-architecture
Draft

feat: dual-path architecture with order manifest support [tracking]#1
mfw78 wants to merge 25 commits into
developfrom
feat/dual-path-architecture

Conversation

@mfw78

@mfw78 mfw78 commented Feb 13, 2026

Copy link
Copy Markdown

Summary

  • Introduces dual-path architecture separating gas-sensitive settlement from rich polling
  • Typed error surface: bytes4 reasonCode selectors end to end, no stringly errors
  • Adds IOrderManifest interface for order enumeration across all order types
  • Emits ConditionalOrderRemoved event on order deauthorization
  • Updates documentation for ERC-1271 wallet support and upstream breaking changes

This PR stays open as the delivery tracker and review record. The work lands as a stack of slices into develop; review threads here remain the canonical design discussion. This branch is frozen and will not be merged.

Slices

Stacked in dependency order; each merges into develop via squash after review.

Discovery train (spec-first; implements #15):

Design gates

Breaking Changes

See docs/architecture.md (#29) for the full migration guide from upstream.

Test Plan

  • 155 tests green at the stack tip, including the TWAP simulation fuzz
  • Each slice builds and tests green standalone
  • Gas: −7.8% vs string-reason baseline

mfw78 added 25 commits February 6, 2026 14:47
Enable require(condition, CustomError()) syntax for cleaner error handling.
BREAKING CHANGE: Replace getTradeableOrder with generateOrder/poll split

- Rename getTradeableOrder to generateOrder as single source of truth
- Add generateOrder to IConditionalOrder base interface
- Rename PollTryAtEpoch to PollTryAtTimestamp for clarity
- Remove PollNever error (use OrderNotValid instead)
- Add IConditionalOrderGenerator with poll(), getNextPollTimestamp(), describeOrder()
- Add PollResultCode enum: SUCCESS, PARTIALLY_FILLED, FILLED, WAIT_TIMESTAMP, WAIT_BLOCK, TRY_NEXT_BLOCK, INVALID
- Add PollResult struct with order, scheduling hints, and fill amount
- Clean up documentation comments
- Add verify() that calls generateOrder() and validates hash
- Add poll() that wraps generateOrder() with try/catch for structured results
- Add error decoding for OrderNotValid, PollTryNextBlock, PollTryAtTimestamp, PollTryAtBlock
- Add default getNextPollTimestamp() returning POLL_AT_VALIDTO (0)
- Add default describeOrder() returning generic message
- Define POLL_AT_VALIDTO and POLL_NEVER constants for scheduling hints
- Use string constant INVALID_HASH for error messages
Add filledAmount(bytes calldata orderUid) for querying order fill status.
- Extend BaseConditionalOrder instead of IConditionalOrderGenerator
- Rename getTradeableOrder to generateOrder
- Use require(condition, CustomError()) syntax
- Use string constants for error messages
- Override getNextPollTimestamp() for multi-part scheduling
- Override describeOrder() for human-readable status
- Clean up documentation comments
- Extend BaseConditionalOrder instead of IConditionalOrderGenerator
- Rename getTradeableOrder to generateOrder
- Use require(condition, CustomError()) syntax
- Use string constants for error messages
- Override getNextPollTimestamp() returning POLL_NEVER for single-shot
- Clean up documentation comments
- Extend BaseConditionalOrder instead of IConditionalOrderGenerator
- Rename getTradeableOrder to generateOrder
- Use require(condition, CustomError()) syntax
- Use string constants for error messages
- Override getNextPollTimestamp() returning POLL_NEVER for single-shot
- Clean up documentation comments
- Extend BaseConditionalOrder instead of IConditionalOrderGenerator
- Rename getTradeableOrder to generateOrder
- Use require(condition, CustomError()) syntax
- Use string constants for error messages
- Clean up documentation comments
- Extend BaseConditionalOrder instead of IConditionalOrderGenerator
- Rename getTradeableOrder to generateOrder
- Use require(condition, CustomError()) syntax
- Use string constants for error messages
- Clean up documentation comments
- Refactor isValidSafeSignature to call handler.verify() directly
- Refactor getTradeableOrderWithSignature to use handler.poll()
- Add fill detection via GPv2Settlement.filledAmount()
- Return PARTIALLY_FILLED or FILLED based on order kind (sell/buy)
- Add checkOrder() for quick tradeable status check
- Store settlement contract reference for fill queries
- Use require(condition, CustomError()) syntax
- Clean up documentation comments
Remove unused comments.
- Clean up unused imports and comments
- Update for new interface signatures
- Add test handlers for each error type (OrderNotValid, PollTryNextBlock, etc.)
- Update TestConditionalOrder to extend BaseConditionalOrder
- Rename getTradeableOrder to generateOrder in mocks
- Update for PollResult return type from getTradeableOrderWithSignature
- Test error decoding for OrderNotValid, PollTryNextBlock, PollTryAtTimestamp, PollTryAtBlock
- Test verify() hash validation
- Test that getTradeableOrderWithSignature uses poll() internally
- Fuzz tests for error message propagation
- Rename test_getTradeableOrder_* to test_generateOrder_*
- Update for PollResult return type from getTradeableOrderWithSignature
- Update expected error types (PollTryAtTimestamp vs OrderNotValid)
- Update fuzz bounds for simulate tests
- Clean up documentation comments
- Document settlement path vs polling path separation
- Add PollResultCode semantics table with PARTIALLY_FILLED and FILLED
- Document fill detection via GPv2Settlement
- Add polling path diagram with fill checking flow
- Document nextPollTimestamp semantics
- Add implementation checklist for new order types
Restructure validToBucket() to calculate bucket number first, then multiply.
Same behavior, clearer intent, no lint warning.
Add out/ to .gitignore to exclude Foundry build artifacts.
Introduces a new interface enabling enumeration of all discrete orders
that a conditional order will produce. Useful for analytics, UI previews,
and order lifecycle tracking.

- Cardinality enum: FINITE, BOUNDED, UNBOUNDED
- ManifestInfo struct for high-level order count information
- ManifestEntry struct with order details and validity window
- Paginated getManifestPage() for efficient enumeration
Provides a default single-shot manifest implementation:
- getManifestInfo() returns FINITE with totalOrders: 1
- getManifestPage() wraps generateOrder() for a single entry
- Adds IOrderManifest to supportsInterface()

Order types can override these for multi-part or unbounded orders.
TWAP:
- FINITE cardinality with n parts
- Full pagination support for all TWAP parts
- Consolidated helpers for order construction

StopLoss:
- Custom manifest showing order structure even when strike not reached
- Helper to check strike condition without reverting

GoodAfterTime:
- Sets validFrom to startTime for proper scheduling display

TradeAboveThreshold:
- Shows order structure even when below threshold

PerpetualStableSwap:
- UNBOUNDED cardinality with hasMore always true
- Emit ConditionalOrderRemoved when orders are deauthorized via remove()
- Update contract natspec to reflect ERC-1271 wallet support
27 tests covering:
- BaseConditionalOrder default manifest behaviour
- TWAP manifest with pagination and timing verification
- StopLoss manifest with oracle condition checking
- GoodAfterTime manifest with validFrom handling
- TradeAboveThreshold manifest with threshold states
- PerpetualStableSwap unbounded manifest
- ConditionalOrderRemoved event emission
Updates architecture documentation to reflect:
- ERC-1271 wallet support (not just Safe wallets)
- ERC1271Forwarder integration pattern
- IOrderManifest interface and implementation details
- ConditionalOrderRemoved event

Adds comprehensive breaking changes section documenting all interface
and contract changes from cowprotocol/composable-cow upstream:
- IConditionalOrder: PollTryAtEpoch renamed, PollNever removed
- IConditionalOrderGenerator: getTradeableOrder() replaced by poll()
- ComposableCoW: return type and fill status changes
- BaseConditionalOrder: manifest support added

@anxolin anxolin 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.

Nice to see you are taking further the learnings from building the indexers to give the composable cow handlers more semantics.

Do you think we can come up with a simpler way to automatically decode the static input? (so UIs and the indexer have an easier time explaining the order)

We have this thing that uses the SDK, but requires you to register the handler.
https://sdk-tools.cow.fi

Comment thread docs/architecture.md
}
```

### PollResultCode Semantics

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I like the general idea, but It feels we mix some concepts inside PollResultCode

I assume the code is about some human readable message that is nice for debugging.
But for example, what if its PARTIALLY_FILLED our order from last poll but we want to also return WAIT_TIMESTAMP for example?

I understand poll result about the generation of a discrete order from the programmatic order. So I don't get too much SUCCESS , PARTIALLY_FILLED, FILLED, and instead should just be POST_NEW_ORDER or similar.

This way, you either get INVALID or it signals to wait for block/timestamp or it tells you to post an order.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe I get FILLED too, as a kind of equivalent to INVALID('already filled'), but I don't get PARTIALLY_FILLED

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

PollResultCode does contain additional information, and nominally there is a discrete order that is authorised if the PollResult struct contains a PullResultCode of SUCCESS, PARTIALLY_FILLED, or FILLED.

In these cases, the nextPollTimestamp would then contain the timestamp as to when to next poll the contract for a discrete order. Semantically, type(uint256).max aligns with similar semantics in that setting type(uint32).max is the settlement contract's preferred method of invalidation (nothing can happen past this time as its the end type bounds).

Given this, there may be some duplication between waitUntil and nextPollTimestamp and we could most probably compress these down into the same part of the struct, though having waitUntil sounds nicer, though no strong opinions there.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry, I still don't get it.

Watch tower will poll, potentially every block.
Each poll will return a PollResultCode. Its possible, that is not time to trade (WAIT_TIMESTAMP/WAIT_BLOCK/TRY_NEXT_BLOCK/INVALID) or its time to trade.

If its time to trade, you are saying you want to return some order data from the handler, but also make that handler responsible of deriving the orderUid and checking in the settlement the traded amounts, so you can decided if its

  • SUCCESS: Untouched order, potentially needs to be created
  • PARTIALLY_FILLED: We can assume it was created. Potentially watch tower doesn't need to do anything
  • FILLED: We can assume it was created. Watch tower needs to stop caring about the order

Is this your idea behind the enum?

@mfw78 mfw78 Feb 17, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It is to make allowance and present to a possible UI that if the ConditionalOrderParams are indexed, a dapp could subsequently do the poll itself and determine the state of the order, potentially reducing load on indexers and shifting load to RPC endpoints.

And yes, the breakdown in all cases is similar to what you mention with some nuances:

  • SUCCESS: Order has been yielded and should be submitted by the off-chain indexer to the API.
  • PARTIALLY_FILLED: Discrete is already in the order book, as evidenced by being partially filled. No need for the off-chain indexer to do anything for this specific discrete order. Off-chain indexer is to respect nextPollTimestamp which has semantics to indicate if it should do anything, or cease monitoring the ConditionalOrder.
  • FILLED: Same semantics as PARTIALLY_FILLED, just with the difference that the order has been completed filled.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Settled in #16 (verdict/fill split); lands with the poll-interface and registry slices.

Comment thread docs/architecture.md
### Single-Shot Orders (StopLoss, GoodAfterTime)

```solidity
function generateOrder(...) public view returns (GPv2Order.Data memory) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Probably not worth it, but do you think there's any value on letting a handler generate more than one order per block?
generateOrder could take a index param , and return the order and a boolean indicating if there's more orders to be generated.

This way you call it once, count=0 and if there's more orders (moreOrders=true), you call again with count=1 and so forth

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

A simple salt can be used so that multiple orders can be done in the same block. This is supported already at the ComposableCoW level, above the handler.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not sure I get this, these are the params

 struct ConditionalOrderParams {
        IConditionalOrder handler;
        bytes32 salt;
        bytes staticInput;
    }

When we create a single order, we create composableCow.create(..) and give this info. So SALT is given.

What I'm asking here, is, imagine we want to make a contract that in each polling creates 2 orders.
For the same handler/salt/staticinput poll, it will return

  1. Call 1: [order1, hasMoreOrders=true]
  2. Call 2: [order2, hasMoreOrders=false]

This way conditional order has a way to yield more than one order

@mfw78 mfw78 Feb 17, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fair in that the salt is a given. What is a concrete example of a conditional order type that would yield multiple discrete orders such as this?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not saying we should do it, but assuming each block generates only one order is arbitrary

  • Multi-currency-DCA: Buys you each month automatically a bunch of tokens
  • Simplified AMM? Creates multiple buy and sell orders
  • Go long and take profit: Sell something , and creates a limit order to take profit once the bought token appreciates

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good examples — opened #17. Short version: offchainInput-keyed orders + manifest enumeration already cover these; proposing we document the pattern rather than change the signature.

Comment thread docs/architecture.md
|-------|---------|
| `0` | Use `order.validTo + 1` as default |
| `> 0` | Poll at this specific timestamp |
| `type(uint256).max` | Final order, stop polling after fill |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

not strong preference, but wouldn't be slightly more natural to make

  • 0 --> stop polling
  • 1 --> validTo+1

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I do prefer the semantics of having "stop" being indicated by the "maximum" timestamp (nothing can happen beyond that point of time if that's the time's bounds), ie. with the purpose / point as outlined in the previous 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.

My thinking was, watch-tower will probably stop watching if the time is in the past. So 0 was saying, don't poll any more. But im fine with yours

Comment thread docs/architecture.md

## Order Type Patterns

### Single-Shot Orders (StopLoss, GoodAfterTime)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can we make a simpler alternative way for these orders?

It has been once of the biggest criticism of composable cow.
It would be a scape hatch, for example

  • A method in composable cow contract createSingleShotOrder(order)
  • The method emits an event that is picked by composable cow
  • The owner of the order is a random contract which implements 1271
  • This contract doesn't need to do the delegation to composable cow at all
  • Composable cow post the order to the API (if ready), if not, tries on every block

What this allow is to make a contract that creates the order by calling this simple method with the order you want to place. It allows a trivial way to integrate.

I see that someone could spam with this (if we keep trying forever), however the indexer can have some policies to pause polling for some orders after a while. Even if we decide to drop it after 30min is already super useful for quick integrations.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ComposableCoW was never designed inherently to support single-shot, and these were somewhat "wedged" into the design in retrospect. It was designed to support primarily generational conditional orders that generate multiple discrete orders.

I don't think that we should be opening some escape hatch mechanism like this, and perhaps should drive more of a defining line that separates multi-discrete order generators and single-shot. The concern with single-shot has always been with trying to define a nullifier that ensures the single-shot property. For example, StopLoss will continue to generate discrete orders so long as the conditions are met, even if the position has already been sold, and so to handle this, it was often done that there would be some intermediate contract whose job was specifically just to hold the funds for that purpose (this is really a mess, and IMO having specific contracts spun up for specific orders such as this is more indicative of an architectural smell as opposed to a sound system).

I would think that there may be scope available for the single-shot conditional order mechanism to be refined / clarified with a combination of indexing work + generalised wrappers, so as to be able to then ensure that some requisite post-hook has run, adjusting state to nullify the single-shot conditional order, though I would determine generalised wrapper implementation for such to be out-of-scope on the current grant (but indexing / architectural design to be within scope).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not sure I got all this.

The point is, currently is super complex for a smart contract to create an order.

They need to use ComposableCoW, which has a lot of indirection built in and people struggle to understand.

So for the simpler cases, of a SC creating an order, and a watch tower picking it, would be nice to have a contract that receives the params, emits the event, and someone creates the order for them.

It doesn't need to be in Composable CoW it can be an independent contract, I just thought watch-tower could index and post order for both flows.

If this is out of scope, fine, I just bring it up as it has been a feedback, and if you are modifying the contract, which will require re-audit and has breaking changes, might be worth it to add this alternative simpler flow.

Comment thread docs/architecture.md
return buildPartOrder(currentPart);
}

function getNextPollTimestamp(...) external view returns (uint256) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

i love moving the polling logic to the handler. Not sure people will put the effort into optimising for this in the contract, but it really makes the indexers simpler.

Comment thread docs/architecture.md
| Cardinality | Description | Example |
|-------------|-------------|---------|
| `FINITE` | Known fixed number of orders | TWAP with n parts |
| `BOUNDED` | Upper bound known; actual count is dynamic | Future order types |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I didn't get exactly what you mean? can you give one example

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Cardinality defines the number in a set. Let's consider some hypothetical "range trading" conditional order that trades for side-ways pairs. It may attempt to do so every n period, but only if certain conditions are established (price at certain range), for some time t. In which case, the upper bound (cardinality) for this conditional order may be t / n, but the price case may not always be met, so it may be less than t / n.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe what confuses me is that BOUNDED is by defiition FINITE.

@mfw78

mfw78 commented Feb 16, 2026

Copy link
Copy Markdown
Author

Do you think we can come up with a simpler way to automatically decode the static input? (so UIs and the indexer have an easier time explaining the order)

We have this thing that uses the SDK, but requires you to register the handler. https://sdk-tools.cow.fi

So, I suppose we could add a 'Metadata' abstract contract interface, but this would present only a string-based representation of the conditional order. Nonetheless, this is the nature of ABI and IMO SDK interfaces coupled with dynamic discovery method to retrieve some application specific module that is capable of decoding (this would then allow, for example, a handler to register via ENS the location of their decoder) is the way to go. Such design is non-trivial, but could be included from an architectural perspective so that some discovery mechanism was able to be implemented at a later date.

@anxolin

anxolin commented Feb 16, 2026

Copy link
Copy Markdown

Do you think we can come up with a simpler way to automatically decode the static input? (so UIs and the indexer have an easier time explaining the order)
We have this thing that uses the SDK, but requires you to register the handler. https://sdk-tools.cow.fi

So, I suppose we could add a 'Metadata' abstract contract interface, but this would present only a string-based representation of the conditional order. Nonetheless, this is the nature of ABI and IMO SDK interfaces coupled with dynamic discovery method to retrieve some application specific module that is capable of decoding (this would then allow, for example, a handler to register via ENS the location of their decoder) is the way to go. Such design is non-trivial, but could be included from an architectural perspective so that some discovery mechanism was able to be implemented at a later date.

What about we force handlers to implememt:

function staticInputAbi() external pure returns (string memory);

This way TWAP can return the string:

TWAPOrder(address sellToken,address buyToken,address receiver,uint256 partSellAmount, ...)

Any web3 lib could use this and the static input and make sense of it

@mfw78

mfw78 commented Feb 17, 2026

Copy link
Copy Markdown
Author
function staticInputAbi() external pure returns (string memory);

This would have to take a bytes parameter as well yes? ie.:

function staticInputAbi(bytes calldata) external pure returns (string memory);

The interface would make no enforcement at a smart contract level as to what the string itself would return, other than that the developer should "convey a human readable understanding of the static input".

Comment thread docs/architecture.md

## Order Type Patterns

### Single-Shot Orders (StopLoss, GoodAfterTime)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can you confirm if the single shot order enforcement is more flexible or if the same pattern will be used -- handler must return a constant order?

In my opinion single-shot orders handlers should be more easier / more flexible to create. Stop loss is a good example that you must hard-code the entire order to make it work (so you can't calculate the amounts based on current price + slippage for example). The current interface limits the flexibility of single shot orders a lot in my understanding.

I don't have full context on how hard would be possible to create an easier way to do it with this new interface. Since the pooling and verifier interfaces changed, my understand is that this could be solved by invalidating the composable cow after its verification. This would only work for fill or kill orders but it still looks like a good feature from my point of view.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I don't have full context on how hard would be possible to create an easier way to do it with this new interface. Since the pooling and verifier interfaces changed, my understand is that this could be solved by invalidating the composable cow after its verification. This would only work for fill or kill orders but it still looks like a good feature from my point of view.

As isValidSignature is view only, it is not possible to modify any state after its verification. The only method to modify any state is by reference to a post-hook, which then runs into authorisation / griefing attack problems (anyone can execute the post-hook that does the voiding action from the HooksTrampoline).

Essentially I think that with the difficulty associated with this, it's more than likely that this kind of design is an anti-pattern when it comes to CoW Protocol in its current incarnation. A much simpler solution, albeit one that requires the use of gas, is to have a conditional order that is triggered via account abstraction or similar, which does setPreSignature and then subsequently voids the conditional order atomically.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As isValidSignature is view only, it is not possible to modify any state after its verification. The only method to modify any state is by reference to a post-hook, which then runs into authorisation / griefing attack problems (anyone can execute the post-hook that does the voiding action from the HooksTrampoline).

This could be convined with Generalise Wrappers to make it work.

We add a wrapper hint to a contract that, marks the order as traded after the settlement. The composable cow handler, only allows to trade if we haven't traded a similar order.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Would this make sense?

image

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If there's a second sister order generated by the handler:

  • If the settlement calls the wrapper (as instructed) --> Reverts
  • If it skips the wrapper, the order will be unsigned
image

@mfw78 mfw78 Feb 26, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Given the advent of the wrappers, does this create any technical debt on the indexing methods that have been proposed in this PR?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't think so. The indexer would pick order1 post it to the API, while in-flight, will pick a second order order2, because the 1st one is not executed, it will post it to the API too.

The API accept both, they both are accepted by the contract.

The first one executing, will automatically invalidate the other. The order will remain in the orderbook until expiration.

mfw78 added a commit that referenced this pull request Jul 30, 2026
Completes the `out/` untracking: ignore the directory and remove
generated artifacts from version control. Artifacts are reproducible
from source via `forge build`.

Part of #1.
mfw78 added a commit that referenced this pull request Jul 30, 2026
- Pin solc 0.8.30 and `evm_version = "cancun"` (PUSH0 + MCOPY codegen;
supported on all deployment chains)
- Add `foundry.lock` for reproducible builds

Part of #1.
mfw78 added a commit that referenced this pull request Jul 30, 2026
The tree mixed `///` and `/** */` NatSpec, and in sixteen files both
forms appeared side by side. Standardise on the block form:

- 57 declaration-level `///` runs converted to `/** */` across `src`,
`test` and `script`
- block form was already the majority (85 of 142 declaration-level doc
comments) and is what the newer interface and handler work in this stack
already uses
- records the rule in `docs/style.md` so it does not drift again

Deliberately out of scope: comments **inside** a function body. solc
ignores NatSpec tags there, so those are implementation comments rather
than documentation, and expanding a one-line note into a three-line
block would make the code harder to read. There are 16 of them and they
are handled separately at the tip of the stack, so that PR can be
dropped independently.

One case needed care: a `solhint-disable-next-line` directive sat in the
middle of a `///` run in `GPv2TradeEncoder.sol`. Converting naively
split the header into two blocks, which silently orphans the leading
one, because only the block immediately preceding a declaration is
NatSpec. The halves are rejoined and the directive dropped (solhint is
not configured or run anywhere in this repo).

## How to test

The change is comment-only, which is machine-checkable:

```sh
git diff chore/solc-0830-toolchain..style/natspec-block-form -U0 \
  | grep '^[+-]' | grep -v '^\(+++\|---\)' \
  | grep -v '^[+-][[:space:]]*\(///\|/\*\*\|\*/\|\*\)'
```

That prints nothing: no non-comment line is touched. `forge build` and
`forge test` are green (74 passing).

Part of #1.
mfw78 added a commit that referenced this pull request Jul 30, 2026
Replaces the `mfw78@rndlabs.xyz` address with `mfw78@nxm.rs` in the
NatSpec `@author` tags across 24 files in `src`, `test` and `script`.

Left alone deliberately:

- the `github.com/rndlabs/safe-contracts` reference in the README, which
is a live repository URL rather than an address
- `broadcast/`, which holds historical deployment and verification
records of what was actually deployed, embedded source included

## How to test

```sh
rg 'rndlabs\.xyz' -g '!lib/**' -g '!out/**' -g '!broadcast/**'
```

That returns nothing. `forge build` and `forge test` are green (74
passing).

Part of #1.
mfw78 added a commit that referenced this pull request Jul 30, 2026
Solidity's CapWords convention does not carry branding capitalisation
into an identifier, and the registry was the last place the fork still
spelled it `ComposableCoW`. The surrounding code had already moved on:
the poller work uses `ComposableCowPoller`, and `TWAP` holds a
`composableCow` while `ERC1271Forwarder` held a `composableCoW`, which
is the same inconsistency in miniature.

Changes:

- `ComposableCoW` -> `ComposableCow`, `composableCoW` ->
`composableCow`, `ComposableCoWLib` -> `ComposableCowLib`, and the
`*Test` contract names
- 14 file renames: `src/ComposableCow.sol`,
`script/deploy_ComposableCow.s.sol`, `test/ComposableCow.*.t.sol`,
`test/libraries/ComposableCowLib.t.sol`
- the `networks.json` top-level key
- prose in `README.md`, `docs/architecture.md`, `docs/discovery.md`
- `.gas-snapshot` keys

Left alone deliberately:

- the `CoW Protocol` spelling in prose, and vendored upstream
identifiers such as `GPv2Settlement` and `CoWSettlement`
- the audit PDFs, which are published artefacts and keep their
filenames. Only the link text in the README is updated, so
`gnosis-ComposableCoWMayJul2023.pdf` still resolves
- `broadcast/`, which records what was actually deployed under the old
name

`dev/verify-contracts.sh` derives its `networks.json` lookup key from
the contract filename, so renaming the key alongside the file keeps
verification working rather than needing a mapping shim.

**BREAKING**: the contract, its build artefact and the `networks.json`
key are all renamed. The manifest describes a version that is not yet
deployed and will be superseded by `deployments/networks.json`, so no
live consumer is affected.

## How to test

`.gas-snapshot` keys were rewritten textually rather than regenerated,
on the grounds that a rename cannot change gas. That is verifiable:

```sh
forge snapshot && git diff --exit-code .gas-snapshot
```

Clean: the textual rewrite and a regeneration agree exactly. `forge
build` and `forge test` are green (74 passing).

Part of #1.
mfw78 added a commit that referenced this pull request Jul 30, 2026
Nothing in this tree is deployed, and the artefacts still said
otherwise.

- **removes the legacy root `networks.json`** (274 lines, 8 contracts x
8 chains). Those are *upstream's* addresses, for contracts this fork no
longer matches after the generateOrder rename, typed errors, poll
interface and proof-payload changes. Leaving the file invited them to be
read as this fork's own deployments. When this fork does deploy, the
canonical record is `deployments/networks.json`, landing in #41.
- **`dev/verify-contracts.sh`** was the file's only consumer. It now
fails with a clear message instead of feeding `null` addresses into
`forge verify-contract`.
- **drops the README address table** and states plainly that there are
no deployments.
- **reframes the audit list.** It previously read "The above deployed
contracts have been audited by" directly under the address table. Those
audits cover upstream before this work, so leaving it unqualified
alongside an unaudited warning would have been contradictory.
- **adds a warning under the README title** that the fork is unaudited,
not deployed, and substantially changed.

**BREAKING**: `networks.json` is removed. Any consumer resolving
addresses from it was resolving upstream deployments, not this fork's.

## How to test

```sh
rg '0x[0-9a-fA-F]{40}' README.md   # no addresses left
test -f networks.json               # gone
./dev/verify-contracts.sh 1         # fails with the no-deployments message
```

Part of #1.
mfw78 added a commit that referenced this pull request Jul 30, 2026
…lsLib (#20)

Reorder the computation in `ConditionalOrdersUtilsLib` to multiply
before dividing, resolving the divide-before-multiply lint and its
precision loss.

Part of #1.
mfw78 added a commit that referenced this pull request Jul 30, 2026
Rename the order-generation entry point to reflect its role as the
single source of truth shared by verification and polling:

- `getTradeableOrder` -> `generateOrder` (interface, base, all order
types, registry call site)
- `PollTryAtEpoch` -> `PollTryAtTimestamp`
- drop the unused `PollNever` error

**BREAKING**: handler ABI renames `generateOrder`; the
`IConditionalOrderGenerator` ERC-165 interface id changes.

Part of #1.
mfw78 added a commit that referenced this pull request Jul 30, 2026
Reason payloads become `bytes4` selectors of handler-declared custom
errors instead of free strings:

- `OrderNotValid`, `PollTryNextBlock`, `PollTryAtTimestamp`,
`PollTryAtBlock` carry `bytes4 reasonCode`
- every reason string constant becomes a declared error
(`StrikeNotReached()`, `BeforeTwapStart()`, ...) — part of the handler
ABI, so consumers resolve codes to names without a bespoke table
- revert data is fixed-size; no stringly errors remain on the error
surface

**BREAKING**: error signatures change.

Part of #1.
mfw78 added a commit that referenced this pull request Jul 30, 2026
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 #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 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.
mfw78 added a commit that referenced this pull request Jul 30, 2026
Override `getNextPollTimestamp` and `describeOrder` where the order type
knows better than the validTo default:

- TWAP schedules the next part (underflow-safe), and treats before-start
and span gaps as WAIT signals rather than terminal failures
- StopLoss and GoodAfterTime are single-shot: stop polling after fill

Part of #1.
mfw78 added a commit that referenced this pull request Jul 30, 2026
A fill-or-kill order with a zero amount settles without ever
incrementing `filledAmount`, so the native replay guard never trips and
the order is indefinitely replayable. Reject zero amounts at handler
validation (closes #12), enforce the TWAP parameter gate on every entry
path (advances #4), and bound the simulation fuzz so the suite runs
green within test memory limits.

## Issues

Closes #12.
Closes #4.

### TWAP degenerate-bundle guard

`generateOrder` reaches `TWAPOrder.validate` through `orderFor`, but
`getNextPollTimestamp` and `describeOrder` did not validate at all. Both
call `_currentPart`, which divides by `twap.t`:

- `t == 0` panicked with `0x12` (division by zero) instead of reverting
`OrderNotValid(InvalidFrequency)`
- `n == 0` returned `POLL_NEVER` silently rather than rejecting

Both now call `TWAPOrder.validate` before any part maths. The manifest
paths landing in #28 already validate through `try
this.validateTwapData`, and `generateOrder` is unaffected, so the
settlement path takes no extra validation.

`poll()` calls `getNextPollTimestamp` inside the try's *success* block,
where a revert would not be caught, but it is only reachable once
`generateOrder` has succeeded and therefore already validated the
bundle. Direct callers were the exposed path.

Three tests cover it. Each was confirmed to fail without the guard: the
`t == 0` cases with the raw `0x12` panic, and the `n == 0` case with
"next call did not revert as expected".

### Unrelated test fix

`test_generateOrder_e2e_fuzz_WithContext` carried a `vm.assume`
commented "guard against overflows" that overflowed itself:

```solidity
vm.assume(currentTime < _blockTimestamp + (FREQUENCY * NUM_PARTS));
```

`_blockTimestamp` is a `uint32`, so the sum wraps for timestamps near
the type maximum, and the test panicked `0x11` on such an input. Widened
to `uint256`. This predates the stack (it is in `develop`); it surfaced
here because the new tests shift the fuzz input sequence.

`test_simulate_fuzz` is no longer excluded from CI-equivalent runs: it
began passing at #24, which changed the registry poll path.

Part of #1.
mfw78 added a commit that referenced this pull request Jul 30, 2026
Sidecar interface (own ERC-165 id, feature-detected, never on the
settlement path) for enumerating the discrete orders a conditional order
will produce:

- `Cardinality` names the `totalOrders` semantics directly: EXACT /
CAPPED / UNBOUNDED (closes #13)
- explicit pagination contract: an empty page with `hasMore == true` is
unreachable, so a naive offset walker always terminates
- `getManifestPage` carries a `bytes4 reasonCode` so a not-yet-active
order is distinguishable from a permanently invalid one on an empty page
(advances #5)
- `BaseConditionalOrder` defaults to a single-shot manifest (EXACT, 1)

## Issues

Closes #13.
Advances #5 (the catch now surfaces the handler's `bytes4 reasonCode`,
but `_decodeErrorToGeneratorResult`'s `code` and `waitUntil` are both
discarded, so a consumer still cannot classify "not yet active" against
"permanently invalid" without per-handler knowledge of the error names,
and the preferred `validFrom` on the page is not carried).

Part of #1.
mfw78 added a commit that referenced this pull request Jul 30, 2026
- TWAP enumerates all n parts with per-part validity windows, validating
parameters before any part math so degenerate bundles yield an empty
manifest instead of underflowing
- PerpetualStableSwap exposes only the current discrete order and
terminates pagination on every branch (closes #7); empty pages carry the
decoded reason (e.g. `NotFunded`)

## Issues

Closes #7.
Advances #4 (gates `getManifestInfo` and `getManifestPage` on
`validateTwapData` before any part math, satisfying that issue's
manifest requirement; the poll-hint entry points remain unvalidated).

Part of #1.
mfw78 added a commit that referenced this pull request Jul 31, 2026
Architecture documentation for the dual-path design: settlement/polling
split, generator verdict and registry fill overlay, typed-error decoding
policy and `tryGenerateOrder` diagnostics, order manifest pagination
contract, the multi-order pattern via `offchainInput` keying (closes
#17), ERC-1271 integration, the handler purity invariant (advances #8),
and the full breaking-changes migration guide from upstream.

## Issues

Closes #17.
Advances #8 (design principle 6 in `docs/architecture.md` states the
invariant, but the NatSpec note on `IConditionalOrder.generateOrder` is
unchanged and there is no test or lint guard - the issue explicitly
prefers those over documentation alone).

Part of #1.
mfw78 added a commit that referenced this pull request Jul 31, 2026
Solidity >=0.8.27 supports `require(condition, CustomError())` in the
legacy pipeline; convert the remaining `if (!(condition)) revert` sites
so preconditions read as preconditions. Bytecode-equivalent.

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.

3 participants