Skip to content

fix(deposit-address): cross-instance write-ahead lock + broadcast verification on the v3 execute path - #3763

Draft
ashwinrava wants to merge 1 commit into
masterfrom
ashwin/pda-execute-write-ahead-lock
Draft

fix(deposit-address): cross-instance write-ahead lock + broadcast verification on the v3 execute path#3763
ashwinrava wants to merge 1 commit into
masterfrom
ashwin/pda-execute-write-ahead-lock

Conversation

@ashwinrava

Copy link
Copy Markdown
Member

What happened

A persistent-deposit-address (PDA) funding transfer was executed twice by the deposit-address bot — two sweep transactions from the same signer landed in consecutive blocks, each referencing the same inbound ERC-20 transfer in their provenance metadata. The second execute succeeded on-chain only because the deposit address also held funds from a different pending deposit; the duplicate silently consumed that neighbor deposit's balance. The neighbor deposit's row then failed the bot's balance pre-check on every later poll (balance < its recorded amount) and stranded until manual intervention.

Two protections existed and both have a structural gap:

  1. The in-flight lock (observedExecutedDeposits) is process memory. The instance coordinator hands over by writing the new instance into Redis and having the old instance notice on a 1s poll — so during a handover, two instances briefly poll the queue concurrently. The indexer's delivery queue intentionally redelivers fresh rows for 15 minutes (at-least-once delivery, so bot restarts can't lose transfers), which guarantees both instances receive the same row. Neither can see the other's in-memory lock.
  2. sendAndConfirmTransaction swallows the broadcast hash on failure. A receipt timeout or RPC error after a successful broadcast returns undefined — indistinguishable from "never sent". The caller releases its lock and re-executes on the next 1s poll while the first transaction is still propagating.

Exactly-once delivery from the queue is not achievable (marking a row consumed happens when the response is sent, not when the bot has durably acted), so dedupe must live at the effect boundary — in this bot.

The fix

1. Cross-instance write-ahead lock (_acquireExecuteLock): before requesting/broadcasting an execute, take a Redis SET NX PX lock keyed by depositKey (180s TTL, token = run identifier). A concurrent instance skips the row instead of double-executing. The lock is released when the attempt provably ended before any broadcast; after an unknown-outcome broadcast it is left to expire as a cool-down. Fails open on Redis errors — a Redis blip must not stop sweeping (reverts to today's single-instance dedupe).

2. Broadcast verification instead of blind retry (_resolvePriorExecuteAttempt): sendAndConfirmTransactionWithHash (new, non-breaking; sendAndConfirmTransaction now delegates to it) surfaces the broadcast tx hash even when confirmation fails. An unknown-outcome broadcast is persisted to Redis (attempted-execute record, 24h TTL). Before any new execute for the same depositKey, the bot checks that hash's receipt:

  • landed & succeeded → promoted to the executed set, never re-executed;
  • reverted → record cleared, retry proceeds;
  • unreceipted → blocked within a 15-minute grace window, presumed dropped after it;
  • unverifiable (provider error) → skip this poll, err on the side of not double-executing.

Notes

  • v3 deposit-execute path only. The v1 deposit path and the withdraw paths have analogous (lower-stakes) exposure and can adopt the same helpers as a follow-up.
  • A duplicate execute on an address holding a single deposit merely reverts (wasted gas); the damaging case is precisely a multi-deposit address, which PDAs are designed to be.
  • Complementary hardening tracked separately: idempotent execute signatures on the API side (duplicate requests returning the same single-use-nonce calldata, so any racing caller reverts on-chain), and indexer-side redelivery suppression once a sweep is indexed.

Tests

  • 6 new unit tests covering: lock-held skip, lock release on pre-broadcast failure, promotion of a landed prior broadcast, grace-window blocking of an unreceipted broadcast, reverted-broadcast clearing, and post-grace retry.
  • All 70 tests in test/DepositAddressHandler.ts pass; tsc --noEmit, eslint, prettier clean.

🤖 Generated with Claude Code

…ification on the v3 execute path

Co-Authored-By: Claude Fable 5 <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