feat(examples): Add "Spending limit account" account abstraction example #8959
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 6 Skipped Deployments
|
theiari
added
sc-platform
| dynamic_field::add(account_id, SpendLimit {}, amount) | ||
| } | ||
|
|
||
| // === View Functions === |
There was a problem hiding this comment.
Now I am not entirely sure which direction we are going in, but if we want this to be in the utilities and accounts
direction, then don't forget to supply detach and rotate functions as well.
There was a problem hiding this comment.
Same as above, added both detach and rotate in 7ae3716.
| dynamic_field::borrow(account_id, SpendLimit {}) | ||
| } | ||
|
|
||
| public fun borrow_mut(account_id: &mut UID): &mut u64 { |
There was a problem hiding this comment.
I think this is a public function not a view one, but I get confused on this from time to time.
There was a problem hiding this comment.
Moved to public function section here: 7ae3716.
|
|
||
| // === Structs === | ||
|
|
||
| public struct SpendLimit has copy, drop, store {} |
There was a problem hiding this comment.
The pattern I tried to follow was to name the dynamic field and the containing module the same way.
spending_limit, SpendLimit, seems to be in minor inconsistency. Maybe it should be "SpendingLimit"?
There was a problem hiding this comment.
Makes sense. I changed it to SpendingLimit in 7ae3716.
|
|
||
| public fun authenticate( | ||
| account: &SpendLimit, | ||
| amount: u64, |
There was a problem hiding this comment.
Can you help me understand how this account type needs to work?
It looks somewhat odd that this amount is passed in as a value. The account can't really provide any protection in this regard right?
I have some vague memory that this is why we have the AuthContext, TxCommands fields, and vaguely remember you raising that there are some technical issues in calculating the gas I believe.
There was a problem hiding this comment.
Good point from Davo. It’s not clear for me how it can be used for real authentication if we trust only the passed value? I assume we need some logic behind the arguments, like Davo mentioned, using Context etc., to work with a gas.
There was a problem hiding this comment.
Yeah, that's a good point. I guess a solution might be to rely on vector<Coin<T>> type instead of u64.
I added a possible implementation here : 7ae3716 .
Not sure if that's enough, maybe there must be some extra logic behind AuthContext
| account::borrow_auth_info_v1(&account.id) | ||
| } | ||
|
|
||
| // === Admin Functions === |
There was a problem hiding this comment.
I guess all of these empty comment sections should be cleaned up?
In account.move as well
There was a problem hiding this comment.
Not necessarily. I started using that pattern to avoid issues in the future where we violate the code conventions.
Others follow different patterns, but this is probably the cleanest as the section names should always be the same and always in the same order.
There was a problem hiding this comment.
Just followed the latest flow 😄
| assert!(amount <= *spending_limit, EOverspend); | ||
| } | ||
|
|
||
| public fun attach(account_id: &mut UID, amount: u64) { |
There was a problem hiding this comment.
In this case it’s a minor thing, but for the full-fledged example it would be nice to have an appropriate functions to change the limit(e.g. Update())
There was a problem hiding this comment.
Yep, you're right. Added both detach and rotate functions here: 7ae3716.
…uthentication of passing a u64 amount
| } | ||
|
|
||
| // Calculate the sum of coin values. | ||
| public fun calculate_coin_sum<T>(coins: &vector<Coin<T>>): u64 { |
There was a problem hiding this comment.
Not entirely sure this is ok and if it should be kept public as well
|
|
||
| public fun authenticate<T>( | ||
| account: &SpendLimit, | ||
| coins: &vector<Coin<T>>, |
There was a problem hiding this comment.
I don't get how this authenticate function prevents me from spending from the account. Some coins are passed here as auth inputs, but who to say that there are no more coins as PTB inputs that the authenticated transactions can spend?
On another note, spending limit without a time frame is rather useless, no? If I can spend $100 at a time, but I can repeat it as many times as I can then it's pretty easy to overspend 
If the intention is to have an account where you can only spend $X/period, it's pretty tought to implement without write access in the auth function.
4 participants
Description of change
Implement "Account with Spending limit" example for abstract accounts.
Links to any relevant issues
fixes #8651
How the change has been tested