Skip to content

[ANCHOR-1302]: Observe destination accounts in SEP6/24 - #2010

Merged
amandagonsalves merged 5 commits into
developfrom
fix/anchor-1302
Sep 9, 2026
Merged

amandagonsalves merged 5 commits into
developfrom
fix/anchor-1302

Conversation

@amandagonsalves

Copy link
Copy Markdown
Collaborator

Description

RequestOnchainFundsHandler.updateTransactionWithRpcRequest assigns a toAccount for the withdrawal/receive leg of every protocol, but only the SEP-31 branch tells the Payment Observer to watch that account — it calls paymentObservingAccountsManager.upsert(txn31.getToAccount(), TRANSIENT) unconditionally, after either the custom-destination path or the auto-generated-deposit-info path. The SEP-6 and SEP-24 branches set toAccount/withdrawAnchorAccount the same way but never call upsert.

In practice this is masked whenever the deposit info generator hands back the anchor's own distribution account, since PaymentObserverBeans registers every configured distribution account as RESIDENTIAL at startup — already-watched, so the missing upsert is a no-op in that case. The gap only surfaces when request_onchain_funds is called with the "none" deposit-info generator and an RPC-supplied destination_account that isn't the anchor's distribution account: nothing ever registers that address with the observer, so an incoming payment to it is never matched back to the transaction.

Changes

  • RequestOnchainFundsHandler.updateTransactionWithRpcRequest: after txn6.setToAccount(...), calls paymentObservingAccountsManager.upsert(txn6.getToAccount(), TRANSIENT); after txn24.setToAccount(...), calls the equivalent for txn24. Placed unconditionally after both branches (custom destination and auto-generated), mirroring the existing SEP-31 call.
  • RequestOnchainFundsHandlerTest: added verify(exactly = 1) { paymentObservingAccountsManager.upsert(DESTINATION_ACCOUNT, TRANSIENT) } to the existing SEP-24 (test_handle_ok_sep24_withExpectedAmount) and SEP-6 (test_handle_sep6_ok_withoutAmountExpected) tests that already exercise a custom destination_account.

Acceptance Criteria

  • A SEP-6 request_onchain_funds call with the "none" generator and an explicit destination_account registers that account with the Payment Observer as TRANSIENT.
  • A SEP-24 request_onchain_funds call with the "none" generator and an explicit destination_account registers that account with the Payment Observer as TRANSIENT.
  • SEP-31 behavior is unchanged.
  • All existing RequestOnchainFundsHandlerTest tests pass.

Context

N/A

Testing

  • Unit: ./gradlew :platform:test --tests "org.stellar.anchor.platform.rpc.RequestOnchainFundsHandlerTest"
  • Integration: ./gradlew :core:test :platform:test — full module regression, no failures

Documentation

N/A

Known limitations

N/A

…ment observer

* request_onchain_funds now upserts the destination account as
  transient for sep-6 and sep-24, matching existing sep-31 behavior

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

Registers SEP-6 and SEP-24 destination accounts with the Payment Observer.

Changes:

  • Adds transient observation for SEP-6/24 destinations.
  • Adds verification for explicit destination accounts.
  • Critical: Registration occurs before transaction persistence, allowing payments to be permanently missed.
  • Moderate: Muxed destinations are registered non-canonically and cannot be matched.
  • Moderate: SEP-6/24-only deployments do not run transient-account cleanup.

Reviewed changes

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

File Description
platform/src/test/kotlin/org/stellar/anchor/platform/rpc/RequestOnchainFundsHandlerTest.kt Verifies SEP-6/24 registration calls.
platform/src/main/java/org/stellar/anchor/platform/rpc/RequestOnchainFundsHandler.java Registers SEP-6/24 destination accounts for observation.
Suppressed comments (2)

platform/src/main/java/org/stellar/anchor/platform/rpc/RequestOnchainFundsHandler.java:331

  • This has the same muxed-account mismatch for SEP-24: upsert stores the M... address, while observer lookups normalize muxed destinations to their base G... address. Consequently, registering a muxed destination here does not make its payments observable. Normalize accounts centrally in PaymentObservingAccountsManager.upsert and add a muxed SEP-24 test.
        paymentObservingAccountsManager.upsert(
            txn24.getToAccount(), PaymentObservingAccountsManager.AccountType.TRANSIENT);

platform/src/main/java/org/stellar/anchor/platform/rpc/RequestOnchainFundsHandler.java:331

  • The SEP-24 account is exposed to the observer before the transaction is saved in pending_user_transfer_start (RpcTransactionStatusHandler.java:180-206). If a payment lands during this interval, the listener cannot match it by status and the observer still advances its cursor, so that payment is not reconsidered. Move registration after successful persistence or introduce retry handling for unmatched observed payments.
        paymentObservingAccountsManager.upsert(
            txn24.getToAccount(), PaymentObservingAccountsManager.AccountType.TRANSIENT);

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…muxed keys

* request_onchain_funds now registers the observed account after the
  transaction save completes instead of before, for sep-6/24/31 alike
* payment observing accounts manager canonicalizes muxed accounts to
  their base account on write, matching existing read-side behavior
* eviction scheduler now starts when sep-6 or sep-24 is enabled, not
  just sep-31
Copilot AI review requested due to automatic review settings September 9, 2026 18:22

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

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

* refactor persistence operations to handle failures gracefully

* add logging for store operation exceptions

* update upsert to canonicalize accounts and delete stale entries

* add tests for resilient persistence and legacy account cleanup
Copilot AI review requested due to automatic review settings September 9, 2026 19:18

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

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

* update account merging logic to preserve newer 'last observed' timestamps

* fix legacy muxed account deletion to ensure persistence on canonical write failures

* add tests for updated account management behaviors
Copilot AI review requested due to automatic review settings September 9, 2026 20:00

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

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

platform/src/main/java/org/stellar/anchor/platform/rpc/RequestOnchainFundsHandler.java:368

  • The acceptance criteria say SEP-31 behavior is unchanged, but this branch moves its registration from updateTransactionWithRpcRequest to a hook that runs after txn31Store.save. That reverses the existing ordering and leaves an interval where the transaction is ready for payment but its non-residential destination is not observed; the observers advance their cursor past unobserved payments. Keep SEP-31 registration in its existing location and limit the new behavior to SEP-6/24, or coordinate the state change and registration so this gap cannot lose an event.
      case SEP_31:
        toAccount = ((JdbcSep31Transaction) txn).getToAccount();
        break;

* refactor upsert and updateLastObservedTime methods to use atomic map operations

* add a concurrent test to validate thread-safe account updates

* add a noop payment observing account store for testing purposes
Copilot AI review requested due to automatic review settings September 9, 2026 20:12

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

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

platform/src/main/java/org/stellar/anchor/platform/rpc/RequestOnchainFundsHandler.java:356

  • This no longer leaves SEP-31 behavior unchanged: the old registration ran during updateTransactionWithRpcRequest, before txn31Store.save, while this hook runs afterward. That creates a new interval where the observer will discard a payment to the destination because the account is not yet registered, and streamed events are not replayed after registration. Please preserve the established SEP-31 ordering (and apply a deliberate, race-safe ordering to SEP-6/24) rather than moving all registrations behind the save.
  @Override
  protected void afterTransactionSaved(JdbcSepTransaction txn, RequestOnchainFundsRequest request)

@amandagonsalves
amandagonsalves merged commit 3c53f94 into develop Sep 9, 2026
12 checks passed
@amandagonsalves
amandagonsalves deleted the fix/anchor-1302 branch September 9, 2026 22:37
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