Transfer SEPS: deprecate refunded boolean, add refunds object to transaction records

[JakeUrban]

resolves #1122

Deprecates the refunded attribute included in objects returned from GET /transaction(s) endpoints for SEP-6, 24, & 31 in favor of adding a refund object.

This refund object contains information about the type of refund, total amount refunded, and the individual payments made back to the sender / user.

Note that SEP-31 refunds are assumed to be made via Stellar (since funds are always delivered to receiving anchors via Stellar).

[leighmcculloch]

👏🏻 Thanks for progressing this discussion to a PR. I have started a few conversations inline.

[leighmcculloch]

Looks good to me. I think we should get weigh in from a variety of folks before merging.

[marcelosalloum]

It's looking good! Could you add a changelog section to these SEPs as (recently) suggested in the SEP template? https://github.com/stellar/stellar-protocol/blob/master/sep-template.md#changelog.

No need to include previous versions there, tracking this and future changes is great to start with.

[marcelosalloum]

It's common for exchanges to charge a refund fee in case something goes wrong with the transaction. Shouldn't we include some kind of fee field as well?

[marcelosalloum]

For Stellar network payments, the anchor could add 2 transactions, one for the refund and the other for the refund fee, but that may not be practical for bank transfers.

[JakeUrban]

Sure, I can add a fee attribute. Shouldn't we have a total_fee attribute or something similar alongside amount_refunded then?

[leighmcculloch]

Is the idea with the fee attribute that this would represented an amount that the user no longer has a right to, but was not included in the refund transaction? How would this affect the amount refunded?

For example:

• Original deposit $100.
• Anchor needs to refund $25, charges $5 per refund.
• Amount refunded = $25? or $20?
• Refunds includes a single refund with id and amount.
• What value does the refunds.payments[0].amount field hold? Is it $25, or $20?

[JakeUrban]

My understanding is that it would look like this:

• Original deposit $100
• Anchor needs to refund $25, and charges $5 per refund
• Lets say that anchor charges a $5 flat fee for transactions (regardless of refund)
• 100 (amount in) - 25 (refund) - 5 (tx fee) - 5 (refund fee) = 65 (amount out)

So:

{
  ...
  "amount_in": "100",
  "amount_out": "65",
  "amount_fee": "5",
  "refunded": false,
  "refunds": {
    "amount_refunded": "25",
    "total_fee": "5",
    "payments": [
      {
        "id": "<stellar hash>"
        "id_type": "stellar",
        "amount": "25",
        "fee": "5"
      }
    ]
  }
}

[marcelosalloum]

That makes sense to me too, since it's consistent with the amount_in description:

amount_in	string	(optional) Amount received by anchor at start of transaction as a string with up to 7 decimals. Excludes any fees charged before the anchor received the funds.

[leighmcculloch]

Got it, so is it correct that this formula should balance to zero?

amount_in
– amount_out
– amount_fee
– refunds.amount_refunded
– refunds.total_fee

And:

refunds.amount_refunded = sum(refunds.payments[].amount
refunds.total_fee = sum(refunds.payments[].fee)

refunds.total_fee should be named refunds.amount_fee I think for consistency with other amount fields.

[JakeUrban]

Yes thats right. In fact I think those formulas are worth including in the specifications.

[leighmcculloch]

Amount received by anchor at start of transaction as a string with up to 7 decimals. Excludes any fees charged before the anchor received the funds.

The excludes statement is ambiguous to me. When I read it I can't tell if it means the amount_in has already had fees excluded, i.e. I'm depositing $100, but amount_in will be $95 because it excludes the fee I paid.

[JakeUrban]

I agree, the amount_in definitions statement about "fees charged before the anchor received the funds" is nonsensical. I'd rather not remove that text in this PR though. I think the formulas make it clear, which I've added.

[CassioMG]

@JakeUrban revisiting this just for clarification: does it mean the user is paying for the fees related to the refunds?

[JakeUrban]

Yes, if the anchor charges fees for processing a refund.

[CassioMG]

@JakeUrban FYI the PR title still says "refund" instead of "refunds"

[CassioMG]

@JakeUrban FYI the PR title still says "refund" instead of "refunds"

Same for the PR description