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

Add SEP-38 support to SEP-24 #1358

Merged
merged 20 commits into from
Jul 17, 2023
Merged

Add SEP-38 support to SEP-24 #1358

merged 20 commits into from
Jul 17, 2023

Conversation

Ifropc
Copy link
Contributor

@Ifropc Ifropc commented Jun 8, 2023

Abstract

Currently, there is no way for a wallet to get a quote from the anchor in a standardized way. Anchors may have in-house endpoints for the wallet, however, wallet would need to implement a new set of API for each anchor it integrates with.
What's more, current fee endpoint is not flexible enough, and doesn't offer complex functionality SEP-38 fee (part of the quote object body) has.
Having a quote and fees before proceeding to the interactive screen is very important for the user experience, as without it, user would not be able to know this information before initiating a transaction.

Proposal

This proposal is aimed to solve this problem. We add integration of the existing SEP-38 endpoint with the SEP-24. The wallet may now use the existing SEP-38 endpoints implemented by the anchor to query a quote.
Later, wallet may use the quote id (for the firm quote) to exchange the asset on agreed rates.

Changelog summary

Add support for the SEP-38 API. Add optional quote_id parameter to post /transactions/deposit/interactive
and POST /transactions/deposit/interactive requests. Add quote_id to the transaction object schema.

Use cases

  1. Wallet can query an indicative quote from the anchor
  2. Anchor can support more complex fee logic via SEP-38 /price and /prices endpoints
  3. Wallet can request a firm quote and initiate a transaction with this quote.

Backward compatibility

This proposal is fully backward compatible

@Ifropc Ifropc requested a review from JakeUrban June 8, 2023 00:41
@Ifropc Ifropc linked an issue Jun 8, 2023 that may be closed by this pull request
@Ifropc Ifropc marked this pull request as draft June 8, 2023 17:47
@Ifropc Ifropc marked this pull request as ready for review June 8, 2023 22:12
@Ifropc Ifropc marked this pull request as draft June 8, 2023 22:17
Copy link
Contributor

@JakeUrban JakeUrban left a comment

Choose a reason for hiding this comment

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

This is a good start!

I don't see any changes to GET /info though, we wanted to make changes there right?

I think the only other change requested is to explore adding a parameter for pre-populating the off-chain asset, although its not strictly necessary since we have a UI.

ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
@Ifropc Ifropc marked this pull request as ready for review June 12, 2023 22:30
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Show resolved Hide resolved
ecosystem/sep-0024.md Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
@leighmcculloch leighmcculloch removed their request for review June 15, 2023 16:42
Copy link
Contributor

@CassioMG CassioMG left a comment

Choose a reason for hiding this comment

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

Lgtm

JakeUrban
JakeUrban previously approved these changes Jul 17, 2023
Copy link
Contributor

@JakeUrban JakeUrban left a comment

Choose a reason for hiding this comment

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

Approved with some comments

ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
ecosystem/sep-0024.md Outdated Show resolved Hide resolved
JakeUrban
JakeUrban previously approved these changes Jul 17, 2023
Comment on lines 975 to 976
by `SEP-38 POST /quote` and `SEP-38 POST /prices` API. If set to yes, anchor must support `quote_id` parameter in
interactive endpoints and transaction response body. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a merge typo?


## Fee

### This endpoint is deprecated. The [SEP-38] `GET /price` endpoint should be used to fetch fees instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use **bold** markdown here rather than a header.

@Ifropc Ifropc merged commit c8a9120 into master Jul 17, 2023
2 checks passed
@JakeUrban JakeUrban deleted the 1273 branch August 21, 2023 20:18
JiahuiWho added a commit to stellar/java-stellar-anchor-sdk that referenced this pull request Nov 27, 2023
### 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
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.

SEP-24: add or update fee endpoint including FX rates
4 participants