Skip to content

settlement: cover the untested fill-detection overlay #2

Description

@mfw78

Problem

The fill-detection overlay that synthesizes FILLED and PARTIALLY_FILLED poll results has zero test coverage, so new consensus-adjacent logic that gates watchtower signature production ships unverified. A wrong orderUid construction or a mis-selected total amount would let a filled order re-present as tradeable, or a still-open order be reported as filled, both of which affect settlement liveness.

Current

ComposableCoW.getTradeableOrderWithSignature (src/ComposableCoW.sol L167-185) runs after poll() returns SUCCESS and, when _getFilledAmount returns a non-zero value, overwrites the result with PollResultCode.FILLED or PollResultCode.PARTIALLY_FILLED depending on filledAmount >= totalAmount. totalAmount is selected as order.sellAmount for KIND_SELL and order.buyAmount for KIND_BUY. _getFilledAmount (src/ComposableCoW.sol L275-278) hand-builds the GPv2 orderUid as abi.encodePacked(GPv2Order.hash(order, domainSeparator), owner, order.validTo) and reads settlement.filledAmount(orderUid). No test under test/ references FILLED or PARTIALLY_FILLED, and none exercises the _getFilledAmount > 0 branch.

Proposed

Add unit tests covering the overlay branch:

  • FILLED path where filledAmount >= totalAmount.
  • PARTIALLY_FILLED path for a partiallyFillable order where 0 < filledAmount < totalAmount.
  • orderUid construction checked against a known-good GPv2 orderUid for the same order and owner.
  • KIND_BUY versus KIND_SELL total-amount selection, asserting the correct field drives isFullyFilled.
    Confirm what units GPv2Settlement.filledAmount stores per order kind and assert the comparison uses matching units. Reason explicitly about fill-or-kill orders (partiallyFillable = false), where PARTIALLY_FILLED is effectively a dead branch, and document that expectation in the test.

Scope

Off-chain only: adds tests under test/. No contract source changes, no bytecode change, no storage-layout change, no interface change. If the unit-semantics check reveals a KIND_BUY mismatch, that becomes a separate on-chain issue requiring its own review.

Notes

Introduced by the dual-path architecture work in PR #1. The unit-semantics confirmation against GPv2Settlement is a prerequisite for signing off the KIND_BUY comparison; capture the finding in the test even if it holds.

Metadata

Metadata

Assignees

No one assigned

    Labels

    component/settlementCoW Protocol settlement integration, swap guards, ERC-1271 signature verificationcomponent/testsFoundry test suites, helpers, and fixturesp1-brokenActually broken. Fix this week.securityVulnerability, exploit surface, or hardening of on-chain funds or authorisation

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions