Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ANCHOR-514] Populate sep38quote info into sep24txn #1194

Merged
merged 12 commits into from
Nov 27, 2023

Conversation

JiahuiWho
Copy link
Contributor

Description

If quote_id is provided upon receiving the sep24 request, fetch quote details and populate it into sep24txn object

Left out the quote validation since it's specified in doc

When quote_id is set, asset_code, source_asset and amount must be validated by the anchor, if present. In case of a conflict with the ones used to create the [SEP-38] quote, this request should be rejected with a 400.

Context

Add quote_id and source_asset_id to sep24 txn so that quotes can be displayed to users while initiate deposit/withdraw sep24 transaction

Testing

  • ./gradlew test

Documentation

Updated in stellar/stellar-protocol#1358

@JiahuiWho JiahuiWho changed the title [ANCHOR 514] Populate sep38quote info into sep24txn [ANCHOR-514] Populate sep38quote info into sep24txn Nov 7, 2023
@JiahuiWho JiahuiWho marked this pull request as ready for review November 8, 2023 19:09
@JakeUrban
Copy link
Contributor

Why we aren't performing validation against the quote's attributes the the requests attributes. If I initiate a SEP-24 deposit transaction and specify an asset_code that doesn't match the specified quote's buy asset, the anchor platform should return 400.

@philipliu
Copy link
Contributor

I think we would also need to update the SEP-38 implementation. Only SEP-6, and SEP-24 are supported currently https://github.com/stellar/java-stellar-anchor-sdk/blob/8e595eab7aa01147d610cb39313afa0c18d75630/api-schema/src/main/java/org/stellar/anchor/api/sep/sep38/Sep38Context.java#L6-L10.

@lijamie98 lijamie98 requested a review from philipliu November 10, 2023 02:10
@@ -236,6 +236,18 @@ public interface Sep24Transaction extends SepTransaction {

String getMessage();

String getQuoteId();
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

The docs would need to be updated to include quote_id under the SEP-24 transaction as well. https://developers.stellar.org/api/anchor-platform/resources/get-transaction

@@ -555,4 +572,74 @@ public InfoResponse getInfo() {
sep24Config.getFeatures().getClaimableBalances()))
.build();
}

private void validatedAndPopulateQuote(
Copy link
Contributor

Choose a reason for hiding this comment

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

I wrote a utility class that does this validation for SEP-6. Do you think we can leverage it here? I think the logic should be the same. https://github.com/stellar/java-stellar-anchor-sdk/blob/8e595eab7aa01147d610cb39313afa0c18d75630/core/src/main/java/org/stellar/anchor/sep6/ExchangeAmountsCalculator.java#L31-L62

Copy link
Collaborator

@lijamie98 lijamie98 left a comment

Choose a reason for hiding this comment

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

LGTM.

Please feel free to merge after the comments are taken care of.

@JiahuiWho JiahuiWho merged commit 4c94a70 into stellar:develop Nov 27, 2023
7 checks passed
@JiahuiWho JiahuiWho deleted the anchor-514 branch December 5, 2023 00:18
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.

4 participants