add transaction_data #197
add transaction_data #197
Conversation
Sakurann
requested review from
ve7jtb,
danielfett,
jogu,
tlodderstedt,
c2bo,
tplooker,
paulbastian,
awoie and
bc-pi
Co-authored-by: Brian Campbell <[email protected]>
Co-authored-by: Brian Campbell <[email protected]>
Co-authored-by: Daniel Fett <[email protected]>
There was a problem hiding this comment.
I still believe that putting transaction data into a
a) a dedicated response_type and/or
b) using a dedicated Credential format would be a better separation.
vp_token and existing Credential Formats assume a three party model, while transaction authorization is a 2 party model, it seems this may create problems in the future that we can't foresee, I would appreciate a better logical separation.
I don't have explicit counter-proposals, just a bad feeling. If there is no support for this direction, you may override my disapproval.
|
|
At the EU Digital Wallet Consortium (EWC) we are about to start piloting a use case that includes using the wallet to confirm payment details (Dynamic Linking requirement from PSD2). Therefore we would be happy and explicitly support transaction_data to go into the next Implementer's Draft so that we can implement and test it and report our findings back to the community. |
Co-authored-by: Christian Bormann <[email protected]>
| The Wallet MUST ignore any unrecognized parameters, other than the `transaction_data` parameter. | ||
| The wallets that do not support the `transaction_data` parameter MUST reject requests that contain it. |
There was a problem hiding this comment.
why is this in this paragraph and not in the new parameters section?
There was a problem hiding this comment.
because this is about extensibility. the previous sentence says ignore if you don't recognize a parameter, and the next sentence says, but for one specific transaction_data parameter, do not ignore (ie the rule in the previous sentence does not apply)
There was a problem hiding this comment.
i can clarified this
Co-authored-by: Michael B. Jones <[email protected]>
I'd made a suggestion that added a reference to the new query language, but that stops the spec compiling because the new query language isn't merged yet and won't be for a few days. To let the spec compile, just remove that reference. We can always add it back later.
Add the necessary entry in {backmatter}.
Committed with Kristina's permission.
Co-authored-by: Christian Bormann <[email protected]> Co-authored-by: Paul Bastian <[email protected]>
|
Looks good to me with the changes! I guess we should not merge this until the Query Language PR is merged so it doesn't break the github action (vp_query reference)? |
|
4 approvals, discussed in multiple WG calls, open for over 4 months, all comments addressed - merging! Thank you everyone! |
We probably want to add this back after query language is merged, but for now we need to keep the spec compiling.
Well, merging after making a trivial tweak so the spec compiles (removing the #vp_query reference) - we should probably add back the two VP query section links once query language is merged. |
resolves #174
resolves #173
resolves #172