test(twap-monitor): cover a removal arriving before its create - #672
Open
lgahdl wants to merge 1 commit into
Open
test(twap-monitor): cover a removal arriving before its create#672lgahdl wants to merge 1 commit into
lgahdl wants to merge 1 commit into
Conversation
module.toml already documents the risk: the ConditionalOrderCreated and
ConditionalOrderRemoved subscriptions merge in arrival order, not chain
order. removal_of_an_unindexed_watch_is_a_no_op already proves a
removal with no known create is silently dropped, and
poll_invalid_drops_watch_and_gates already proves the poll-drop
self-heal path - but only for one of the five recognised legacy
IConditionalOrder revert reasons (OrderNotValid). Neither test chains
the two together, and neither exercises the unrecognised-selector
fallback path that SingleOrderNotAuthed() - the actual, parameterless
custom error the deployed ComposableCow.sol reverts with for a removed
order - would take through LegacyRevertAdapter::classify.
This test runs the full sequence for real: a removed log for an
unindexed watch (no-op), its create arriving after (watch persists with
no memory of the earlier removal), then an on_block dispatch where the
mocked eth_call returns keccak256("SingleOrderNotAuthed()")[..4] as the
revert data. Confirms the watch is gone afterward and nothing was
submitted to the venue.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
module.tomlalready documents the risk: theConditionalOrderCreatedandConditionalOrderRemovedsubscriptions "merge in arrival order, not chain order." The existing suite covers each half of that race separately:removal_of_an_unindexed_watch_is_a_no_opproves a removal with no known create is silently dropped.poll_invalid_drops_watch_and_gatesproves the poll-drop self-heal path — but only forOrderNotValid, one of the five recognised legacyIConditionalOrderrevert reasons.Neither test chains the two together, and neither exercises the unrecognised-selector fallback path that
SingleOrderNotAuthed()— the actual, parameterless custom error the deployedComposableCow.solreverts with for a removed order (require(singleOrders[owner][ctx], SingleOrderNotAuthed());in_auth) — would take throughLegacyRevertAdapter::classify.What this adds
One test,
removal_before_its_create_self_heals_via_the_first_poll, running the full sequence for real:ConditionalOrderRemovedlog for an unindexed watch → confirmed no-op (empty store).ConditionalOrderCreatedlog arrives after → confirmed the watch persists, with no memory of the earlier removal.on_blockdispatch where the mockedeth_callreturnskeccak256("SingleOrderNotAuthed()")[..4]as the revert data (the real selector, not a synthetic recognised one) → confirmed the watch is gone afterward and nothing was submitted to the venue.No production code changes — this only adds coverage for a gap the module's own documentation already flags but the test suite didn't close.
Testing
cargo test --manifest-path modules/twap-monitor/Cargo.toml: 26/26 pass (25 pre-existing + the new one), no regressions.AI Assistance: Claude Code was used for the investigation and this test, as part of a follow-up TWAP-flow red-team pass.