fix(quoter-bot): ignore matured configured markets - #186
fix(quoter-bot): ignore matured configured markets#186prd-carapulse[bot] wants to merge 1 commit into
Conversation
Allow configured post-maturity markets to remain in MARKET_IDS without failing the setup readiness gate. Co-authored-by: Julien Thomas <61523188+julien-devatom@users.noreply.github.com>
There was a problem hiding this comment.
Devin Review found 2 potential issues.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| if (latestTimestamp !== undefined && book.maturity <= latestTimestamp) { | ||
| reasons.push(`matured at ${book.maturity}`) | ||
| } |
There was a problem hiding this comment.
🔴 Mature markets still stop quoting
When a matured ladder market needs publication, setup passes but buildLadderTree rejects it. The failed cycle stops every market and starts global cleanup.
Prompt for agents
A matured configured market now passes SetupCheckService, but the writer workflows still treat that lifecycle state as a fatal cycle failure. In bots/quoter-bot/src/infrastructure/ladder/ladder-offer.utils.ts, buildLadderTree throws LadderAdapterError('market-matured') whenever publication or replacement is attempted after maturity. LadderQuoterService records that as a failed reconciliation; runContinuously then halts and the combined QuoterBotService stops all workflows. Audit both ladder and bootstrap processing for matured configured markets. Convert maturity into a non-failing skipped/cleanup outcome that invalidates any owned offers when needed, while allowing other configured markets and later cycles to continue. Add integration coverage for a multi-market cycle where one market is matured and another remains quoteable.
Was this helpful? React with 👍 or 👎 to provide feedback.
| config: SetupCheckConfig, | ||
| latestTimestamp?: bigint | ||
| ) => { | ||
| const bookProblems = (requestedId: `0x${string}`, book: BookSetup, config: SetupCheckConfig) => { |
There was a problem hiding this comment.
🔴 Unused timestamp outage halts startup
After maturity validation is removed, booksCheck still fails every market when the separate timestamp read fails. A redundant RPC outage prevents startup or halts monitoring.
Prompt for agents
Maturity was the only successful use of the latest block timestamp in setup validation. The changed bookProblems no longer accepts it, but SetupCheckService.check still calls SetupStateService.getLatestTimestamp and booksCheck still accepts the capture and appends timestampProviderError to every market. Remove this obsolete readiness dependency across setup-check.service.ts and setup-check.utils.ts, update the SetupStateService contract and ViemSetupStateService implementation if no other caller remains, and replace timestamp retry/failure tests with coverage proving setup performs no timestamp read after maturity is removed.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a56dba04f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| config, | ||
| timestamp.ok ? timestamp.value : undefined | ||
| ) | ||
| : bookProblems(requestedId, response.value, config) |
There was a problem hiding this comment.
Remove the obsolete timestamp readiness dependency
When getLatestTimestamp() rejects while all actual book reads and invariants succeed, booksCheck still receives that failed capture and appends timestampProviderError on the next line, causing assertReady() to retry and eventually prevent the quoter from starting. Since this change removes the maturity comparison—the timestamp value's only use—the timestamp read, parameter, and failure propagation should also be removed so a now-irrelevant RPC failure cannot fail readiness.
Useful? React with 👍 / 👎.
| expect(maturityBoundary.checks.find(check => check.name === 'books')).toMatchObject({ | ||
| status: 'passed', | ||
| observed: [] |
There was a problem hiding this comment.
Exercise the production book reader for matured markets
This test only injects a synthetic getBook result, but production ViemSetupStateService.getBook obtains that result from MidnightApi.fetchBooks and throws unless exactly one row is returned (bots/quoter-bot/src/infrastructure/setup-state/viem-setup-state.service.ts:330-369). The repository's API schema states that /v0/midnight/books excludes past maturities even when IDs are requested (bots/midnight-crossed-books/router-api.json:29-33), so in the production scenario being fixed bookProblems is never reached and setup still fails. Use a metadata path that can return matured markets and cover it at the adapter level rather than only mocking the application port.
Useful? React with 👍 / 👎.
| status: 'passed', | ||
| observed: [] | ||
| }) | ||
| expect(maturityBoundary.ready).toBe(true) |
There was a problem hiding this comment.
Skip matured ladder configs before declaring setup ready
Even if the production book reader is changed to return matured metadata, marking the whole setup ready leaves the matured market in the ladder workflow. For a matured configured market with nonzero capacity and desired rungs, buildLadderTree explicitly throws LadderAdapterError('market-matured') (bots/quoter-bot/src/infrastructure/ladder/ladder-offer.utils.ts:136-138); reconciliation converts that to a failed result (bots/quoter-bot/src/application/ladder/ladder-quoter.service.ts:588-624), and runContinuously halts on that failed cycle (bots/quoter-bot/src/application/ladder/ladder-quoter.service.ts:283-288), aborting the other monitored markets. Filter or explicitly rest/clean up matured strategies before reporting readiness so they are actually ignored.
Useful? React with 👍 / 👎.
Why
A market ID can remain configured after its Midnight market reaches maturity. Treating that expected lifecycle state as a setup invariant failure stops the quoter instead of allowing other configured markets and cleanup logic to continue.
What changed
Verification
pnpm --filter @morpho-org/quoter-bot run typecheck(pass; Node 26 engine warning)pnpm --filter @morpho-org/quoter-bot exec vitest run test/application/setup/setup-check.service.test.ts(46 passed)pnpm exec oxfmt --check bots/quoter-bot/src/application/setup/setup-check.utils.ts bots/quoter-bot/test/application/setup/setup-check.service.test.ts(pass)Closes MKT-2187
Requested by: <@U02N5KRFDB9> | Julien Thomas