SEP-6: Accept financial information via SEP-12

[philipliu]

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.

[philipliu]

I'm skipping a version here as I assume that the deposit PR will be merged first.

[JakeUrban]

I definitely agree with the outcome we're trying to achieve here, but I think we should encourage clients to provide financial account data via SEP-12's PUT /customer requests, rather than SEP-6's PATCH /transaction request.

I think this simplifies client implementations (and the design of the anchor platform) because we don't have to support PATCH /transaction or the pending_transaction_info_update status. What do you think?

[philipliu]

The challenge with using SEP-12 for accepting financial account information is when a user wants to use two different bank accounts (or two accounts of the same type) between withdrawals.

Let's suppose a user requests a withdrawal to the first bank account. Then, they submit another withdrawal request to another bank account. I see two ways of doing this:

1. Associate a set of bank account fields per transaction. Something like bank_account_number.e0045691-23a7-4f9e-8c56-aeaf3a78010e, for each transaction. When the transaction completes, we would need to delete these bank account fields. SEP-12 does not currently support deleting a subset of fields for a customer.
2. Block the second withdrawal until the first completes, then ask the user to submit new bank account information again using SEP-12. This would not allow for a user not to have concurrent withdrawals.

I think multi-account withdrawals are a use case we want to support. SEP-12 does not look like it's designed for ephemeral/transaction-related data, so using it would require us to design a partial delete API. I agree that using /PATCH is not ideal, but I feel like it's a simpler approach.

[JakeUrban]

I'm not sure I understand how this approach solves the multiple-account-per-type-per-customer problem though.

If a first-time customer initiates a withdraw to bank account and omits dest and dest_extra, the anchor can move to pending_transaction_info_update and request the customer to provide bank details via PATCH /transaction, but the 2nd time they initiate a withdrawal transaction, how does the anchor know the customer wants to use a different bank account this time?

[philipliu]

/PATCH transaction would only update the bank details for a single transaction.

The second withdrawal request would be moved into pending_transaction_info_update and the customer would be asked to provide bank details again using PATCH /transaction.

[JakeUrban]

If the business doesn't store bank details of their customers for future transactions, then that puts responsibility on the wallet to do so, otherwise wallets will need to ask their users for the their bank account info on each transaction. And I don't know if it makes sense to have wallets store customer bank details, especially noncustodial wallets.

While its not ideal, the existing approach of having at most 1 account per type on file works and ensures that customers only have to provide their bank details once. If customers want to use a different bank account, they can update their account on file with the anchor, and the wallet can decide how to build the UX for that. But I'd rather not try to solve a problem a small percentage of customers will use at the expense of additional complexity for wallets or a worse UX for the majority of customers who just use one off-chain account.

[philipliu]

Thanks, @JakeUrban, that makes sense. I'll make the changes then.

[Ifropc]

Can you update Basic Wallet Implementation section to reflect this change?