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

Add withdrawal support for notify_refund_pending RPC method #1353

Merged
merged 8 commits into from
May 8, 2024

Conversation

Ifropc
Copy link
Contributor

@Ifropc Ifropc commented May 7, 2024

Description

Add support for withdrawals for existing notify_refund_pending RPC method

Context

I noticed refunds are not available for pending_user_transfer_complete and pending_external statuses.

Generally speaking, in the regular flow transaction goes from pending_user_transfer_start -> pending_anchor -> completed
However, when the anchor takes times to process a transaction and it's still in pending_anchor , user wants to refund it and can do so.
In the cases where status is pending_user_transfer_complete (funds are available for pickup) or pending_external (funds are processed by external entity) we want to fist move it to pending_anchor to indicate that funds are not available for pickup anymore, (or that bank transfer was cancelled), i.e. indicate that refund has been initiated.
In this case, if we have issues with Stellar transfer or Circle, etc. transaction won't be in status that shows user wrong information. (Especially in pending_user_transfer_complete case, where user funds is technically no longer available for pickup anymore, but status doesn't show that)
After that, we can use existing notify_refund_send RPC.

Testing

  • ./gradlew test
  • Added new test cases

Documentation

Flow charts should be updated (https://stellarorg.atlassian.net/browse/ANCHOR-688)

Known limitations

N/A

Copy link

github-actions bot commented May 7, 2024

Code Coverage

Overall Project NaN% NaN% 🍏

There is no coverage information present for the Files changed

Copy link
Contributor

@philipliu philipliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good although we should update the Platform API tests to cover the new flows.

Copy link
Collaborator

@lijamie98 lijamie98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with some minor comments.

@@ -27,7 +27,8 @@ import org.stellar.anchor.platform.AbstractIntegrationTests
import org.stellar.anchor.platform.TestConfig
import org.stellar.anchor.util.GsonUtils

@Disabled
@Disabled // TODO: https://stellarorg.atlassian.net/browse/ANCHOR-693
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we put a little details here so that we don't need to go back to the ticket?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, let's do this little TODOs later on to not forget things that were put away!

@Ifropc Ifropc merged commit 7d3f6c1 into develop May 8, 2024
8 checks passed
@Ifropc Ifropc deleted the ANCHOR-685-refunds branch May 8, 2024 21:20
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 this pull request may close these issues.

None yet

3 participants