[NWC] Add get_budget command for per-connection budget limits. #1504
Conversation
This separates the notion of budget from `get_balance` so that permissions can be more appropriately controlled by the wallet. Budgets can be configured by users like how Alby supports, and client apps can query their remaining budget and when it will renew next. Breaking out a separate command for this purpose means that a user can let a client app see their own remaining budget without giving the app access to see their entire wallet balance.
jklein24
force-pushed
the
feat/get_budget
branch
from
9e6a987
to
17cd5be
Compare
Co-authored-by: Roland <[email protected]>
Thanks! We have it implemented internally, but hoping to get that live in the next couple of weeks. If there's no one to grab that ticket in alby hub and it's just in a public repo, I might be open to grabbing that ticket myself too :-) |
FYI @rolznz alby hub implementation is here: getAlby/hub#726 |
| ### `get_budget` | ||
|
|
||
| Budgets can be used to limit the amount of funds that can be spent by a connection in a given time period. | ||
| Budgets are optional and can be set by the user if their wallet supports it. Support for this command indicates |
There was a problem hiding this comment.
@rolznz and I were discussing on the alby-hub implementation what should happen if there's no budget set for a connection. My initial thinking was that absence of a budget would be represented by an empty struct return value. However, @rolznz suggests to respond with an error instead. Some tradeoff off the top of my head:
Empty response:
- [+] Logically makes sense to return no budget when no budget is set. It's not really an error case, but an expected scenario.
- [-] Making each of the fields optional/nullable in the response is not great from an ergonomics standpoint. It's nicer to be able to just expect a full response
Error response
- [+] No null fields. Pretty explicit that there's no budget and that the response is intentionally different.
- [-] Global error handling in client apps may be triggered from this, when it's not really an error, but rather an expected state. I could imagine accidental error toasts, etc. in client apps.
- [-] Client apps may not know ahead of time whether there's a budget on the connection, so they can't just avoid calling this method to avoid the error.
@rolznz wdyt of these tradeoffs? Anything I'm missing?
There was a problem hiding this comment.
Client apps may not know ahead of time whether there's a budget on the connection, so they can't just avoid calling this method to avoid the error.
@jklein24 if the app calls get_info it could know if it can call get_budget or not. I think this is a good thing for the app to do first so it knows what methods it can call. What do you think?
If it returned an empty response, would the developer check if total_budget is not empty? (e.g. in javascript: if (!!response.total_budget) { /* app has a budget */ }? ) - if we go this way, should we add some note in this spec so developers more easily know what to do?
There was a problem hiding this comment.
@jklein24 if the app calls get_info it could know if it can call get_budget or not. I think this is a good thing for the app to do first so it knows what methods it can call. What do you think?
Yeah, that's true. In general, is the assumption that apps normally call get_info before doing things with a connection?
If it returned an empty response, would the developer check if total_budget is not empty? (e.g. in javascript: if (!!response.total_budget) { /* app has a budget */ }? ) - if we go this way, should we add some note in this spec so developers more easily know what to do?
Yeah, agreed this is a little gross :-/. Either way we decide on this, we should definitely document it here.
There was a problem hiding this comment.
Yeah, that's true. In general, is the assumption that apps normally call get_info before doing things with a connection?
Yeah, this is our recommendation. Because a wallet service could post an info event about what methods it supports, but the app connection itself could be different, based on what permissions the user selects when configuring the connection.
There was a problem hiding this comment.
That makes sense. I guess I can be convinced on the error instead. @shreyav @bsiaotickchong curious if either of you have thoughts on this one.
There was a problem hiding this comment.
I'd expect if wallets/users using budgets are also mainly creating connections with budgets, on average it'd be faster to just call get_budget without having to wait for the get_info response every time. In this case I think the empty response for get_budget is preferable
There was a problem hiding this comment.
Note: Alby Hub + JS SDK currently implement the empty response option for the no budget case.
This separates the notion of budget from
get_balanceso that permissions can be more appropriately controlled by the wallet. Budgets can be configured by users like how Alby supports, and client apps can query their remaining budget and when it will renew next.Breaking out a separate command for this purpose means that a user can let a client app see their own remaining budget without giving the app access to see their entire wallet balance.