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-6, 24, 31: add transaction status for discontinued transactions #1109

Closed
JakeUrban opened this issue Jan 13, 2022 · 9 comments
Closed

SEP-6, 24, 31: add transaction status for discontinued transactions #1109

JakeUrban opened this issue Jan 13, 2022 · 9 comments
Assignees
Labels
SEP Represents an issue that requires a SEP.

Comments

@JakeUrban
Copy link
Contributor

What problem does your feature solve?

Anchors may want to communicate to wallet application that specific in-progress transactions are no longer valid. For example, if a transaction is in pending_user_transfer_start for over X minutes, the anchor may want to consider this transaction abandoned by the user.

What would you like to see?

All SEPs that process transactions (6, 24, & 31), should have a status value that indicates to client-side applications that the transaction cannot be continued.

Status valid candidates:

  • canceled
  • invalid
  • discontinued
  • abondoned

cc @lijamie98

@JakeUrban JakeUrban self-assigned this Jan 13, 2022
@JakeUrban JakeUrban added the SEP Represents an issue that requires a SEP. label Jan 13, 2022
@lijamie98
Copy link
Contributor

lijamie98 commented Jan 27, 2022

In the MGI meeting on 1/27/2022, MGI wish to show something like expired. I think we may add expired.

For invalid, when does that happen?

@JakeUrban
Copy link
Contributor Author

The values I listed are just suggestions, we could use expired instead 👍

I can see a future where we have both expired and canceled though, since they indicate two different things. For now lets roll with expired.

@lijamie98
Copy link
Contributor

Yes, that looks good.

In your original list, I thought you are going to add more of the statuses to SEP24. Should we add the others as well?

@JakeUrban
Copy link
Contributor Author

The "status value candidates" list was just ideas for the name, I didn't mean to indicate that we should add all those as distinct status values.

@JakeUrban
Copy link
Contributor Author

cc @accordeiro

We're currently considering two new statuses:

  • expired: indicates that the user did not complete the interactive flow or did not deliver funds to the anchor within a timeframe defined by the anchor
  • canceled: indicates that funds were delivered but the transaction was canceled for some reason. In this case, the refunded attribute (i.e. not a status value) should be true.

It sounds like we agree on expired. Regarding cancelled, there are some alternative suggestions:

  • just use completed, but mark refunded as true. No need for an additional status.
    • The primary concern here is that the client needs to take 2 attributes into account.
  • make refunded a status value and deprecate the refunded attribute.

@accordeiro
Copy link
Member

I think we should make refunded a first-class status, since combining a refunded boolean flag with other statuses is confusing (like what does it mean to have refunded=true and status=pending_anchor)?

So I like the plan of adding the following statuses:

  • expired
  • cancelled
  • refunded

And deprecating the boolean flag. Although I'm not sure how that flag is used in the wild, if at all.

@accordeiro
Copy link
Member

I think we should think about this problem and #1122 together, since it touches the refund process.

@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 Mar 12, 2022
@JakeUrban JakeUrban reopened this Jun 8, 2022
@JakeUrban
Copy link
Contributor Author

Reopening this because #1222 did not address this issue. We added the refunded status value, but not the expired status.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SEP Represents an issue that requires a SEP.
Projects
None yet
Development

No branches or pull requests

3 participants