[ANCHOR-1226]: SEP-6 deposit/withdraw-exchange skips the quote buy-asset check SEP-24 enforces, binding a quote to a mismatched destination asset#1969
Merged
Conversation
* update calculatefromquote method signature to include buy asset for validation * update sep6service to pass buy asset to quote calculation * add tests for buy asset validation in quote calculation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request closes a validation gap in SEP-6 deposit/withdraw-exchange flows by ensuring a provided SEP-38 quote is validated against the destination (buy) asset, aligning SEP-6 behavior with SEP-24 and preventing quotes from being bound to mismatched destination assets.
Changes:
- Updated
ExchangeAmountsCalculator.calculateFromQuote(...)to accept abuyAssetand validate it against the quote’sbuy_asset. - Updated SEP-6 deposit-exchange and withdraw-exchange services to pass the resolved destination asset as
buyAsset. - Expanded/updated unit tests to cover mismatched vs matching buy-asset behavior and verify SEP-6 passes the correct buy asset.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| core/src/main/java/org/stellar/anchor/util/ExchangeAmountsCalculator.java | Adds buyAsset parameter to calculateFromQuote and validates it against the quote’s buy asset. |
| core/src/main/java/org/stellar/anchor/sep6/Sep6Service.java | Passes resolved destination asset into quote-based amount calculation for deposit/withdraw exchange. |
| core/src/test/kotlin/org/stellar/anchor/util/ExchangeAmountsCalculatorTest.kt | Updates call sites for new signature and adds tests for buy-asset mismatch/match behavior. |
| core/src/test/kotlin/org/stellar/anchor/sep6/Sep6ServiceTest.kt | Updates mocks for new signature and adds tests asserting SEP-6 passes the correct buy asset. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
28
to
31
| * @param quoteId The quote ID | ||
| * @param sellAsset The asset the user is selling | ||
| * @param buyAsset The asset the user is buying (the request's destination asset) | ||
| * @param sellAmount The amount the user is selling |
JiahuiWho
approved these changes
Jul 10, 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
SEP-6
deposit-exchange/withdraw-exchangenever checked that a SEP-38 quote's buy asset matched the request'sdestination_asset.ExchangeAmountsCalculator.calculateFromQuote— the only entry point SEP-6 uses — hardcoded the buy-asset argument tonullwhen callingvalidateQuoteAgainstRequestInfo, skipping the buy-asset-mismatch check that method already implements (and that SEP-24 already relies on via a different entry point). A user could bind a quote priced for a cheap asset to a transaction recorded against a different, valuabledestination_asset, causing the operator to lose the difference at settlement.Changes
ExchangeAmountsCalculator.calculateFromQuote: signature changed to acceptbuyAsset, forwarded intovalidateQuoteAgainstRequestInfoinstead ofnull. No new overload — the 3-arg version was unconditionally unsafe and had exactly two callers, both already fixed below.Sep6Service.depositExchange/withdrawExchange: both now pass their already-resolved destination/buy asset into the call.ExchangeAmountsCalculatorTest: updated all 8 existing calls for the new signature; added tests proving the fixed method rejects a mismatched buy asset and accepts a matching one.Sep6ServiceTest: updated all 8 mock stubs for the new arity; added two tests (deposit-exchange, withdraw-exchange) capturing thecalculateFromQuotecall and asserting the resolved destination asset is what's actually passed asbuyAsset.Acceptance Criteria
deposit-exchange/withdraw-exchangereject a quote whose buy asset doesn't matchdestination_asset.destination_assetsucceeds unchanged.calculateFromQuoteexists or broke (only the twoSep6Servicecall sites in production code).Context
#3815607
Testing
./gradlew :core:test --tests "org.stellar.anchor.util.ExchangeAmountsCalculatorTest"./gradlew :core:test --tests "org.stellar.anchor.sep6.Sep6ServiceTest"./gradlew :core:buildDocumentation
N/A
Known limitations
None.