Skip to content

fix(market-making): batch invalidation with Midnight multicall - #125

Merged
devatom-adm merged 5 commits into
mainfrom
hermes/batch-offer-revocation
Aug 5, 2026
Merged

fix(market-making): batch invalidation with Midnight multicall#125
devatom-adm merged 5 commits into
mainfrom
hermes/batch-offer-revocation

Conversation

@prd-carapulse

@prd-carapulse prd-carapulse Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Why

Explicit maker-wide invalidation previously submitted one Midnight transaction per offer group. Midnight already exposes an atomic native multicall, so using it reduces nonce pressure and transaction count without adding a helper deployment, authorization, configuration, or dependency surface.

What changed

  • batch explicit all-groups invalidation into one zero-value Midnight multicall(bytes[])
  • require invalidateBatch on every invalidation port, with a read-only no-op implementation, so maker-wide scope cannot fall through to serial invalidation
  • encode every ordered inner call as setConsumed(groupId, MAX_OFFER_CAP, maker) using @morpho-org/midnight-sdk@1.3.0's ABI and constant
  • require the submitting wallet account to equal the configured maker; there is no arbitrary onBehalf input
  • policy-decode the outer multicall and every inner setConsumed call, enforcing the configured Midnight target, zero value, exact ordered groups, amount, and maker
  • report the one confirmed transaction hash against every selected group, then forget all confirmed bot-owned groups together
  • preserve the direct Midnight setConsumed path for explicit single-group invalidation and preserve read-only behavior
  • never retry serially after a submitted multicall fails
  • leave normal bootstrap and ladder mutation loops unchanged
  • remove the proposed RevokeOffers contract, Foundry test, deployment script, generated export, package script/dependency, REVOKE_OFFERS_ADDRESS configuration/YAML, and authorization/deployment documentation

Safety guarantees

  • extra, missing, reordered, duplicate-substituted, malformed, wrong-selector, wrong-amount, wrong-target, nonzero-value, and wrong-maker calldata is rejected before submission
  • Midnight native multicall uses delegatecall, preserving the maker EOA as msg.sender for each internal setConsumed
  • maker-wide invalidation always uses the mandatory batch capability for selected groups; invalidate is reserved for explicit single-group scope
  • a reverted or failed submitted multicall is surfaced against every selected group with the shared hash and is never serially retried
  • ownership cleanup happens once, for all groups, only after multicall confirmation; cleanup failures retain the shared hash for every group
  • read-only mode never derives a signer or submits a transaction

Protocol evidence

The installed @morpho-org/midnight-sdk@1.3.0 exports:

  • multicall(bytes[])
  • setConsumed(bytes32,uint128,address)
  • MAX_OFFER_CAP = 340282366920938463463374607431768211455 (type(uint128).max)

Validation

  • TDD RED: the mandatory-batch read-only regression failed because maker-wide scope serialized through invalidate; GREEN invokes invalidateBatch exactly once and never calls invalidate
  • focused invalidation tests: 24 passed, 0 failed
  • bun run --filter @morpho-org/market-making-bot typecheck: passed
  • bun lint: 0 warnings, 0 errors
  • bun format: completed
  • bun run --filter @morpho-org/market-making-bot jsdoc:build: passed
  • bun run --filter @repo/contracts build: passed (required to generate workspace contract exports before the full suite)
  • bun test: 1,426 passed, 3 skipped, 0 failed
  • tracked repository and final added-line scans contain no RevokeOffers or REVOKE_OFFERS_ADDRESS

Requested by: @julien (Slack)

@prd-carapulse prd-carapulse Bot changed the title feat(market-making): batch offer invalidation feat(market-making): batch invalidation with native multicall Aug 4, 2026
@prd-carapulse prd-carapulse Bot changed the title feat(market-making): batch invalidation with native multicall fix(market-making): batch invalidation with Midnight multicall Aug 4, 2026
@julien-devatom

Copy link
Copy Markdown
Collaborator

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 9b5b2f439b

ℹ️ 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".

@devatom-adm
devatom-adm marked this pull request as ready for review August 5, 2026 09:17
@devatom-adm
devatom-adm merged commit 3a573f6 into main Aug 5, 2026
5 of 6 checks passed
@devatom-adm
devatom-adm deleted the hermes/batch-offer-revocation branch August 5, 2026 09:17

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 1 additional finding in Devin Review.

Open in Devin Review

Comment on lines +25 to +32
export const invalidateOffersBatch = async (parameters: {
wallet: BatchOfferInvalidationWallet
midnight: Address
maker: Address
groupIds: readonly Hex[]
receiptTimeoutMs: number
onTransactionSubmitted?: (txHash: Hex) => void | Promise<void>
}) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 New public code file is left out of the documentation coverage inventory

The new batch invalidation helper file is added to the bot's public source surface (invalidateOffersBatch at bots/market-making/src/infrastructure/invalidation/batch-offer-invalidation.utils.ts:25) without being registered in the documentation inventory and generator configuration, so its documentation is never verified or published.
Impact: The documentation build silently skips this new public code, so missing or wrong docs for it go undetected.

Mandatory boundary-file registration in check-jsdoc.ts and typedoc.json

.agents/skills/build-jsdoc/SKILL.md states: "When adding another boundary file, add it to both scripts/check-jsdoc.ts and typedoc.json in the same change." The inventory in bots/market-making/scripts/check-jsdoc.ts:300-386 is a static allowlist and lists the sibling invalidation files (infrastructure/invalidation/offer-invalidation-group.utils.ts, offer-invalidation-transaction.utils.ts, production-offer-invalidation.ts) but not infrastructure/invalidation/batch-offer-invalidation.utils.ts. The same omission exists in bots/market-making/typedoc.json:58-61.

Prompt for agents
The new public boundary file bots/market-making/src/infrastructure/invalidation/batch-offer-invalidation.utils.ts exports invalidateOffersBatch but is not registered in the JSDoc coverage inventory (bots/market-making/scripts/check-jsdoc.ts, static sourceFiles allowlist around line 355) nor in the TypeDoc entry points (bots/market-making/typedoc.json, invalidation entries around line 58). The build-jsdoc skill requires adding any new boundary file to both in the same change. Add the path alongside the other infrastructure/invalidation entries in both files and re-run the jsdoc check/build to confirm the printed inventory includes the new callable exactly once.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

haydenshively added a commit that referenced this pull request Aug 5, 2026
main gained #121/#124/#125 after this branch's merge-base, which is why GitHub
reported the stack as conflicting and suppressed its checks entirely — the
mergeability of a stacked PR is computed against main, not the immediate parent.
This is the "six open PRs add new bun:test files and Bun.* calls" follow-up the
PR body anticipated, now due for the three that merged.

Applied the same mechanical port to what main brought in:

- 8 test files from bun:test to vitest (`mock()` -> `vi.fn()`), plus 2 more
  where 130 had already converted the import and main added new `mock(` sites
- Bun.file -> readFile, Bun.serve -> the node:http adapter this branch already
  uses, Bun.sleep -> node:timers/promises, in the setup-state test
- 3 more floating `expect(...).rejects` awaited — the same latent bug class this
  branch documents, arriving from main where bun's typings still hid them

Verified: 1459 pass / 1 skip / 141 files, zero failures; vitest collects exactly
the 141 tracked *.test.ts files (diffed, zero delta); pnpm lint 0/0; knip clean;
12/12 typecheck; jsdoc:build exits 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
haydenshively added a commit that referenced this pull request Aug 7, 2026
main gained #121/#124/#125 after this branch's merge-base, adding test files
that still import bun:test and call bun's mock(). Linearizing the stack dropped
these conversions along with the merge commit that carried them, so they are
restored here as their own commit: 7 files moved to vitest imports and
mock() -> vi.fn().

Tree is byte-identical to the verified pre-rebase state.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
haydenshively added a commit that referenced this pull request Aug 7, 2026
main gained #121/#124/#125 after this branch's merge-base, adding test files
that still import bun:test and call bun's mock(). Linearizing the stack dropped
these conversions along with the merge commit that carried them, so they are
restored here as their own commit: 7 files moved to vitest imports and
mock() -> vi.fn().

Tree is byte-identical to the verified pre-rebase state.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
haydenshively added a commit that referenced this pull request Aug 7, 2026
main gained #121/#124/#125 after this branch's merge-base, adding test files
that still import bun:test and call bun's mock(). Linearizing the stack dropped
these conversions along with the merge commit that carried them, so they are
restored here as their own commit: 7 files moved to vitest imports and
mock() -> vi.fn().

Tree is byte-identical to the verified pre-rebase state.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

2 participants