Skip to content

fix(cow): attribute an unreadable poll refusal before acting on it - #694

Closed
mfw78 wants to merge 1 commit into
mainfrom
cow/692-empty-revert
Closed

fix(cow): attribute an unreadable poll refusal before acting on it#694
mfw78 wants to merge 1 commit into
mainfrom
cow/692-empty-revert

Conversation

@mfw78

@mfw78 mfw78 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Closes #692. Found by the anvil-fork harness in #691.

The bug

classify_revert looked for a selector in the revert payload and fell through to TryNextBlock whenever it could not find one, so a refusal with no readable payload put the commitment back on the block clock, every block, forever, and silently: the operator saw only poll ... -> TryNextBlock, indistinguishable from a healthy scheduling gap.

The poll interface is closed, so an empty payload is a defect

A generator answers through GeneratorResult codes and never reverts; the registry has its own refusals and they carry selectors. Nothing in that contract produces an empty payload, so one names a defect rather than a state to wait out.

It does not say whose, and the candidates are not alike:

  • A codeless owner. The registry probes the owner for ERC-1271 support before building a signature. Solidity guards that call with extcodesize, so an owner with no code makes it revert in the caller"s own frame with nothing attached. This is the case the fork run hit.
  • A gas cap too low for the handler. POLL_GAS_CAP is 30M; exhausting it reverts empty.
  • A registry address that is not the fork. The selector misses, the fallback reverts empty.

The first is the registration answering for itself. The other two are the operator"s, and identical for every commitment.

So it attributes before acting

classify_revert no longer guesses. It returns a Refusal (Named, Unattributed, Transport) and the keeper resolves the unattributed case with one eth_getCode:

  • Owner has no code -> drop, logged at error level naming the owner. No later poll of this commitment changes it.
  • Owner has code -> back off an hour and name the likely causes. Dropping here would delete an entire watch set to report a typo, which is the opposite of reporting it.
  • The read fails -> unattributable, so it takes the backoff path. The safe reading of "no answer" is the loud one.

Why the codeless case is not simply terminal

It is terminal for the commitment, and it drops. Worth naming why it is not treated as permanent in principle: an EOA can gain code through an EIP-7702 delegation, which is exactly the flow #658 describes. Someone who registers first and delegates second is recoverable, and the drop is loud enough for them to see it.

Relationship to the removed Unpollable

This is the condition ParkReason::Unpollable documented, "the node executed the call and failed without a revert payload ... retrying burns the poll budget forever".
That variant was never constructed, and park was removed in #688 for being inert and consuming the owner quota. What was missing was live handling, which this adds without reintroducing park.

Verification

236 tests pass, up from 234 on main.
cargo fmt --check, cargo clippy --workspace --all-targets --all-features -- -D warnings, RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps, just build-modules and the doctests all exit 0.

Four mutations are each caught by one test: a codeless owner backing off instead of dropping, a contract owner dropping instead of backing off, an unreadable payload classifying as named, and a transport fault losing its own class.

A diagnostic regression was caught on the way: restructuring the drop log dropped the node message that poll_invalid_drops_commitment_and_gates asserts on. It is back.

AI Assistance: Claude Code used for the fix and the tests.

`classify_revert` fell through to `TryNextBlock` whenever it could not
find a selector, so a refusal with no readable payload re-polled on
every block indefinitely, silently, with nothing to distinguish it from
a healthy scheduling gap.

The poll interface is closed: a generator answers through
`GeneratorResult` codes and the registry's own refusals carry selectors.
An empty payload is outside that, so it names a defect. It does not say
whose, and the candidates are not alike, so the classifier no longer
guesses. `classify_revert` returns a `Refusal` and the keeper attributes
it with one `eth_getCode`.

A codeless owner cannot answer the registry's ERC-1271 probe, which
makes that call revert in the caller's own frame with nothing attached.
That is the registration answering for itself and it drops, loudly.

Every other cause points outward: a gas cap too low for the handler, or
a registry address that is not the fork. Those are the same for every
commitment, so a drop would delete a whole watch set to report an
operator's typo. They back off for an hour and name the likely cause.
A failed `eth_getCode` reads as unattributable and takes that path too.

Reachable in production by an ordinary mistake, and the codeless case
is recoverable rather than permanent: an EOA can gain code through an
EIP-7702 delegation, which is exactly the flow #658 describes.

Closes #692.

AI Assistance: Claude Code used for the fix and the tests.
@mfw78
mfw78 force-pushed the cow/692-empty-revert branch from 2be728b to 4cd442d Compare September 9, 2026 01:42
@mfw78 mfw78 changed the title fix(cow): back off a payload-free revert instead of re-polling forever fix(cow): attribute an unreadable poll refusal before acting on it Sep 9, 2026
@mfw78

mfw78 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Folded into #691, which is where the bug was found and where the harness that surfaced it lives. The commit rides there unchanged; nothing is dropped.

One thing did not survive the fold, and it is worth recording. I tried to add an end-to-end case for this path, registering from a genuinely codeless owner, and it does not reproduce under the engine.

The chain behaves as the fix assumes: OwnedTWAP.poll returns POST for that owner, and getTradeableOrderWithSignature with the module own call shape reverts with empty data. Under the engine the same registration polls to TryNextBlock with no warning logged, which means the eth_call succeeded and decoded a TRY_NEXT_BLOCK generator code instead of reverting. I could not reconcile the two, so I did not ship a test I cannot explain. The behaviour stays covered by unit tests over classify_revert and the attribution.

Also recorded on #691: anvil account zero already carries an EIP-7702 delegation on mainnet, which a fork inherits, so it is not a codeless owner. An earlier round of this investigation was wrong for exactly that reason.

@mfw78 mfw78 closed this Sep 9, 2026
@mfw78
mfw78 deleted the cow/692-empty-revert branch September 9, 2026 02:20
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.

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

1 participant