SEP-24: Withdraw transaction refund handling.

[lijamie98]

It seems Plan-E is the clear consensus.

Plan E: Add extra field refund

GET /transaction?id=xxx
{
    "id": "xxx",
    "kind": "withdrawal",
    "amount_in": 110, # amount of original transaction.
    ...,
    "status": "completed",
    // original stellar transaction ID
    stellar_transaction_id: "b9d0b2292c4e09e8eb22d036171491e87b8d2086bf8b265874c8d182cb9c9020",
    "refunds": {
        "type": "partial",
        "amount_refunded": "10",
        "transactions": [
            {
                "amount": 10,  # amount refunded in stellar network
                "stellar_transaction_id": "b9d0b2292c4e09e8eb22d036171491e87b8d2086bf8b265874c8d182cb9c9020"
            },
            {
                "amount": 15, # amount refunded in external network, if applicable.
                "external_transaction_id": "54321ab047a193c6fda1c47f5962cbcca8708d79b87089ababd57532c21c5402"
            }
        ]
    }
}

---

What problem does your feature solve?

In the withdraw process, the user may transfer the USDC to the anchor. However, the anchor may need to refund the transaction due to technical or business reasons.

In that scenario, there are two Stellar network transactions but only one SEP-24 transaction. The wallet won't be able to see the "anchor-initiated" refund transactions.

What would you like to see?

The wallet should be able to see both the withdraw transaction and the "reverse" transaction.

The goal of this ticket is to choose one of the three alternatives (others are welcome) to propose a change for the refund process.

What alternatives are there?

There are three plans:

Plan A: Create a new value: refund ore reverse of the kind field.

In this plan, a new possible value, refund will be added to SEP-24. A new transaction schema will be added.

Pros:

• New schema does not impact the existing withdraw and deposit transactions without overloading the meanings of the existing fields.
• First class citizen. Easier to maintain in the long run
• The wallet will be able to link multiple refund transactions with the existing withdraw or deposit transaction.

Cons:

• Impact to the existing wallet to interpret the refund kind.

The impact may be limited because for existing wallets working with existing anchor, the new refund kind has no impact. For wallets working with this new anchor, we can quickly implement.

Plan B: Create a separate deposit transaction

The anchor can create a separate deposit transaction with the same amount of the withdraw transaction. We may overload the refunded` field to indicate a refund transaction.

or use the memo field to indicate if this is a refund.

Pros:

• Least impact to existing wallets.

Cons:

• The user may see deposit transaction not initiated from the wallet.
• The wallet won't be able to link both transactions together.

Plan C: Create a separate deposit transaction with memo

The anchor may create a separate deposit transaction with a memo that links to the withdraw transaction.

Pros:

• Least impact to wallets.

Cons:

• Overloading the memo field may have impact with other functions.
• It is yet another protocol to between the anchor and the wallet to interpret the memo to link both transactions.

Plan D: Add extra field in the existing transaction schema

Add an extra field such as sub_transactions or refund_transactions to the Transaction schema to subsequent transactions related to the original one.

Example:

GET /transaction?id=xxx

{
    "id": "xxx",
    "kind": "withdrawal",
    "amount_in": 110,
    ...,
    status: "completed",
    // original stellar transaction ID
    stellar_transaction_id: "b9d0b2292c4e09e8eb22d036171491e87b8d2086bf8b265874c8d182cb9c9020",
    sub_transactions: [
        {
            "type": "refund",
            "total_refunded": "10",
            // refund transaction ID
            "transaction_id": "54321ab047a193c6fda1c47f5962cbcca8708d79b87089ababd57532c21c5402"
        },
       {
            "type": "future_type...",
           ...,
        }
       ]
}

Pros

• Non-breaking
• Can attach multiple subsequent transactions to the original one.

Cons

• TBD (cannot think of one at this moment)

Plan E: Add extra field refund

GET /transaction?id=xxx

{
    "id": "xxx",
    "kind": "withdrawal",
    "amount_in": 110, # amount of original transaction.
    ...,
    "status": "completed",
    // original stellar transaction ID
    stellar_transaction_id: "b9d0b2292c4e09e8eb22d036171491e87b8d2086bf8b265874c8d182cb9c9020",
    "refunds": {
        "type": "partial",
        "amount_refunded": "25",
        "transactions": [
            {
                "amount": 10,  # amount refunded in stellar network
                "stellar_transaction_id": "b9d0b2292c4e09e8eb22d036171491e87b8d2086bf8b265874c8d182cb9c9020"
            },
            {
                "amount": 15, # amount refunded in external network, if applicable.
                "external_transaction_id": "54321ab047a193c6fda1c47f5962cbcca8708d79b87089ababd57532c21c5402"
            }
        ]
    }
}

[accordeiro]

@JakeUrban @leighmcculloch @CassioMG would love your thoughts here too.

[CassioMG]

Plan A: "The wallet will be able to link multiple refund transactions with the existing withdraw or deposit transaction."

@lijamie98 Just for clarity, how would the wallet link this refund transaction with other existing withdraw/deposit transactions?

[lijamie98]

@lijamie98 Just for clarity, how would the wallet link this refund transaction with other existing withdraw/deposit transactions?

Yes, we may add a linked_transaction_id or original_transaction_id field so that the wallet can easily link them together.

[JakeUrban]

I think creating a new transaction record that is visible on the GET /transaction(s) endpoints is the wrong approach, so I would not vote for any of the options outlined in the description.

1. Refund payments made on the Stellar network are not the same as transactions initiated by client applications. Introducing a new object schema for these types of payments on an existing endpoint is a breaking change that wallets may not be able to handle.
2. From an API design perspective, we want to minimize the variance of the objects returned in API responses. And from a RESTful API design perspective, a refund payment is a different "resource" than the existing transaction resource.

Instead, I think we should try to update the transaction record schema to include the necessary information on the refund payments made in association with it. What do you think about the following schema change?

GET /transaction?id=xxx

{
    "id": "xxx",
    "kind": "withdrawal",
    "amount_in": 110,
    ...,
    status: "completed",
    refund: {
        "type": "partial",
        "total_refunded": "10",
        "transaction_ids": [
            // for withdraw transactions, these are stellar transaction hashes
            // for deposit transactions, these are off-chain identifiers
            "b9d0b2292c4e09e8eb22d036171491e87b8d2086bf8b265874c8d182cb9c9020",
            "54321ab047a193c6fda1c47f5962cbcca8708d79b87089ababd57532c21c5402"
        ]
    }
}

[accordeiro]

Interesting @JakeUrban, I like the non-disruptive approach. Do you see a problem with also including the refund object in the (all) /transactions endpoint in your proposal?

[accordeiro]

I'd imagine most wallets that don't support this would simply ignore the attribute. But of course this could cause some breakage - which is also true even if we only alter the /transaction?id=<id> endpoint

[JakeUrban]

The refund sub-object should be included in all transaction records (from both GET /transaction and GET /transactions), since the record schema is the same for those endpoints today.

I actually don't think this will cause any breakage because the refund sub-object is additive. We do already have a refund boolean attribute unfortunately, so maybe we should use a different name such as refund_info. Then we would just deprecate the refund boolean attribute?

[lijamie98]

I can guess there may be transactions other than refund that may be attached to the original transaction. We may consider these as sub-transactions attached to the original one as this:

GET /transaction?id=xxx

{
    "id": "xxx",
    "kind": "withdrawal",
    "amount_in": 110,
    ...,
    status: "completed",
    // original stellar transaction ID
    stellar_transaction_id: "b9d0b2292c4e09e8eb22d036171491e87b8d2086bf8b265874c8d182cb9c9020",
    sub_transactions: [
        {
            "type": "refund",
            "total_refunded": "10",
            // refund transaction ID
            "transaction_id": "54321ab047a193c6fda1c47f5962cbcca8708d79b87089ababd57532c21c5402"
        },
       {
            "type": "future_type...",
           ...,
        }
       ]
}

[lijamie98]

Adding Plan D in the issue description.

[JakeUrban]

I like the idea of generalizing related transactions.

Another type of sub-transaction is the transaction needed to create the account if it doesn't exist.

[CassioMG]

I can guess there may be transactions other than refund that may be attached to the original transaction. We may consider these as sub-transactions attached to the original one as this:

GET /transaction?id=xxx

{
    "id": "xxx",
    "kind": "withdrawal",
    "amount_in": 110,
    ...,
    status: "completed",
    // original stellar transaction ID
    stellar_transaction_id: "b9d0b2292c4e09e8eb22d036171491e87b8d2086bf8b265874c8d182cb9c9020",
    sub_transactions: [
        {
            "type": "refund",
            "total_refunded": "10",
            // refund transaction ID
            "transaction_id": "54321ab047a193c6fda1c47f5962cbcca8708d79b87089ababd57532c21c5402"
        },
       {
            "type": "future_type...",
           ...,
        }
       ]
}

@lijamie98 this looks good! In this sample you posted, should the wallet expect a Sep-24 transaction to be found and returned by the Anchor once fetching GET /transaction?stellar_transaction_id= 54321ab047a193c6fda1c47f5962cbcca8708d79b87089ababd57532c21c5402 ?

[lijamie98]

@CassioMG That's a good point. I would vote for a YES. Any concerns?
@leighmcculloch @marcelosalloum @accordeiro @JakeUrban

[leighmcculloch]

I prefer the refund transactions that are explicitly denoted as such, outlined in #1122 (comment). I think that is easier for an integrating developer to understand. It also gives us some freedom since the refund resource can contain refund specific fields, as is demonstrated where the example @JakeUrban shared has a total amount refunded explicitly shown.

What type of sub-transactions do we anticipate there being? And would it be clearer to make those their own resources as they are encountered?

[lijamie98]

I prefer the refund transactions that are explicitly denoted as such, outlined in #1122 (comment). I think that is easier for an integrating developer to understand. It also gives us some freedom since the refund resource can contain refund specific fields, as is demonstrated where the example @JakeUrban shared has a total amount refunded explicitly shown.

What type of sub-transactions do we anticipate there being? And would it be clearer to make those their own resources as they are encountered?

I think the type: field is consistent with the design of the SEP-24 transaction schema.

In addition, it will be awkward if we have multiple refunds with different amount.

The type of sub-transactions are unknown at this moment. It serves as a placeholder for future types.

[leighmcculloch]

One thing that I think is confusing about the sub-transactions design is that the total_refunded is inside a single transaction. Is that total refunded for all refunds, or just a single tx? Would a developer just need to iterate over the list to find transactions that are refunds and add the values together?

[accordeiro]

The balance here is between creating something generic and futureproof, and creating something incremental and simple that addresses the concern.

I'm not particularly a fan of introducing a new concept of nested sub-transactions, because this creates a new dimension of complexity. What are sub-transactions? What are the criteria to define whether one transaction is related to another? Do sub-transactions have sub-sub-transactions? What is the guidance we'll give to UIs consuming this API?

The problem we're trying to solve is around refunds, which is a well-understood problem: there's a {0,1}-1 mapping to a transaction, and it has known properties which can be different from top-level transactions. I don't have a strong opinion against having a flat transaction list and introducing new kinds of transactions, but I see @JakeUrban 's points on #1122 (comment)

So I guess I'm leaning towards @JakeUrban's suggestion from here: #1122 (comment)

// Transaction Structure

{
    "id": "xxx",
    "kind": "withdrawal",
    "amount_in": 110,
    ...,
    "status": "completed",
    "refund_info": {
        "type": "partial",
        "total_refunded": "10",
        "transaction_ids": [
            // for withdraw transactions, these are stellar transaction hashes
            // for deposit transactions, these are off-chain identifiers
            "b9d0b2292c4e09e8eb22d036171491e87b8d2086bf8b265874c8d182cb9c9020",
            "54321ab047a193c6fda1c47f5962cbcca8708d79b87089ababd57532c21c5402"
        ]
    }
}

This design doesn't prevent us from creating new kinds of top-level transactions in the future, and IMO is the easiest to parse in the frontend and provide a good UX. The trade-off here is that this might be more challenging for anchors to implement than a simple flat list, so I'd love some feedback from implementers here.

[leighmcculloch]

+1

I think we can also tweak the refunds object a little so that over time we can add more fields if needed, by making the list of transaction_ids a list of transactions with an id field, where other fields can be added if needed, (since SEP-24 offers no way to query additional details about things like this separately).

{
    "id": "xxx",
    "kind": "withdrawal",
    "amount_in": 110,
    ...,
    "status": "completed",
    "refunds": {
        "type": "partial",
        "amount_refunded": "10",
        "transactions": [
            {
                "id": "b9d0b2292c4e09e8eb22d036171491e87b8d2086bf8b265874c8d182cb9c9020"
            },
            {
                "id": "54321ab047a193c6fda1c47f5962cbcca8708d79b87089ababd57532c21c5402"
            }
        ]
    }
}

[accordeiro]

Would we be able to hit /transactions?id=xxx using the nested transaction ids (like /transaction?id= b9d0b2292c4e09e8eb22d036171491e87b8d2086bf8b265874c8d182cb9c9020) on @leighmcculloch's example above? Or would that return a 404? If the latter, we should probably use a different name like "refund_tx_ids" so it doesn't seem like we're referring to the same transaction top level resource.

[leighmcculloch]

According to @JakeUrban's earlier comment the IDs are Stellar transaction hashes or external off-chain identifiers.

// for withdraw transactions, these are stellar transaction hashes
// for deposit transactions, these are off-chain identifiers

[accordeiro]

So we could have:

{
    "id": "xxx",
    "kind": "withdrawal",
    "amount_in": 110,
    ...,
    "status": "completed",
    "refunds": {
        "type": "partial",
        "amount_refunded": "10",
        "transactions": [
            {
                "stellar_id": "b9d0b2292c4e09e8eb22d036171491e87b8d2086bf8b265874c8d182cb9c9020"
            },
            {
                "external_id": "54321ab047a193c6fda1c47f5962cbcca8708d79b87089ababd57532c21c5402"
            }
        ]
    }
}

So we know there is no pure id associated with them?

[CassioMG]

This is exactly what I was about to ask, and the answer is somewhat still unclear to me. I'll try exemplifying for clarification.

If I'm understanding correctly in this example that @JakeUrban posted the user completed a withdrawal of amount 110 but then the Anchor partially refunded it with an amount of 10, which appears as a payment transaction (of amount 10) on stellar network. Is that correct?

So, let's say the wallet gets this stellar_tx_id_123 from this payment transaction of amount 10 (through horizon) and then try fetching more details from Anchor through GET /transaction?stellar_transaction_id=stellar_tx_id_123.

What should be the expected response here? Should the Anchor return the same Sep-24 transaction object as if the wallet were fetching the original withdrawal transaction?

I think it'd be great to get the same transaction object since the wallet could easily iterate over refunds.transactions to check if stellar_tx_id_123 is there to discover if it's one of the refunds tx or the original one. Or even simply compare if stellar_tx_id_123 is different than the original "id" ("xxx" in the example).

Does that make sense?

[lijamie98]

Updated the description by adding Plan E.

When I first read about SEP, my felt traces of incremental changes and that sometimes confused me when I read it. That's why I kind of leaning towards a future proof design. But have something that's future proof can be disruptive and may have interest conflict with incremental design.

Plan A, Plan D and Plan E represents three different directions. I don't have strong opinions on which one is better because they have pros and cons.

@CassioMG I think people are leaning towards plan E. Once we decide on one of the plans, we can deep dive and confirm. I also added some comments. Hopefully that makes sense to you to calculate the final amount.

[marcelosalloum]

I'm leaning towards plan E as well.

---

About how to retrieve the transaction using a refund transaction id, it makes more sense to me if we do one of the following:

1. Add a new field just for refund IDs, like: GET /transaction?refund_tx_id={id}.
2. Add a new field that searches through all kinds of IDs in a transaction, something like GET /transaction?contains_id={id}. This format would be explicitly generic and work for the transaction main ID, as well as external_transaction_id, stellar_transaction_id, and the refund IDs.

The reason I don't like to overload the ?stellar_transaction_id={id} as @CassioMG suggested is because we would have 2 different types of stellar transactions, the initial and the refund ones, and that could be non-intuitive as per the current usage of SEP-24 and its description. Also, what if the refund is not a stellar transaction but an external one? The wallet would need to figure that out in advance and then choose between sending a request with ?stellar_transaction_id={id} or ?external_transaction_id={id}.

[marcelosalloum]

The question I'd like to answer first is how the wallet uses the GET /transaction?key=val endpoint? Where does the tx ID come from? When the wallet gets the tx ID, does it already know it's internal/external? Does it already know it's a refund?

If the wallet is uncertain about the transaction being internal/external, then we shouldn't overload the query parameters ?stellar_transaction_id={id} or ?external_transaction_id={id}.

If the wallet is not certain about it being a refund, then we shouldn't go with GET /transaction?refund_tx_id={id} either.

Thoughts?

[JakeUrban]

I vote for plan E. The sub-transactions idea is interesting but I'd rather design for the known use case rather than generalize prematurely.

I agree with @marcelosalloum that we should not overload the stellar_transaction_id or external_transaction_id GET /transaction parameters.

I personally like the refund_tx_id parameter idea, but see Marcelo's point about the wallet not knowing whether or not it is a refund. The contains_id parameter argument is interesting, although it may require a heavy query or set of queries for the anchor who effectively has to search across multiple dimensions.

Before we choose to standardize something like GET /transaction?contains_id=, are we sure that wallet applications will need to fetch transaction records based on the refund ID? If I'm a wallet and I'm rendering a transaction history view, I'd think something like this is the most intuitive:

1. transaction A
2. transaction B (refunded)
3. transaction C
...

The user can then click on transaction B to get the refund info.

Are we thinking the wallet will do something like this? Showing the refund transaction as a distinct transaction?

1. transaction A
2. transaction B
3. transaction B refund
4. transaction C

[leighmcculloch]

are we sure that wallet applications will need to fetch transaction records based on the refund ID?

This is a good point. Unclear. I think if we can limit the refund transactions to living inside the refunds.transactions list and not be a top-level transaction, that might keep the use simpler? If we say they are queryable via the /transactions endpoint, shouldn't they also show up in lists? That seems problematic for backwards compatibility.

[JakeUrban]

I agree @leighmcculloch, my preference would be to not support fetching transaction records by a refund ID.

[CassioMG]

I vote for plan E. The sub-transactions idea is interesting but I'd rather design for the known use case rather than generalize prematurely.

I agree with @marcelosalloum that we should not overload the stellar_transaction_id or external_transaction_id GET /transaction parameters.

I personally like the refund_tx_id parameter idea, but see Marcelo's point about the wallet not knowing whether or not it is a refund. The contains_id parameter argument is interesting, although it may require a heavy query or set of queries for the anchor who effectively has to search across multiple dimensions.

Before we choose to standardize something like GET /transaction?contains_id=, are we sure that wallet applications will need to fetch transaction records based on the refund ID? If I'm a wallet and I'm rendering a transaction history view, I'd think something like this is the most intuitive:

1. transaction A
2. transaction B (refunded)
3. transaction C
...

The user can then click on transaction B to get the refund info.

Are we thinking the wallet will do something like this? Showing the refund transaction as a distinct transaction?

1. transaction A
2. transaction B
3. transaction B refund
4. transaction C

I have this scenario in mind: let's say a stellar wallet app has a History section in which it'd like to display all transactions that happened for this user in the stellar network. In this case the refund transaction would appear like the below since by the end of the day it's a new payment transaction from the Anchor to the user:

1. transaction A
2. transaction B
3. transaction B (*refund)
4. transaction C

In case we can't use the refunded tx id to GET /transaction to display more info about this refund transaction to the user then the wallet would have to make some heavy lifting on the frontend to fetch all transactions (GET /transactions) from each Anchor to search across all of Sep-24 transactions to discover if any of those stellar transactions is a refund transaction.

I have 2 suggestions in which we could split this heavy lifting between Anchor & Wallet:

A) - Allow GET /transaction?refund_tx_id= (instead of the heavy contains_id=) so that it could work like this for wallets:

1. Wallet displays all stellar network transactions listed in History page
2. User taps on a History item
3. Wallet tries fetching more info using GET /transaction?stellar_transaction_id=
4. In case 3. gets a 404 then wallet could retry using GET /transaction?refund_transaction_id=
5. In case 4. succeeds then this means it's a refund transaction and wallet is able to show more info
   ^ Making the user await for a retry doesn't seem like a big issue in this case since refunds should rarely happen

B) - Allow something like GET /refunds or GET /transactions?refunds_only=true so that wallets could easily fetch all refund transactions from an Anchor beforehand in order to detect if a stellar network payment is a refund tx or not when showing it on History page (again, since refunds should rarely happen this fetching should return a list of only a few refund transactions which could be quickly mapped on the frontend)

I also like plan E, I think the biggest question is whether Anchors should support directly querying refund transactions or not. I think they should somehow provide that, specially considering a refund could happen in any point in time (like it could happen the day after, the month after, or the year after) so wallets won't be sure in which timeframe they should be looking at considering the scenario I mentioned.

How does that sound?

[JakeUrban]

let's say a stellar wallet app has a History section in which it'd like to display all transactions that happened for this user in the stellar network

🤔 I see your point. If you're using the Stellar Network as your source of data (transaction IDs), then you wouldn't be able to quickly look up the SEP-related transaction data from the anchor. This is assuming that you know that the particular transaction ID on the stellar network relates to a specific anchor's SEP service though.

I'm not against a refund_tx_id parameter, but unless we have a concrete use case (i.e. Vibrant) that wants this functionality, I would suggest that we wait and see what the demand is from the ecosystem.

[CassioMG]

🤔 I see your point. If you're using the Stellar Network as your source of data (transaction IDs), then you wouldn't be able to quickly look up the SEP-related transaction data from the anchor. This is assuming that you know that the particular transaction ID on the stellar network relates to a specific anchor's SEP service though.

You are right, the wallet wouldn't need to fetch all transactions from all anchors but instead all transactions related to this specific Anchor which sent this payment.

I'm not against a refund_tx_id parameter, but unless we have a concrete use case (i.e. Vibrant) that wants this functionality, I would suggest that we wait and see what the demand is from the ecosystem.

Sounds good, maybe before going deeper into this we could check with the Anchor how they feel about supporting a refund_tx_id parameter. What do you think?

[JakeUrban]

Sounds like a plan to me. I'll start drafting the PR 👍

[lijamie98]

Sounds like a plan to me. I'll start drafting the PR 👍

Thanks @JakeUrban !