Skip to content

cow: an empty-data revert re-polls the commitment on every block forever #692

Description

@mfw78

Found by the anvil-fork harness in #691 on its first run against real contracts.

What happens

classify_revert in crates/composable-cow/src/fork.rs reads the revert payload:

let Some(data) = rpc.data.as_deref() else {
    return Verdict::TryNextBlock { reason: Selector::ZERO };
};

A revert that carries no data therefore classifies as TryNextBlock, silently and with no log.
The commitment stays armed on the block clock, so it is re-polled on every block, indefinitely.
Nothing drops it, backs it off, or reports it, and the operator sees only a poll ... -> TryNextBlock line that is indistinguishable from a healthy scheduling gap.

It is reachable by an ordinary mistake

Registering a conditional order from an EOA is enough.

Verified against the deployed contracts on a mainnet fork:

  • OwnedTWAP.generateOrder returns a valid WETH to USDC order, so the part is tradeable.
  • OwnedTWAP.poll returns GeneratorResultCode::POST.
  • ComposableCow.getTradeableOrderWithSignature reverts with empty data.

The registry builds the ERC-1271 signature only on a POST verdict, and that path requires the owner to answer supportsInterface(ISignatureVerifierMuxer).
An EOA cannot, and the failure arrives with no payload.

So the worst case is not a rare node fault: it is a user registering against an address that is not a Safe, and shepherd then polling that commitment forever.

This is what Unpollable was for

ParkReason::Unpollable documented exactly this: "The node executed the call and failed without a revert payload, which a fixed gas cap makes deterministic. Retrying burns the poll budget forever."

It was never constructed anywhere, so the condition it named went unhandled, and the variant was removed with the rest of park in #688.
The reasoning in that PR stands: park was inert and consumed the owner quota.
What is missing is a live handling of the condition, not the park mechanism.

Options

A fixed gas cap makes an empty-data revert deterministic, so retrying every block cannot clear it.

  • Back it off rather than retry next block, so the poll budget is not burned, and log it once so it is visible.
  • Or drop it after a repeat, on the grounds that a deterministic payload-free failure is a broken registration.

The second matches how #688 treats registration-fixed refusals: an EOA owner is fixed at registration, and no future poll changes it.
The first is safer if a transient node condition can also produce an empty revert, which is worth confirming before choosing.

Done when

  • An empty-data revert no longer re-polls on every block.
  • It is logged once, distinguishably from a healthy TryNextBlock.
  • A test covers the EOA-owner case end to end, or as a unit over classify_revert.

AI Assistance: Claude Code used for the analysis.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions