Skip to content

feat(providers): surface revert reason from failed RPC sends#1473

Open
droplet-rl wants to merge 1 commit into
masterfrom
droplet/retry-provider-revert-reason
Open

feat(providers): surface revert reason from failed RPC sends#1473
droplet-rl wants to merge 1 commit into
masterfrom
droplet/retry-provider-revert-reason

Conversation

@droplet-rl

Copy link
Copy Markdown
Contributor

Context

From a Slack thread: the relayer logged a confusing error on an eth_estimateGas for fillRelay:

cannot estimate gas; transaction may fail or may require manual gas limit
(error={"reason":"Not enough providers succeeded. Errors:
Provider https://opt-mainnet.g.alchemy.com failed with error: ... execution reverted ... "data":"0x8f260c60" ...

Two issues were raised:

  1. Why does it say "Not enough providers succeeded" when this should be an immediate failure?
  2. Can we reliably catch the revert reason to improve logging?

On (1): that string is RetryProvider.send's unconditional failure wrapper — it fires whenever the required-quorum providers don't all succeed, regardless of cause. For eth_estimateGas/eth_call-at-latest the quorum is 1, so it really means "the one provider we needed reverted." It did fail fast (failImmediate short-circuits on revert keywords), the wording just doesn't reflect that — and the decoded revert (0x8f260c60 = SpokePool RelayFilled(), a benign already-filled race) is left buried in the raw response body.

On (2): yes — the structured error is already parseable via the existing parseJsonRpcError(). This PR demonstrates that and wires it into the failure path.

Change

  • Add getRevertReason() in src/providers/utils.ts — a thin wrapper over parseJsonRpcError() that returns e.g. "execution reverted (0x8f260c60)".
  • In RetryProvider.send(), when the required providers fail:
    • prepend a decoded Revert reason: <message> (<selector>) line to the wrapper message, and
    • keep passing the original rejection as the thrown error's cause, so callers can recover the structured { code, message, data } via parseJsonRpcError(err.cause) instead of scraping the message string.

No behavioural change to control flow (retries/quorum/fail-fast all unchanged) — this only enriches the error surface.

Tests

Added unit tests in test/providers/utils.test.ts that reconstruct the exact ethers v5 SERVER_ERROR ("processing response error") shape from the report and assert reliable extraction:

  • parseJsonRpcError returns { code: 3, message: "execution reverted", data: "0x8f260c60" } (and undefined for non-JSON-RPC errors / null data handled).
  • getRevertReason returns "execution reverted (0x8f260c60)".
  • End-to-end: the revert reason is recoverable from a createSendErrorWithMessage(...) wrapper via parseJsonRpcError(err.cause).
  parseJsonRpcError
    ✔ extracts the JSON-RPC error (code/message/data) from an ethers SERVER_ERROR
    ✔ extracts a revert with null data
    ✔ returns undefined for a plain Error with no JSON-RPC body
    ✔ returns undefined for a non-object
  getRevertReason
    ✔ surfaces the revert message and ABI-encoded selector (RelayFilled = 0x8f260c60)
    ✔ omits the selector when the node returns no data
    ✔ returns undefined when the error is not a JSON-RPC revert
  createSendErrorWithMessage
    ✔ ... (existing, still green)
    ✔ keeps the revert reason recoverable from `cause` via parseJsonRpcError

  13 passing

Follow-ups (not in this PR)

  • Decode the selector to a name (0x8f260c60RelayFilled) against the SpokePool interface for even friendlier logs — the relayer already maps known selectors.
  • The relayer's MultiCallerClient.knownRevertReasons already lists RelayFilled; preserving the structured cause here lets that filtering work even when the error is wrapped by estimateGas/UNPREDICTABLE_GAS_LIMIT.

🤖 Generated with Claude Code

When a required provider fails a send (e.g. an eth_estimateGas that
reverts on-chain), RetryProvider throws a generic "Not enough providers
succeeded" error. For a deterministic revert the underlying JSON-RPC
error - including the ABI-encoded custom error selector - is buried in
the raw response body, so callers and logs can't easily tell that the
tx simply reverted (e.g. RelayFilled / 0x8f260c60) versus a real
provider-availability problem.

Add `getRevertReason()`, a thin wrapper over the existing
`parseJsonRpcError()`, and use it to:
- prepend a decoded "Revert reason: <message> (<selector>)" line to the
  wrapper error message, and
- preserve the original rejection on the thrown error's `cause`, so
  callers can recover the structured `{ code, message, data }` via
  `parseJsonRpcError(err.cause)`.

Add unit tests demonstrating reliable extraction from the exact ethers
v5 SERVER_ERROR ("processing response error") shape seen in production.

Co-Authored-By: Claude Opus 4.8 <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.

1 participant