Split dip 8 to funds pull pre approval and charge + auth/capture #2

danprinz · 2021-03-29T21:05:40Z

No description provided.

Disclaimer regarding the discovery phase Extended scope for fund pull pre-approval Revoke an authorization in more details

Co-authored-by: kphfb <kph@fb.com>

davidiw

Coming along well.

I'm going to leave some crazy ideas here for now and we can setup some time to chat:

Move as much to the interaction between Web services as possible to speed up UX and to make it less Diem off-chain dependent
Don't reinvent off-chain protocol, create a new payment command that leverages all the existing fields but results in an auth instead of a payment -- it might be kyc, but we can effectively shoehorn in kyb -- we will work (together) on a better off-chain and make that ideal -- but I don't want to add too much (more) complexity now
Use off-chain to trigger capture
Define an on-chain extension for registering a URL for a small logo to represent a VASP on a merchant website -- an parallel resource to the DiemId resource defined in Dip-10 -- but completely VASP managed -- this would also be where the VASP registers if login -- which I didn't actually see mentioned here

davidiw · 2021-03-31T04:50:36Z

dips/dip-8.md

-Version 0 of the Off-Chain Protocol is described in [DIP 1](https://dip.diem.com/dip-1/).  Version 1 as described here is an extension of the Off-Chain Protocol and adds features to support more advanced functionality - particularly targeted to support merchant use-cases.  This is inclusive of pull payments, recurring payments, and auth/capture flows.
+Versions 1/2 of the Off-Chain Protocol are described in [DIP 1](https://dip.diem.com/dip-1/). 
+
+Version 3 as described here is an extension of the Off-Chain Protocol and adds features to support more advanced functionality - particularly targeted to support merchant use-cases. 


Don't think we need a new version here, this should be a different command, otherwise we have to consider version negotiation, which I'm not convinced is worth it at this point in time.

davidiw · 2021-03-31T05:25:43Z

dips/dip-8.md

+This DIP aimed to support a scenario of setting a Diem wallet as a long term payment method in an app or an eCommerce website. 
+
+The high level flow is:
+1. The user asks to add their wallet to the app as a payment method


This seems ... a little too ambiguous to me

User enters checkout flow

User clicks "Pay with Diem"

The user is presented with a list of registered Diem VASPs that support Merchant payments

The user selects their VASP

The merchant site opens an iframe sending forwarding the appropriate information for a FPPA request

The user logins and confirms the transaction

The VASP sends an FPPA response to the Merchant

Behind the scenes

The FPPA request sent entirely in the iframe URL

Upon login, the VASP initiates an off-chain kyc exchange with the merchant

Upon the conclusion of the off-chain, the VASP and Merchant have agreed upon fppa_id for future pull payments

davidiw · 2021-03-31T05:27:09Z

dips/dip-8.md

+| checkout_data_type         | str      | Y          | The fixed string `FUNDS_PULL_PRE_APPROVAL` |
+| action                     | str      | Y          | The fixed string `FUNDS_PULL_PRE_APPROVAL` |


I don't understand why we have this twice ... why not make this part of the URL syntax?

davidiw · 2021-03-31T05:28:45Z

dips/dip-8.md

+
+## Funds pull pre approval request fields
+
+The following fields should be used to define a funds pull pre approval request. These fields should be encoded into a series of URL parameters appended to the query string. In case of QR code, the image should include the full link with all parameters. 


so part of me wonders... we're going to have to go off-chain at some point anyway, so should we just pass along a payment_id and a vasp_address

davidiw · 2021-03-31T05:30:00Z

dips/dip-8.md

+
+| Field                  | Type    | Required?  | Description                                    |
+|------------------------|---------|------------|------------------------------------------------|
+| vasp_address               | str      | Y          | Address of account from which billing will happen. The address is encoded using bech32 and should uniquly identify the merchant. For Diem addresses format, refer to the "account identifiers" section in [DIP-5](https://dip.diem.com/dip-5/#account-identifiers). |


I'm not sure what vasp_address means... maybe this should say psp_account and the right side will actually be one of many possibilities: DiemId, dip-5, or even other identifiers... otherwise we continue to make bespoke diem protocols, yay.

davidiw · 2021-03-31T05:33:33Z

dips/dip-8.md

-All requests between VASPs are structured as [`CommandRequestObject`s](https://dip.diem.com/dip-1/#commandrequestobject) and all responses are structured as [`CommandResponseObject`s](https://dip.diem.com/dip-1/#commandresponseobject).  For a fund pre-approval command, the resulting request takes a form of the following:
+All requests between VASPs are structured as [`CommandRequestObject`s](https://dip.diem.com/dip-1/#commandrequestobject) and all responses are structured as [`CommandResponseObject`s](https://dip.diem.com/dip-1/#commandresponseobject). 
+
+For a funds pull pre-approval command, the resulting request takes a form of the following:

 ```
 {


since we already sent most of this via a auth framework, why are we sending it again via off-chain protocol? I think this function should purely be about doing a pre auth kyc... so this should largely be a copypasta of dip-1 -- but with the new states rauth / sauth, right?

dips/dip-158.md

Disclaimer regarding the discovery phase Extended scope for fund pull pre-approval Revoke an authorization in more details

Co-authored-by: kphfb <kph@fb.com>

…lit-dip-8

Co-authored-by: dahliamalkhi <dahliamalkhi@outlook.com>

…lit-dip-8

Co-authored-by: David Wolinsky <isaac.wolinsky@gmail.com>

Notes: Agreed that we will create new structures for P2M purposes rather than impose changes on existing structures from 0 < DIP < 2 (did not want to use the actual DIP name :)). For example, we will NOT extend or change the PaymentActorObject. Instead, we will create a new object suitable for P2M needs (e.g. BusinessActorObject). Agreed that business_data (i.e. merchant basic details) should be part of the InfoCommand response Agreed that payer_data (i.e. payer information) should be part of the InitCommand(s) request Agreed that, for the time being, there is no support for an additional step to ask for more data. The receiving VASP can either accept or reject the payment based on the information provided. We mentioned that the receiving VASP should be able to state "extra" information that is required on top of the payer_data (e.g. DOB might be required for buying alcohol) as part of the InfoCommand response. This will be achieved by an additional optional element (type will be JSON) A similar optional element will be added to the InitCommand(s) request. This element is to be used by the sending VASP to send the additional information requested (e.g. DOB) Agreed (with a heavy heart...) to stop using the functionality names and stick with the command types Agreed to use two different init command types (for each payment type) to replace the single InitCommand We will change the name of the ReadyCommand to be ReadyNotification Add dual attestation specification for transactions over the limit

Main changes: * Removed DIP-8 fro the pull request * Change InfoCommand name * Added image_url to business_data * Spelling and grammar issues

* Removed usage of statuses

…and request

* Naming consistency * Optional redirect URL for QR flow (POS scenario)

Added diagram images

kphfb and others added 13 commits March 15, 2021 13:57

Update dip-8.md

42d917f

Update lip-8.md:

f97e396

Disclaimer regarding the discovery phase Extended scope for fund pull pre-approval Revoke an authorization in more details

Update lips/lip-8.md

ab2d9c1

Co-authored-by: kphfb <kph@fb.com>

Update lips/lip-8.md

0e39526

Co-authored-by: kphfb <kph@fb.com>

Update lips/lip-8.md

d2eb4b1

Co-authored-by: kphfb <kph@fb.com>

Update lips/lip-8.md

ebfbb42

Co-authored-by: kphfb <kph@fb.com>

Update lips/lip-8.md

e84bda4

Co-authored-by: kphfb <kph@fb.com>

Update lips/lip-8.md

20886a6

Co-authored-by: kphfb <kph@fb.com>

Update lips/lip-8.md

85c89e2

Co-authored-by: kphfb <kph@fb.com>

Update lip-8.md

16c19a7

Apply suggestions from code review

649f868

Co-authored-by: kphfb <kph@fb.com>

PR fixes

9e93f63

split DIP-8 to funds pre approval and auth/capture

67a5c5e

davidiw reviewed Mar 31, 2021

View reviewed changes

danprinz and others added 2 commits April 5, 2021 15:32

add dip 158 for p2m charge + auth/capture

fbc8c89

Created DIP-158 (AKA P2M DIP).

b6e3476

danprinz commented Apr 14, 2021

View reviewed changes

YinonFirstDAG and others added 13 commits April 18, 2021 12:36

Incorporated additional comments

4e27e12

Update dip-8.md

21bbf2e

Update lip-8.md:

ad626e7

Disclaimer regarding the discovery phase Extended scope for fund pull pre-approval Revoke an authorization in more details

Update lips/lip-8.md

150a752

Co-authored-by: kphfb <kph@fb.com>

Update lips/lip-8.md

091ed30

Co-authored-by: kphfb <kph@fb.com>

Update lips/lip-8.md

2a9bafa

Co-authored-by: kphfb <kph@fb.com>

Update lips/lip-8.md

569cb35

Co-authored-by: kphfb <kph@fb.com>

Update lips/lip-8.md

fdba30c

Co-authored-by: kphfb <kph@fb.com>

Update lips/lip-8.md

5cda1b8

Co-authored-by: kphfb <kph@fb.com>

Update lips/lip-8.md

2bf7735

Co-authored-by: kphfb <kph@fb.com>

Update lip-8.md

becffc1

Apply suggestions from code review

4e08ec8

Co-authored-by: kphfb <kph@fb.com>

PR fixes

75b17b5

danprinz and others added 4 commits April 18, 2021 14:30

split DIP-8 to funds pre approval and auth/capture

3ee179f

add dip 158 for p2m charge + auth/capture

47ef81c

Created DIP-158 (AKA P2M DIP).

05b3537

Incorporated additional comments

116f663

danprinz force-pushed the split-dip-8 branch from 4e27e12 to 116f663 Compare April 18, 2021 11:30

danprinz and others added 25 commits April 18, 2021 14:32

Revert DIP-8 initial changes draft

e0cefc2

Merge branch 'split-dip-8' of https://github.com/firstdag/dip into sp…

c69da04

…lit-dip-8

Update dips/dip-158.md

fb6a549

Co-authored-by: dahliamalkhi <dahliamalkhi@outlook.com>

Update dips/dip-158.md

d211b5c

Co-authored-by: dahliamalkhi <dahliamalkhi@outlook.com>

Update dips/dip-158.md

d8557ac

Co-authored-by: dahliamalkhi <dahliamalkhi@outlook.com>

Update dips/dip-158.md

09d73ee

Co-authored-by: dahliamalkhi <dahliamalkhi@outlook.com>

Update dips/dip-158.md

184b768

Co-authored-by: dahliamalkhi <dahliamalkhi@outlook.com>

Merge branch 'split-dip-8' of https://github.com/firstdag/dip into sp…

217967d

…lit-dip-8

Updated based on latest comments from Dahlia

5a73ca9

Added glossary item for Reference ID based on Dahlia's comments.

0a6a149

Update dips/dip-158.md

045d1c7

Co-authored-by: David Wolinsky <isaac.wolinsky@gmail.com>

Update dips/dip-158.md

38408b2

Co-authored-by: David Wolinsky <isaac.wolinsky@gmail.com>

Updated based on David W. comments

1d89b20

Revert DIP-8 to master version to remove from pull request

7bc8bcf

Another cycle of comments incorporated.

ffa4e61

Main changes: * Removed DIP-8 fro the pull request * Change InfoCommand name * Added image_url to business_data * Spelling and grammar issues

Changed the order of sections and added internal links

dcfb3e2

Internal links fix

e418157

Appendix A link

a8a89f4

Added AddressObject and NationalIdObject

9c97c83

* Introduced PaymentCommandErrorObject

3163dbf

* Removed usage of statuses

add dual attestation info to charge command response and capture comm…

a67ac6f

…and request

* Grammar issue

3783ea1

* Naming consistency * Optional redirect URL for QR flow (POS scenario)

Added sequence diagrams and partial capture support

74efb7c

Update dip-158.md

f11559e

Added diagram images

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split dip 8 to funds pull pre approval and charge + auth/capture #2

Split dip 8 to funds pull pre approval and charge + auth/capture #2

danprinz commented Mar 29, 2021

davidiw left a comment

davidiw Mar 31, 2021

davidiw Mar 31, 2021

davidiw Mar 31, 2021

davidiw Mar 31, 2021

davidiw Mar 31, 2021

davidiw Mar 31, 2021

		\| checkout_data_type \| str \| Y \| The fixed string `FUNDS_PULL_PRE_APPROVAL` \|
		\| action \| str \| Y \| The fixed string `FUNDS_PULL_PRE_APPROVAL` \|


		## Funds pull pre approval request fields

		The following fields should be used to define a funds pull pre approval request. These fields should be encoded into a series of URL parameters appended to the query string. In case of QR code, the image should include the full link with all parameters.

Split dip 8 to funds pull pre approval and charge + auth/capture #2

Are you sure you want to change the base?

Split dip 8 to funds pull pre approval and charge + auth/capture #2

Conversation

danprinz commented Mar 29, 2021

davidiw left a comment

Choose a reason for hiding this comment

davidiw Mar 31, 2021

Choose a reason for hiding this comment

davidiw Mar 31, 2021

Choose a reason for hiding this comment

davidiw Mar 31, 2021

Choose a reason for hiding this comment

davidiw Mar 31, 2021

Choose a reason for hiding this comment

davidiw Mar 31, 2021

Choose a reason for hiding this comment

davidiw Mar 31, 2021

Choose a reason for hiding this comment