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: Add support for asynchronous deposit instructions #1379

Merged
merged 17 commits into from
Aug 30, 2023

Conversation

philipliu
Copy link
Contributor

@philipliu philipliu commented Aug 14, 2023

Proposal

This adds new fields to the transaction object to facilitate Anchors providing deposit instructions outside of GET /deposit response.

Backwards Compatability

This change is backward compatible as it does not introduce new required fields if Anchors can provide deposit instructions in the GET /deposit response.

Resolves #1372, #1368.

@philipliu philipliu force-pushed the anchor-386-deposit branch 5 times, most recently from 09eb55a to 16212a2 Compare August 15, 2023 01:38
@philipliu philipliu marked this pull request as ready for review August 15, 2023 19:53
@philipliu philipliu requested review from JakeUrban and Ifropc August 15, 2023 19:54
@philipliu philipliu self-assigned this Aug 15, 2023
ecosystem/sep-0006.md Outdated Show resolved Hide resolved
ecosystem/sep-0006.md Outdated Show resolved Hide resolved
@philipliu philipliu requested a review from JakeUrban August 16, 2023 16:02
ecosystem/sep-0006.md Outdated Show resolved Hide resolved
@philipliu philipliu requested a review from JakeUrban August 16, 2023 18:19
Copy link
Contributor

@JakeUrban JakeUrban left a comment

Choose a reason for hiding this comment

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

Should be the last batch of comments!

ecosystem/sep-0006.md Outdated Show resolved Hide resolved
ecosystem/sep-0006.md Outdated Show resolved Hide resolved
ecosystem/sep-0006.md Outdated Show resolved Hide resolved
ecosystem/sep-0006.md Outdated Show resolved Hide resolved
@philipliu
Copy link
Contributor Author

I appreciate the thorough review @JakeUrban! I believe I've addressed your comments now.

JakeUrban
JakeUrban previously approved these changes Aug 16, 2023
Copy link
Contributor

@JakeUrban JakeUrban left a comment

Choose a reason for hiding this comment

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

Ok, I think this is ready! Can we move forward with the implementation while we let this bake for a bit and allow the community to comment if they have feedback? I've let MyKobo and Beans know.

ecosystem/sep-0006.md Outdated Show resolved Hide resolved
ecosystem/sep-0006.md Outdated Show resolved Hide resolved
JakeUrban
JakeUrban previously approved these changes Aug 16, 2023
@marcinx
Copy link
Contributor

marcinx commented Aug 17, 2023

Good improvement. how was indeed a bit non-normalized and different anchors were using it differently. We should probably keep it around alongside instructions for a good while though.

@ydag
Copy link
Contributor

ydag commented Aug 28, 2023

Great update! Enhances UX significantly!

@philipliu philipliu merged commit d0ef9fb into stellar:master Aug 30, 2023
@philipliu philipliu deleted the anchor-386-deposit branch August 30, 2023 19:26
philipliu added a commit that referenced this pull request Aug 30, 2023
### Abstract
Today, wallets are required to send a user's financial account
information mainly through the `dest` and `dest_extra` request
parameters when requesting a withdrawal. This is a security risk
as web servers often log their GET requests which will include
personally identifiable information such as a user's bank account
number. The standard should define an alternative method for
allowing users to provide their information.

### Proposal
This PR proposes excluding the `fields` from the withdrawal types
returned by the `GET /info` endpoint as a means to collect these
fields through an alternative flow. This will force the anchor to
put the transaction into the `pending_customer_info_update` status.
The wallet will use then SEP-12 `PUT /customer` to provide the
missing financial account information. This allows us to deprecate
the `dest` and `dest_extra` request parameters by making them
optional when a withdraw type's `fields` object is missing.

### Backwards Compatibility
This change is backward compatible as wallets will continue sending
financial account information through the request parameters if the
anchors define them as part of the `GET /info` response.

This depends on some changes from
#1379.
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.

SEP-6: Providing deposit instructions asynchronously
5 participants