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

SEP-24: add or update fee endpoint including FX rates #1273

Closed
JakeUrban opened this issue Jul 25, 2022 · 19 comments · Fixed by #1358
Closed

SEP-24: add or update fee endpoint including FX rates #1273

JakeUrban opened this issue Jul 25, 2022 · 19 comments · Fixed by #1358
Assignees

Comments

@JakeUrban
Copy link
Contributor

JakeUrban commented Jul 25, 2022

What problem does your feature solve?

SEP-24's GET /fee endpoint assumes the off-chain asset is exchanges 1-for-1 with the on-chain asset. But it is increasingly common for anchors to accept off-chain assets that are not exchanged 1-for-1 with the on-chain assets they support.

What would you like to see?

We either need to update the existing GET /fee endpoint or add a new endpoint for requesting the FX rate + any fees.

The endpoint we design will likely look very similar to SEP-38's GET /price or GET /prices (or we could add endpoints for both).

What alternatives are there?

We could recommend that SEP-24 anchors implement SEP-38, since the use case is so similar.

However, implementing the entire SEP-38 API may be overkill in the context of SEP-24, since users can commit to FX rates in the anchor's UI, which means there is no reason for SEP-24 anchors to expose the SEP-38 POST /quote endpoint.

@JakeUrban JakeUrban self-assigned this Jul 25, 2022
@leighmcculloch
Copy link
Member

I think this is reasonable extension to SEP-24.

I agree implementing /quote may be overkill, unless the UI hopes to display more accurate estimate of an amount before sending the user down the UI. At the least we can leave /quote out on a first pass and introduce it if the need for it arises.

Do we see more use of /price or /prices for SEP-38?

Should we deprecate /fee in time?

@JakeUrban
Copy link
Contributor Author

Do we see more use of /price or /prices for SEP-38?

I think so. We've had multiple wallets express desire for the functionality offered by these endpoints, primarily for monitoring FX rates out-of-band of the transaction initiation flow. For example, sending notifications to users when rates are favorable, or displaying an anchor's live rates on the wallet's website to entice downloads of their app.

Should we deprecate /fee in time?

I think so (again). The GET /price endpoint from SEP-38 can do everything SEP-24's GET /fee endpoint does, so we don't lose existing functionality, and GET /fee nowadays only works for a increasingly small subset of anchors.

@leighmcculloch
Copy link
Member

Do we see more use of /price or /prices for SEP-38?

I think so. We've had multiple wallets express desire for the functionality offered by these endpoints

I was meaning do we see folks primarily using /price or /prices? Or do folks really need both?

@JakeUrban
Copy link
Contributor Author

They serve different use cases.

GET /prices would likely be used most outside transaction flows for monitoring prices.

But GET /price includes information on any additional fees charged for processing transactions on top of the FX rates, so that endpoint would be used when the user is looking at a particular pair and about to initiate a transaction.

My inclination is that GET /prices would be used more in the context of SEP-24 anchors, since additional fees can be displayed in the anchor's UI when users initiate a transaction, but from the wallet's perspective I'm sure they would prefer to have both endpoints available.

@leighmcculloch
Copy link
Member

Sounds reasonable to me to add both now.

@github-actions
Copy link

This issue is stale because it has been open for 30 days with no activity. It will be closed in 30 days unless the stale label is removed.

@github-actions github-actions bot added the stale label Aug 24, 2022
@JakeUrban JakeUrban removed the stale label Aug 24, 2022
@github-actions
Copy link

This issue is stale because it has been open for 30 days with no activity. It will be closed in 30 days unless the stale label is removed.

@github-actions github-actions bot added the stale label Sep 24, 2022
@JakeUrban JakeUrban removed the stale label Sep 26, 2022
@github-actions
Copy link

This issue is stale because it has been open for 30 days with no activity. It will be closed in 30 days unless the stale label is removed.

@github-actions github-actions bot added the stale label Oct 26, 2022
@JakeUrban JakeUrban removed the stale label Oct 26, 2022
@github-actions
Copy link

This issue is stale because it has been open for 30 days with no activity. It will be closed in 30 days unless the stale label is removed.

@github-actions github-actions bot added the stale label Nov 26, 2022
@JakeUrban JakeUrban removed the stale label Nov 26, 2022
@github-actions
Copy link

This issue is stale because it has been open for 30 days with no activity. It will be closed in 30 days unless the stale label is removed.

@github-actions github-actions bot added the stale label Dec 27, 2022
@JakeUrban JakeUrban removed the stale label Jan 3, 2023
@github-actions
Copy link

github-actions bot commented Feb 3, 2023

This issue is stale because it has been open for 30 days with no activity. It will be closed in 30 days unless the stale label is removed.

@github-actions github-actions bot added the stale label Feb 3, 2023
@JakeUrban JakeUrban removed the stale label Feb 3, 2023
@github-actions
Copy link

github-actions bot commented Mar 6, 2023

This issue is stale because it has been open for 30 days with no activity. It will be closed in 30 days unless the stale label is removed.

@github-actions github-actions bot added the stale label Mar 6, 2023
@JakeUrban JakeUrban removed the stale label Mar 6, 2023
@github-actions
Copy link

github-actions bot commented Apr 6, 2023

This issue is stale because it has been open for 30 days with no activity. It will be closed in 30 days unless the stale label is removed.

@github-actions github-actions bot added the stale label Apr 6, 2023
@JakeUrban JakeUrban removed the stale label Apr 6, 2023
@JakeUrban
Copy link
Contributor Author

@Ifropc assigning you here.

@github-actions
Copy link

github-actions bot commented May 7, 2023

This issue is stale because it has been open for 30 days with no activity. It will be closed in 30 days unless the stale label is removed.

@github-actions github-actions bot added the stale label May 7, 2023
@Ifropc Ifropc removed the stale label May 8, 2023
@Ifropc Ifropc linked a pull request Jun 8, 2023 that will close this issue
@github-actions
Copy link

github-actions bot commented Jun 8, 2023

This issue is stale because it has been open for 30 days with no activity. It will be closed in 30 days unless the stale label is removed.

@github-actions github-actions bot added the stale label Jun 8, 2023
@JakeUrban
Copy link
Contributor Author

@Ifropc can you link the PR you have to this issue?

@Ifropc
Copy link
Contributor

Ifropc commented Jun 8, 2023

image
Has already been linked

@github-actions github-actions bot removed the stale label Jun 9, 2023
@github-actions
Copy link

This issue is stale because it has been open for 30 days with no activity. It will be closed in 30 days unless the stale label is removed.

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 a pull request may close this issue.

4 participants
@leighmcculloch @JakeUrban @Ifropc and others