Skip to content

[ANCHOR-1218]: StellarRpcPaymentObserver indexes filtered op list with full-tx operationIndex → payments never credited - #1953

Merged
amandagonsalves merged 5 commits into
developfrom
fix/rpc-observer-op-index-domain
Jun 23, 2026
Merged

amandagonsalves merged 5 commits into
developfrom
fix/rpc-observer-op-index-domain

Conversation

@amandagonsalves

Copy link
Copy Markdown
Collaborator

Description

Before this change, StellarRpcPaymentObserver.processTransferEvent selected the credited operation with:

LedgerOperation op = txn.getOperations().get(result.event.getOperationIndex().intValue());

event.getOperationIndex() is a 0-based index into the full on-chain operation array as returned by the Soroban-RPC getEvents API. txn.getOperations() is not that full array — it is the compacted output of LedgerClientHelper.getLedgerOperations(), which silently drops every operation for which convert() returns null (MANAGE_DATA, CHANGE_TRUST, CREATE_ACCOUNT, BUMP_SEQUENCE, SET_OPTIONS, any INVOKE_HOST_FUNCTION whose direct function name is not "transfer", etc.) with no placeholder. Using the full-list index against the shorter compacted list produced two failure modes:

  • Silent drop (Variant A): any multi-operation transaction containing a filtered op at or before the payment → IndexOutOfBoundsException → caught by the generic handler → event permanently dropped, SEP-6/24/31 transaction frozen, user funds received but never credited.
  • Sub-invocation skip (Variant B): a top-level INVOKE_HOST_FUNCTION whose direct function name is not "transfer" (contract/SEP-45 sub-invocation) emits a transfer event at the full-list index of that op, which convert() drops → same out-of-bounds or wrong-element result.

The fix stops trusting positional alignment. convert() stores the TOID — derived from (sequenceNumber, applicationOrder, 1-based opIndex) — as each operation's id. The event's operationIndex (0-based) is converted to the same 1-based coordinate, the TOID for the target operation is derived, and the compacted list is stream-filtered by identity match. When no match is found (sub-invocation variant), the event is logged at error level and skipped rather than silently swallowed as a generic WARN.

Changes

  • StellarRpcPaymentObserver.processTransferEvent: replaced txn.getOperations().get(operationIndex) with a TOID-based identity lookup — derives wantedOpId from new TOID(sequenceNumber, applicationOrder, operationIndex + 1).toInt64(), then stream-filters txn.getOperations() by getOperationId(o).equals(wantedOpId). Adds import org.stellar.sdk.TOID.
  • StellarRpcPaymentObserver.getOperationId: new private helper that extracts the id field from whichever sub-operation (paymentOperation, pathPaymentOperation, or invokeHostFunctionOperation) is set on a LedgerOperation.
  • StellarRpcPaymentObserver.processTransferEvent null branch: when TOID lookup yields no match, logs an errorF message identifying the transaction hash and operation index, then returns — no exception is swallowed, the observer stays RUNNING.
  • StellarRpcPaymentObserverTest: three new unit regression tests for processTransferEvent — single-op at operationIndex=0 still credited, multi-op with non-payment op at index 0 and payment at index 1 correctly credited, sub-invocation with empty compacted list skips without calling handleEvent.
  • StellarRpcObserverIndexDomainTest (new file): three integration tests using a real scheduler (observer.start()) and a CaptureListener — multi-op transaction credited end-to-end, sub-invocation skipped with observer remaining RUNNING, mixed batch crediting both normal and multi-op events while skipping the sub-invocation.

Acceptance Criteria

  • A two-operation transaction [MANAGE_DATA, PAYMENT→dist] with transfer event at operationIndex=1 is credited with the correct from/to/amount/operationId.
  • A transfer event whose corresponding full-list operation is absent from the compacted list (contract sub-invocation) is not credited; no PaymentTransferEvent is dispatched to listeners.
  • A mixed batch containing a normal single-op, a multi-op, and a sub-invocation event processes all three without crashing, crediting the two creditable ops and skipping the sub-invocation.
  • Observer status remains RUNNING after any of the above scenarios.
  • Existing StellarRpcPaymentObserverTest and StellarRpcObserverPoisonResilienceTest tests continue to pass unchanged.
  • Horizon mode (HorizonPaymentObserver) is unaffected.

Context

#3791580

Testing

  • Unit: ./gradlew :platform:test --tests "org.stellar.anchor.platform.observer.stellar.StellarRpcPaymentObserverTest"
  • Integration: ./gradlew :platform:test --tests "org.stellar.anchor.platform.observer.stellar.StellarRpcObserverIndexDomainTest"
  • Full platform suite: ./gradlew :platform:test

Documentation

N/A

Known limitations

The sub-invocation case (Variant B) — a transfer event emitted by a contract that moves tokens internally rather than via a top-level SAC transfer call — is logged as an error and skipped. Crediting from the authenticated event fields (the reporter's Option 2) would handle this variant without requiring a matching operation in the compacted list, but it requires verifying that the SAC contract address can be reliably resolved to an anchor asset from the event alone, which is not currently supported by SacToAssetMapper. Variant B is tracked separately.

* fix operation lookup for soroban events using `toid`
* refactor operation identification to correctly handle `operationindex`
* add a method to get `ledgeroperation` id
* add check to skip contract sub-invocations without direct operations
* add tests for operation index mapping and sub-invocation scenarios

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a critical indexing bug in StellarRpcPaymentObserver.processTransferEvent where Soroban-RPC event operationIndex (full on-chain op array) was incorrectly used to index into a filtered/compacted LedgerTransaction.operations list, causing some valid payment events to be dropped and never credited.

Changes:

  • Replace positional indexing with a TOID-derived operation-id lookup to find the correct compacted LedgerOperation.
  • Add a helper to extract operation ids from the concrete sub-operation type on LedgerOperation.
  • Add unit + domain/integration regression tests covering single-op, multi-op-with-filtered-op, and sub-invocation (no matching compacted op) scenarios.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
platform/src/main/java/org/stellar/anchor/platform/observer/stellar/StellarRpcPaymentObserver.java Uses TOID-based identity matching to select the correct operation from the compacted list and logs/skips when no match exists.
platform/src/test/kotlin/org/stellar/anchor/platform/observer/stellar/StellarRpcPaymentObserverTest.kt Adds focused unit regression tests for TOID-based lookup and “no matching op” skip behavior.
platform/src/test/kotlin/org/stellar/anchor/platform/observer/stellar/StellarRpcObserverIndexDomainTest.kt Adds scheduler-driven domain tests validating end-to-end crediting/skipping behavior across mixed batches.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

amandagonsalves and others added 3 commits June 15, 2026 16:35
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* refactor error message into multiple lines for improved readability
@travertischio
travertischio force-pushed the develop branch 2 times, most recently from f84f3ae to 2bbb21a Compare June 23, 2026 15:43
@amandagonsalves
amandagonsalves force-pushed the fix/rpc-observer-op-index-domain branch 2 times, most recently from e84a53a to 2765f36 Compare June 23, 2026 16:36
@amandagonsalves
amandagonsalves merged commit 2ca9a4c into develop Jun 23, 2026
17 of 21 checks passed
@amandagonsalves
amandagonsalves deleted the fix/rpc-observer-op-index-domain branch June 23, 2026 20:15
amandagonsalves added a commit that referenced this pull request Jul 1, 2026
### Description

Merges release/4.5.0 into main for the 4.5.0 release.

### Context

#1957
#1962
#1960
#1954
#1953
#1955

### Testing

- `./gradlew test`

### Documentation
N/A

### Known limitations
N/A
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.

3 participants