SEP-38: add `context` request parameter and `fee` & `total_price` response attributes #1204

marcelosalloum · 2022-05-17T00:36:40Z

What

Add the ability for SEP-38 quotes to detail the quote fees under the fee response field, and calculate the fees and price based on the input parameter context:

context is a mandatory request field and can be one of sep6 or sep31.
total_price is a new response field that describes the conversion rate with the fees included.
price will describe the conversion rate without including fees.

The GET /quote/:id response will look like this:

{
  "id": "de762cda-a193-4961-861e-57b31fed6eb3",
  "expires_at": "2021-04-30T07:42:23",
  "total_price": "5.42",        // 542/100
  "price": "5.00",              // (542-42)/100
  "sell_asset": "iso4217:BRL",
  "sell_amount": "542",
  "buy_asset": "stellar:USDC:GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN&",
  "buy_amount": "100",
  "fee": {
    "total": "42.00",
    "asset": "iso4217:BRL",
    "details": [
      {
        "name": "PIX fee",
        "description": "Fee charged in order to process the outgoing PIX transaction.",
        "amount": "12.00"
      },
      {
        "name": "Service fee",
        "amount": "30.00"
      }
    ]
  }
}

Also, updated the description of SEP-6 and SEP-31 transactions amount_fee and amount_fee_details, stating that it should be consistent with SEP-38 fee_details when provided.

Why

Comparing the previous design with partners that have a long history in the currency conversion business we found out that it's pretty common to use not only margin but also additive/inclusive fees.

That's why we decided to change the SEP design, so we can be compliant with these businesses.

Regarding the inclusion of "total_price", this was introduced so the conversion price and fees can be displayed to the user regardless of the remittance platform using additive or inclusive fees. For instance:

Moneygram uses additive fees, so they use price only:
wise.com uses inclusive fees, so they use total_price:

…` request parameters, so anchors can calculate the quote taking into consideration what they will be used for. ([#????](https://github.com/stellar/stellar-protocol/pull/????))

…ters, so Anchors can detail the margin costs.

…unt_fee` and `amount_fee_asset` fields when a `quote_id` is used.

…nt_fee` and `amount_fee_asset` fields when a `quote_id` is used.

ecosystem/sep-0038.md

JakeUrban · 2022-05-18T21:50:34Z

@marcelosalloum I forgot to add a top-level comment so I'll do that here. Overall I think this is the right approach, although it seemed to be limiting the fees to only using the margin. I thought we had agreed to support additive / inclusive fees too, but I'm happy to discuss further.

marcelosalloum · 2022-05-18T21:59:49Z

@marcelosalloum I forgot to add a top-level comment so I'll do that here. Overall I think this is the right approach, although it seemed to be limiting the fees to only using the margin. I thought we had agreed to support additive / inclusive fees too, but I'm happy to discuss further.

Since you're suggesting to support additive fees as part of SEP-38, should we enforce that SEP-31 amounts should be consistent with SEP-38's? For instance, sep31.amount_in == sep38.sell_amount (when quotes are used).

…/fee mandatory for /price and /quote.

JakeUrban

I have one requested adjustment but otherwise this looks ready!

ecosystem/sep-0031.md

…eeds to be true only when `quote_id` is used.

JakeUrban

I've asked @leighmcculloch to take a look, but if he is busy we can merge.

leighmcculloch · 2022-05-23T18:36:59Z

This PR doesn't communicate why this change is being made. Could you add that context? I'm specifically interested why applications need a price and an effective_price.

leighmcculloch · 2022-05-23T18:49:09Z

price will describe the conversion rate without including fees.

Does this statement refer only to the fees that get added on, and not fees built into any rates?

marcelosalloum · 2022-05-23T18:58:36Z

@leighmcculloch:

This PR doesn't communicate why this change is being made. Could you add that context? I'm specifically interested why applications need a price and an effective_price.

Good point! I've just updated the PR description.

--

price will describe the conversion rate without including fees.

Does this statement refer only to the fees that get added on, and not fees built into any rates?

Yes, only refers to fees that appear under the "fee" response parameter.

leighmcculloch · 2022-05-23T20:12:46Z

Is "effective price" the best name we can give the price-inclusive-of-fees concept? A quick internet search revealed there is another concept named "effective exchange rate" which has different meaning. If there's not a better name sgtm, but I also don't see the effective on either of the screenshots, so maybe there's a better name we can use that will align with the existing money transfer industry?

marcelosalloum · 2022-05-23T20:52:17Z

Is "effective price" the best name we can give the price-inclusive-of-fees concept? A quick internet search revealed there is another concept named "effective exchange rate" which has different meaning. If there's not a better name sgtm, but I also don't see the effective on either of the screenshots, so maybe there's a better name we can use that will align with the existing money transfer industry?

there's something called "out-the-door price" that has a similar meaning but is only used in the car selling business. I didn't really find a well known concept for this, so the options I have in mind are:

effective_price
out_the_door_price
final_price
total_price
customer_price

None of them seems great though, I'd continue with effective_price unless you guys think another option is clearly better.

JakeUrban · 2022-05-23T21:00:48Z

The only other decent option we have (in my opinion) is price_including_fee or price_including_fees. I don't have a preference here.

The important thing is that this attribute communicates the discrepancy between price and the value you get when you calculate the rate using sell_amount and buy_amount.

leighmcculloch · 2022-05-23T21:09:25Z

I would lean total price, given that the term total is used in at least one of the screenshots above. Effective has a non-specific sense to it for me, where-as the price being described is a concrete value. But I don't feel strongly, and in light of all the options effective also seems fine too.

…tellar-protocol#1204.

…-protocol#1204 (#285) ### What Update platform code to be compliant with the protocol change added in stellar/stellar-protocol#1204 & stellar/stellar-anchor-platform#32. Changes in the SEP: * SEP-38 `GET /price` and `POST /quote` requests now need the new parameter `context=sep6|sep31`. * SEP-38 `GET /price` and `GET|POST /quote` responses now return the new parameters `total_price` and `fee`. Changes in the Platform->Anchor event schema: * The `QuoteEvent` schema contains 4 new parameters: `"fee"`, `"total_price"`, `"send_amount"` and `"buy_amount"` parameters. Changes in the Platform->Anchor `GET /rate` callback: * Update types from `["indicative", "firm"]` to `["indicative_prices", "indicative_price", "firm"]`. * Add the request parameter `"context=sep6|sep31"`, that should be mandatory for `["firm", "indicative_price"]` types. * Add `"fee"`, `"total_price"`, `"send_amount"` and `"buy_amount"` response parameters. ### Why Close #270, addressing the changes from stellar/stellar-protocol#1204 & stellar/stellar-anchor-platform#32.

### What Update SEP-38 tests based on stellar/stellar-protocol#1204. The changes include: * SEP-38 `GET /price` and `POST /quote` now require the mandatory `context` request parameter. * SEP-38 `GET /price` and `GET|POST /quote` now return the mandatory response parameters `total_price` and `fee`. * The updated formulas from [SEP38#price-formulas](https://github.com/stellar/stellar-protocol/blob/faa99165050dcd44a9e0f700c3d019258d8b4321/ecosystem/sep-0038.md#price-formulas) are now being validated.

marcelosalloum added 3 commits May 16, 2022 16:46

Update links to pull requests.

e359c8c

SEP-38: add parameter context to the POST /quote and `GET /prices…

a63ffc5

…` request parameters, so anchors can calculate the quote taking into consideration what they will be used for. ([#????](https://github.com/stellar/stellar-protocol/pull/????))

Add optional field fee_details to prices and quotes response parame…

2d93cd8

…ters, so Anchors can detail the margin costs.

marcelosalloum self-assigned this May 17, 2022

marcelosalloum mentioned this pull request May 17, 2022

SEP-38 + SEP-31 + SEP-6: update how SEP-38 displays its fees and how its amounts interact with SEP-31 and SEP-6 amounts #1199

Closed

marcelosalloum added 5 commits May 17, 2022 11:26

Update link to PR.

3a1856d

Polish SEP-38 changes.

c79720a

Polish SEP-31.

23b4bea

App information about how a SEP-31 transaction will populate the `amo…

240b9e4

…unt_fee` and `amount_fee_asset` fields when a `quote_id` is used.

App information about how a SEP-6 transaction will populate the `amou…

bfc687d

…nt_fee` and `amount_fee_asset` fields when a `quote_id` is used.

marcelosalloum changed the title ~~[WIP] SEP-38: add context and fee_details~~ SEP-38: add context request parameter and fee_details response parameter May 17, 2022

marcelosalloum marked this pull request as ready for review May 17, 2022 20:48

marcelosalloum requested review from JakeUrban, leighmcculloch, lijamie98, reecexlm and stfung77 May 17, 2022 20:48

lijamie98 reviewed May 18, 2022

View reviewed changes

ecosystem/sep-0038.md Outdated Show resolved Hide resolved

ecosystem/sep-0038.md Outdated Show resolved Hide resolved

ecosystem/sep-0038.md Outdated Show resolved Hide resolved

marcelosalloum added 2 commits May 18, 2022 10:31

Update fee_details.name description.

9098dcc

Update fee_details.value to fee_details.amount.

4b1f4e4

JakeUrban reviewed May 18, 2022

View reviewed changes

ecosystem/sep-0038.md Outdated Show resolved Hide resolved

ecosystem/sep-0038.md Outdated Show resolved Hide resolved

ecosystem/sep-0038.md Outdated Show resolved Hide resolved

marcelosalloum added 2 commits May 18, 2022 17:40

Remove context and fee_details from GET /prices.

854b1d0

Renamed fee_details to fee, update the object schema and made context…

85b7bb0

…/fee mandatory for /price and /quote.

marcelosalloum marked this pull request as draft May 19, 2022 00:51

marcelosalloum added 2 commits May 19, 2022 13:07

Add relation between amounts when quotes are used.

2e53866

Polishes.

243c2f7

marcelosalloum force-pushed the sep38-context-and-fee-details branch from 5423002 to 243c2f7 Compare May 19, 2022 20:44

marcelosalloum marked this pull request as ready for review May 19, 2022 20:44

marcelosalloum requested review from JakeUrban and removed request for JakeUrban and reecexlm May 20, 2022 21:55

JakeUrban changed the title ~~SEP-38: add context request parameter and fee_details & effective_price response parameters~~ SEP-38: add context request parameter and fee & effective_price response parameters May 23, 2022

JakeUrban changed the title ~~SEP-38: add context request parameter and fee & effective_price response parameters~~ SEP-38: add context request parameter and fee & effective_price response attributes May 23, 2022

JakeUrban reviewed May 23, 2022

View reviewed changes

ecosystem/sep-0031.md Outdated Show resolved Hide resolved

marcelosalloum added 2 commits May 23, 2022 10:31

Make it clear in SEP-31 that request.amount == response.amount_in n…

147aff2

…eeds to be true only when `quote_id` is used.

Merge branch 'master' into sep38-context-and-fee-details

5f757dd

marcelosalloum requested a review from JakeUrban May 23, 2022 17:33

JakeUrban previously approved these changes May 23, 2022

View reviewed changes

Merge branch 'master' into sep38-context-and-fee-details

9bd2b58

Replace effective_price with total_price.

edee126

marcelosalloum dismissed JakeUrban’s stale review via edee126 May 23, 2022 21:39

JakeUrban approved these changes May 23, 2022

View reviewed changes

JakeUrban changed the title ~~SEP-38: add context request parameter and fee & effective_price response attributes~~ SEP-38: add context request parameter and fee & total_price response attributes May 23, 2022

marcelosalloum merged commit faa9916 into master May 23, 2022

marcelosalloum deleted the sep38-context-and-fee-details branch May 23, 2022 21:46

marcelosalloum mentioned this pull request May 23, 2022

Update SEP-38 documentation by adding the details field stellar/anchor-platform#231

Closed

marcelosalloum added a commit to stellar/anchor-platform that referenced this pull request May 25, 2022

Start updating platform and reference server according with stellar/s…

a67749d

…tellar-protocol#1204.

This was referenced May 26, 2022

SEP-38: update platform and reference server based on stellar/stellar-protocol#1204 stellar/anchor-platform#285

Merged

SEP-38: update SEP-38 tests based on stellar/stellar-protocol#1204 stellar/stellar-anchor-tests#86

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SEP-38: add `context` request parameter and `fee` & `total_price` response attributes #1204

SEP-38: add `context` request parameter and `fee` & `total_price` response attributes #1204

marcelosalloum commented May 17, 2022 •

edited

Loading

JakeUrban commented May 18, 2022

marcelosalloum commented May 18, 2022

JakeUrban left a comment

JakeUrban left a comment

leighmcculloch commented May 23, 2022

leighmcculloch commented May 23, 2022

marcelosalloum commented May 23, 2022

leighmcculloch commented May 23, 2022

marcelosalloum commented May 23, 2022

JakeUrban commented May 23, 2022

leighmcculloch commented May 23, 2022 •

edited

Loading

SEP-38: add context request parameter and fee & total_price response attributes #1204

SEP-38: add context request parameter and fee & total_price response attributes #1204

Conversation

marcelosalloum commented May 17, 2022 • edited Loading

What

Why

JakeUrban commented May 18, 2022

marcelosalloum commented May 18, 2022

JakeUrban left a comment

Choose a reason for hiding this comment

JakeUrban left a comment

Choose a reason for hiding this comment

leighmcculloch commented May 23, 2022

leighmcculloch commented May 23, 2022

marcelosalloum commented May 23, 2022

leighmcculloch commented May 23, 2022

marcelosalloum commented May 23, 2022

JakeUrban commented May 23, 2022

leighmcculloch commented May 23, 2022 • edited Loading

SEP-38: add `context` request parameter and `fee` & `total_price` response attributes #1204

SEP-38: add `context` request parameter and `fee` & `total_price` response attributes #1204

marcelosalloum commented May 17, 2022 •

edited

Loading

leighmcculloch commented May 23, 2022 •

edited

Loading