Fix replay cache reservation before transaction validation causing incorrect AlreadyProcessed errors

[jeffbrownn]

Summary

Description

This PR fixes an issue where a transaction signature is inserted into the replay/cache before full validation and scheduling are completed.

Currently, the flow calls push(signature, None) prior to:

• ensure_transaction_accounts()
• transaction scheduling/execution pipeline

If any of these steps fail, the transaction is not actually processed, but its signature remains reserved in the replay cache. As a result, retrying the same signed transaction may incorrectly return AlreadyProcessed, effectively breaking the retry mechanism.

Solution

Move replay cache insertion to occur only after the transaction is successfully handed off to the execution pipeline (schedule / execute).

This ensures that:

• failed validation or pre-checks do not reserve the signature
• retries of the same transaction remain valid
• replay protection only applies to transactions that actually entered processing

Compatibility

• No breaking changes
• Config change (describe):
• Migration needed (describe):

Testing

• tests (or explain)

Checklist

• docs updated (if needed)
• closes #

Summary by CodeRabbit

• Refactor
  • Optimized transaction processing and signature cache management timing.

[coderabbitai]

📝 Walkthrough

Walkthrough

The handler now performs an early check for transaction signatures in the cache and immediately returns an error if found, without initially reserving the signature. After handling account resolution and executing or scheduling the transaction, it then reserves the signature in the cache. If the cache reservation fails at this later stage, it returns an already processed error. This changes the timing of when signatures are cached—previously they were checked and reserved upfront, now reservation is deferred until after preflight and scheduling operations complete.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

magicblock-aperture/src/requests/http/send_transaction.rs (1)

37-56: ⚠️ Potential issue | 🔴 Critical

The deferred contains()/push() flow is racy and can clobber cached results.

Between Line 38 and Line 55, two concurrent requests with the same signature can both pass contains() and both enter schedule()/execute(). ExpiringCache::contains (magicblock-aperture/src/state/cache.rs:79-82) is only a read, and ExpiringCache::push (magicblock-aperture/src/state/cache.rs:57-72) is an upsert, so the loser not only detects the duplicate too late but can also overwrite an existing Some(result) with None. This needs a true insert-if-absent reservation immediately before handoff, plus rollback if the handoff fails.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@magicblock-aperture/src/requests/http/send_transaction.rs` around lines 37 -
56, The current contains()/push() window is racy: replace the deferred
check+push with an atomic insert-if-absent reservation in the transactions cache
(use ExpiringCache API or add an insert_if_absent method) so that when handling
a signature you immediately reserve it (e.g., insert a placeholder
None/Reserved) before calling ensure_transaction_accounts() or
transactions_scheduler.schedule/execute; if schedule/execute returns an error
you must rollback/remove the reservation to avoid clobbering an existing
Some(result); update the flow around contains(), push(),
self.transactions_scheduler.schedule/execute, and push() usage to perform
reserve→handoff→commit/rollback atomically.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@magicblock-aperture/src/requests/http/send_transaction.rs`:
- Around line 37-56: The current contains()/push() window is racy: replace the
deferred check+push with an atomic insert-if-absent reservation in the
transactions cache (use ExpiringCache API or add an insert_if_absent method) so
that when handling a signature you immediately reserve it (e.g., insert a
placeholder None/Reserved) before calling ensure_transaction_accounts() or
transactions_scheduler.schedule/execute; if schedule/execute returns an error
you must rollback/remove the reservation to avoid clobbering an existing
Some(result); update the flow around contains(), push(),
self.transactions_scheduler.schedule/execute, and push() usage to perform
reserve→handoff→commit/rollback atomically.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 44ca7f06-c95f-475d-b4a6-9b627ba02881

📥 Commits

Reviewing files that changed from the base of the PR and between f8a5871 and dc8dd7a.

📒 Files selected for processing (1)

• magicblock-aperture/src/requests/http/send_transaction.rs