[ANCHOR-1231]: Race condition between RPC events and transaction index#1962
Merged
Conversation
* fix potential race condition when fetching stellar transactions * update transaction fetching to use helper that waits for availability
…ndition retry * add a new test for * add verification that retries * add coverage for a race condition where rpc events index outpaces transaction index
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a race where Soroban RPC “transfer” events can be observed before the corresponding transaction is available via the transaction index, by adding a retry/wait layer around transaction lookup and validating the behavior with a new unit test.
Changes:
- Update
StellarRpcPaymentObserver.processTransferEvent()to wait/retry untilgetTransaction()returns a transaction (or times out), viaLedgerClientHelper.waitForTransactionAvailable(...). - Add a unit test covering the “events index ahead of transactions index” scenario by simulating
getTransaction()returningnullon the first call and a populated transaction on the second call.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| platform/src/main/java/org/stellar/anchor/platform/observer/stellar/StellarRpcPaymentObserver.java | Uses LedgerClientHelper.waitForTransactionAvailable() to tolerate RPC/tx-index timing races when processing transfer events. |
| platform/src/test/kotlin/org/stellar/anchor/platform/observer/stellar/StellarRpcPaymentObserverTest.kt | Adds a regression test ensuring transaction lookup is retried when the first lookup returns null. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* update soroban authorization entry validity from 10 to 60 ledgers * add explicit error handling for failed sendtransaction responses by throwing a runtimeexception
…eval * refactor transaction retrieval in rpc handlers * update to use * update to use * remove manual transaction check
…and transaction waiting * refactor sharedTestPaymentValues with double-checked locking for thread safety * update waitForTransactionAvailable with explicit timeout parameters
amandagonsalves
marked this pull request as draft
June 29, 2026 21:23
* update from 60 to 10 ledgers * ensure faster expiration of soroban authorization entries for improved testing and simulation
* update the number of past ledgers checked when fetching stellar rpc events initially * fix potential missing events by checking more historical ledgers on startup
* refactor how test payment values are shared, moving from a global state to a per-class map * add a retry mechanism for sending test payments to the ledger, improving integration test robustness
* fix start ledger from being less than 1 * update stellar event fetching to start from a valid ledger
* update log message when transaction confirmation times out * update log message when transaction is rejected
* delete log utility usage * refactor error handling for missing stellar transactions * update exception type from to
* update retry count in sep24 and sep31 e2e tests * increase test stability for asynchronous operations
* update expected exception from to * update expected error message for failed stellar transaction retrieval
amandagonsalves
marked this pull request as ready for review
June 30, 2026 00:14
JiahuiWho
approved these changes
Jun 30, 2026
amandagonsalves
added a commit
that referenced
this pull request
Jul 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fix a family of RPC race conditions. All changes address the same root cause: one RPC call observes a
state that a subsequent call on a different node hasn't indexed yet.
StellarRpcPaymentObserver— replace baregetTransaction()withwaitForTransactionAvailable()so the observer retries before giving up and losing the eventNotifyOnchainFundsSentHandler— same retry wrapper for the deposit pathNotifyOnchainFundsReceivedHandler— same retry wrapper for the withdrawal/SEP-31 pathStellarRpcPaymentObserver— change initial cursor offset fromlatestLedger - 1tolatestLedger - 5to prevent startup loop whengetLatestLedgerandgetEventshit nodes at different ledgersWalletClient/PaymentClient— surfacesendTransactionerrors immediately instead of silently returning a hash for a rejected transactionPlatformAPITestBase— per-class payment map with retry loop to fix both sequence number collisions between parallel test classes and cross-class payment hash isolationContext
ANCHOR-1231
Observer race (
StellarRpcPaymentObserver/ RPC handlers)getEventsandgetTransaction(hash)hit different nodes. The events index is ahead of the transactions index →getTransactionreturnsnull→ exception swallowed → cursor advances → event permanently lost. Confirmed in CI logs:getEventsreturned the SAC transfer event at13:41:36.443,getTransactionreturned null 44 ms later at13:41:36.487.Fix:
LedgerClientHelper.waitForTransactionAvailablepollsgetTransactionup to 10 × 1 s.Observer startup cursor loop (
StellarRpcPaymentObserver)On a null cursor (startup or reset), the observer calls
getLatestLedger()and thengetEvents(startLedger = latestLedger - 1). These hit different nodes. If the second node is even one ledger behind, it returnsstartLedger out of range. With a null cursor, the observer loops on startup forever and processes no events. This caused all observer-dependent tests (SEP-6, SEP-24, SEP-31 full flows) to fail simultaneously.Fix: use
latestLedger - 5as the initial offset. Tolerates up to 5 ledgers of inter-node lag.Test parallel sequence number collision (
PlatformAPITestBase)testPaymentValueswas an instance-levellateinit var. Four test subclasses running in parallel all calledsendTestPayment()from the same signing key, all read the same sequence number before any transaction landed, and three gottxBAD_SEQ.Fix: shared companion object with double-checked locking — only one payment is submitted per JVM.
Testing
./gradlew :platform:test --tests \ "org.stellar.anchor.platform.observer.stellar.StellarRpcPaymentObserverTest" ./gradlew :platform:test ./gradlew :core:test ./gradlew runEssentialTestsThe new unit test (
processTransferEvent retries getTransaction when RPC events index races ahead of transactions index) mocksgetTransactionto returnnullon the first call and a valid transaction on the second, asserts exactly two calls were made, and verifies the event was still delivered to the listener.Documentation
N/A
Known limitations
waitForTransactionAvailableis 10 × 1 s. An unusually slow node could still exceed it; in that case the event is logged as a warning and the cursor advances (same behavior as before the fix — no regression).