-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add GET /outgoing-payment-grant endpoint #24
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
Conversation
and remove required identity field on outgoing payment access
openapi/resource-server.yaml
Outdated
| required: | ||
| - spentReceiveAmount | ||
| - spentDebitAmount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should we return when there is no outgoing payment grant spent amount? Would happen if the grant has been created but no payments yet, for example. Or maybe if there is an interval and no payments have been made in it yet.
My instinct was that simply constitutes 0. However, what will the asset scale and code be? We dont know without the spent amount. Can we reliably get what it will be somewhere else? Im not sure we can. Maybe we could get it off the quote but the quote is optional.
So alternatives I see would be changing this to not be required (null or undefined then client checks that before doing any incrementing, number comparison, etc.). Or make the asset scale and code optional for that case. Or values like an empty string, 0 ... but yuck.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having these fields as optional would be the most straightforward. Or, if we want the amounts before any payments have been created, then we can use the limits object in the outgoing payment access of the token introspection response
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will change them to optional but also look into using the limits when implementing in rafiki. I suppose maybe even if we want to use limits we may not always have them (a grant without spent or receive limits? - edge case because why would you look them up, but still)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we may not always have them
Actually yes, that is true. For outgoing payment limits in a grant, they are debitAmount XOR receiveAmount. So we won't be able to get both assets for spent amounts lookup before any payments are created.
| required: | ||
| - type | ||
| - actions | ||
| - identifier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in the grant request we can still keep this as a required, no?
Token introspection, we will just have a unique case where the identifier is not required when calling this route
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will remove this, after digging into it a bit more.
I was doing this because the token introspection request was failing at the validator. This is because on my (old) branch the token introspection open api spec references this auth-server spec. on Rafiki main I see we are now inlining all that stuff, so I can just update the token introspection spec in Rafiki (after updating my branch 😬) . I also see the point that this change has more implications than just loosening token introspection.
openapi/auth-server.yaml
Outdated
| info: | ||
| title: Open Payments Authorization Server | ||
| version: '1.1.1' | ||
| version: '1.2.1' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| version: '1.2.1' | |
| version: '1.2.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed all e107895
openapi/resource-server.yaml
Outdated
| required: | ||
| - spentReceiveAmount | ||
| - spentDebitAmount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having these fields as optional would be the most straightforward. Or, if we want the amounts before any payments have been created, then we can use the limits object in the outgoing payment access of the token introspection response
Co-authored-by: Max Kurapov <max@interledger.org>
GET /outgoing-payment-grantendpointfixes #15
will be used to completed interledger/rafiki#3375