Skip to content

fix(delivery): classify the receiver's MessageAlreadyProcessed as idempotent success - #52

Merged
DylanVerstraete merged 1 commit into
mainfrom
fix/duplicate-delivery-receiver-selector
Sep 1, 2026
Merged

fix(delivery): classify the receiver's MessageAlreadyProcessed as idempotent success#52
DylanVerstraete merged 1 commit into
mainfrom
fix/duplicate-delivery-receiver-selector

Conversation

@DylanVerstraete

Copy link
Copy Markdown
Collaborator

Found via last night's soak on usc-devnet: 144 deliveries, 143 succeeded, and the single Reverted was not a failure at all.

What happened

A restart replayed the delivery checkpoint and resent a message that had already been fully processed:

ERROR ❌ delivery reverted; no further retries chain_key=8
  message_id=0x635ab2e7… reason=… execution reverted, data: "0x730ac1e2635ab2e7…"

0x730ac1e2 decodes to MessageAlreadyProcessed(bytes32)MessageReceiverBase's duplicate guard — carrying our own messageId. The delivery goal was already achieved, but the classifier only knew the Inbox duplicate guard (MessageAlreadyValidated / "Already validated"), so the receiver guard fell through to the terminal path: gas burned on the resend, a terminal ERROR logged, and deliver_tx{status=Reverted} incremented — polluting the one status that should mean a genuine failure.

Change

  • revert_already_validatedrevert_duplicate_delivery, now matching both guards, each three ways (revert string, decoded error name, raw 4-byte selector) since node dialects differ. All three call sites (simulate, send error, mined-and-reverted) flow through this single classifier.
  • Minimal IMessageReceiver mirror in write_ability::abi, asserted against the compiled MessageReceiverBase artifact by the ABI drift gate.
  • Dialect test extended with the receiver cases — including the usc-devnet revert string verbatim — and the selector pinned to [0x73, 0x0a, 0xc1, 0xe2] so a mirror edit that silently stops matching real reverts fails in CI rather than on devnet.

Verification

  • cargo test --workspace — 150 tests pass, 0 failed.
  • ABI gate run against a compiled asc-contracts checkout: passes, proving the mirror matches the real artifact.
  • clippy -D warnings clean, fmt clean.

…mpotent success

Two on-chain guards can reject a delivery as a duplicate, and only one was
matched. The Inbox guard (MessageAlreadyValidated / the SimpleInbox "Already
validated" string) was treated as idempotent success; the receiver-side guard —
MessageReceiverBase.MessageAlreadyProcessed, which fires when the callback
already ran for this messageId — was not, so it fell through to the terminal
revert path.

A restart replaying the delivery checkpoint hit exactly this on usc-devnet
(2026-09-01):

    ERROR ❌ delivery reverted; no further retries chain_key=8
      message_id=0x635ab2e7… reason=… execution reverted,
      data: "0x730ac1e2635ab2e7…"

0x730ac1e2 is MessageAlreadyProcessed(bytes32) carrying our own messageId: the
message was long since delivered and processed, yet the relayer burned gas on
the resend, logged a terminal ERROR and counted status=Reverted — polluting the
one deliver_tx status that should mean a real failure.

Rename revert_already_validated to revert_duplicate_delivery and match the
receiver guard the same three ways as the Inbox guard (string, decoded name,
raw selector). All three call sites — simulate, send error, mined-and-reverted —
flow through this one classifier.

The mirror lives in write_ability::abi as a minimal IMessageReceiver interface
and is asserted against the compiled MessageReceiverBase artifact by the ABI
drift gate; the dialect test additionally pins the selector to 0x730ac1e2 so a
signature edit that silently stops matching real reverts fails in CI.
@cursor

cursor Bot commented Sep 1, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Narrow change to delivery revert classification and ABI mirroring; it reduces false terminal failures without altering auth, funding, or send semantics.

Overview
Checkpoint replay after a relayer restart could resend deliveries that had already finished on the destination. The worker only treated Inbox duplicate reverts (MessageAlreadyValidated / "Already validated") as idempotent success, so receiver duplicates (MessageReceiverBase.MessageAlreadyProcessed) were logged as terminal failures and counted as Reverted even though the message had already been processed.

The shared classifier is renamed to revert_duplicate_delivery and extended to match both guards the same way (revert string, decoded error name, and raw selector) across simulate, gas estimate, and send paths. write-ability gains a minimal IMessageReceiver mirror for that error, the ABI drift gate pins it to MessageReceiverBase, and unit tests cover the usc-devnet revert payload plus a fixed selector byte pin so mirror drift fails in CI.

Reviewed by Cursor Bugbot for commit 8d07c98. Bugbot is set up for automated code reviews on this repo. Configure here.

@DylanVerstraete
DylanVerstraete merged commit 81a13ee into main Sep 1, 2026
10 checks passed
@DylanVerstraete
DylanVerstraete deleted the fix/duplicate-delivery-receiver-selector branch September 1, 2026 16:59
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