-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
add permissions for fungible assets operation #14567
base: 09-04-implement_rust_logics_for_permissioned_signer
Are you sure you want to change the base?
add permissions for fungible assets operation #14567
Conversation
⏱️ 2h 15m total CI duration on this PR
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 09-04-implement_rust_logics_for_permissioned_signer #14567 +/- ##
====================================================================================
Coverage 60.1% 60.1%
====================================================================================
Files 857 857
Lines 210762 210762
====================================================================================
+ Hits 126723 126744 +21
+ Misses 84039 84018 -21 ☔ View full report in Codecov by Sentry. |
dec4a79
to
6cd5fd0
Compare
350db4c
to
dad258f
Compare
6cd5fd0
to
88ad9a1
Compare
dad258f
to
6b4da66
Compare
88ad9a1
to
5f90e3a
Compare
6b4da66
to
b1fd015
Compare
5f90e3a
to
ef7be0f
Compare
b1fd015
to
6592c01
Compare
ef7be0f
to
cd6090b
Compare
6592c01
to
54c34ad
Compare
cd6090b
to
0df1c5a
Compare
54c34ad
to
1835d3d
Compare
0df1c5a
to
1bd468b
Compare
1835d3d
to
42decc7
Compare
1bd468b
to
40796a4
Compare
42decc7
to
6ac9449
Compare
40796a4
to
4940d7d
Compare
6ac9449
to
3ae6a87
Compare
4940d7d
to
d8742c4
Compare
3ae6a87
to
7fbfa3e
Compare
d8742c4
to
5d53f18
Compare
7fbfa3e
to
f3015ce
Compare
2a56afe
to
bde993b
Compare
c1bc146
to
790baf2
Compare
bde993b
to
415d5c2
Compare
790baf2
to
c557d1d
Compare
415d5c2
to
e0409cc
Compare
c557d1d
to
1212bb1
Compare
e0409cc
to
fb5cd39
Compare
1212bb1
to
57713ac
Compare
fb5cd39
to
0f881a5
Compare
57713ac
to
289ab4d
Compare
0f881a5
to
1302971
Compare
289ab4d
to
75ffedc
Compare
@@ -194,6 +196,10 @@ module aptos_framework::fungible_asset { | |||
metadata: Object<Metadata> | |||
} | |||
|
|||
struct WithdrawPermission has copy, drop, store { | |||
metadata_address: address, |
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.
let's make this have store_addr as well (by default people will be giving permission to PFS only)
maybe this should be an enum?
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 making it an enum is a great idea. However, I would avoid adding store_addr
at the moment as this would require coordination with wallet side how the permission should be displayed.
aptos-move/framework/aptos-framework/sources/fungible_asset.move
Outdated
Show resolved
Hide resolved
@@ -6,6 +6,7 @@ module aptos_framework::fungible_asset { | |||
use aptos_framework::event; |
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.
should we have a FungibleAssetManagementPermission - i.e. gating creating fungible assets and modifying their metadata, etc.
not sure if freezing should be part of the same, or separate one
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.
That's not needed thanks to the FA design. Those are already encoded as permission objects.
/// | ||
/// If `owner` is a permissioned signer, the signer will be granted with permission to withdraw | ||
/// the same amount of fund in the future. | ||
public fun deposit_with_signer(owner: &signer, fa: FungibleAsset) acquires DeriveRefPod { |
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.
do we need coin::deposit_with_signer as well?
1302971
to
4505dc2
Compare
d09b7ba
to
7d7eed9
Compare
4505dc2
to
2dfa163
Compare
7d7eed9
to
4db0036
Compare
2dfa163
to
7b93a7d
Compare
4db0036
to
d932531
Compare
d932531
to
15dfa6e
Compare
Description
Implemented permissions for FA transfer.
deposit_with_signer
so that permissions can be added to the signer back again.Type of Change
Which Components or Systems Does This Change Impact?
How Has This Been Tested?
Added test cases. In particular, tested the entire migration path of Coin and FA.
Key Areas to Review
Make sure FA permission works well throughout the entire migration path of FA.
Checklist