feat: add third party to the declarative payment network#528
feat: add third party to the declarative payment network#528
Conversation
vrolland
changed the title
vrolland
changed the title
…uestNetwork/requestNetwork into adv-logic-src-third-part-declarative
vrolland
requested review from
Dadogg80,
benjlevesque,
bertux,
siibars and
yomarion
and removed request for
benjlevesque
yomarion
left a comment
yomarion
left a comment
There was a problem hiding this comment.
Nice ! I wonder if I miss something about payer impersonation or if just disagree, but we may have to talk a bit :)
| if (!requestState.payer && !extensionState.values.thirdparty) { | ||
| throw Error(`The request must have a payer or a thirdparty`); |
There was a problem hiding this comment.
I'm a bit surprised by this condition, a declarative request allows the third party to detect that the payment has been made, which is an action done by the issuer, not the payer. Am I missing something?
There was a problem hiding this comment.
@yomarion this action is a "payment sent" action
There was a problem hiding this comment.
Yes, I know. I think there are flows, it's not because the issuer trusts the 3rd party that the payer has to trust it.
packages/advanced-logic/src/extensions/payment-network/declarative.ts
Outdated
Show resolved
Hide resolved
packages/advanced-logic/src/extensions/payment-network/declarative.ts
Outdated
Show resolved
Hide resolved
| if ( | ||
| !Utils.identity.areEqual(actionSigner, requestState.payer) && | ||
| !Utils.identity.areEqual(actionSigner, extensionState.values.thirdparty) | ||
| ) { | ||
| throw Error(`The signer must be the payer or the thirdparty`); |
There was a problem hiding this comment.
I don't think the third-party should have payer privileges. Maybe there should be a payee third party and a payer third party (in a later version ?) but one actor can only delegate his own action to a third-party, in my opinion.
Let's take bank accounts, if a third-party appointed by the issuer can make sure the refunds has been sent, it cannot know on behalf of the payer that the money has been received.
(this comment applies for all the functions below where the third party can impersonate the payer)
There was a problem hiding this comment.
this is in line with my earlier comment about multiple thirdparties
There was a problem hiding this comment.
OK, are we in line also to make the multiple third parties in another release ?
benjlevesque
left a comment
benjlevesque
left a comment
There was a problem hiding this comment.
looks good! We need to decide on @yomarion's comment about the thirdparty privileges.
| if (!requestState.payer && !extensionState.values.thirdparty) { | ||
| throw Error(`The request must have a payer or a thirdparty`); |
There was a problem hiding this comment.
@yomarion this action is a "payment sent" action
| if ( | ||
| !Utils.identity.areEqual(actionSigner, requestState.payer) && | ||
| !Utils.identity.areEqual(actionSigner, extensionState.values.thirdparty) | ||
| ) { | ||
| throw Error(`The signer must be the payer or the thirdparty`); |
There was a problem hiding this comment.
this is in line with my earlier comment about multiple thirdparties
| name: EVENTS_NAMES; | ||
| parameters?: TEventParameters; | ||
| timestamp?: number; | ||
| from?: IIdentity; |
There was a problem hiding this comment.
this will be used only by declarative, right?
I think adding a comment is required here
There was a problem hiding this comment.
Only for declarative for now. But could be use somewhere else if needed later.
4 participants
I have added
fromin the event to make the difference between declaration from the thirdpary and the payee/payer.