This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Assets Pallet: reintroduce fungibles::Destroy trait #12690
Closed
Closed
Changes from all commits
Commits
Show all changes
78 commits
Select commit
Hold shift + click to select a range
6a9d9d1
Support repeated destroys to safely destroy large assets
tonyalaribe 7db1fd5
require freezing accounts before destroying
tonyalaribe cbe14b2
support only deleting asset as final stage when there's no assets left
tonyalaribe 001eaef
Merge branch 'master' into aa/safely-destroy-large-assets
tonyalaribe 1beae07
pre: introduce the RemoveKeyLimit config parameter
tonyalaribe c9ce171
debug_ensure empty account in the right if block
tonyalaribe 3067e38
update to having separate max values for accounts and approvals
tonyalaribe 7c4f610
add tests and use RemoveKeyLimit constant
tonyalaribe aca497f
add useful comments to the extrinsics, and calculate returned weight
tonyalaribe 634345a
Merge branch 'master' of github.com:paritytech/substrate into aa/safe…
tonyalaribe 93d886d
add benchmarking for start_destroy and finish destroy
tonyalaribe 2d3b650
push failing benchmark logic
tonyalaribe 5a612d1
add benchmark tests for new functions
tonyalaribe b961255
update weights via local benchmarks
tonyalaribe 91e3c18
remove extra weight file
tonyalaribe 60f954a
Update frame/assets/src/lib.rs
tonyalaribe ce00b5b
Update frame/assets/src/types.rs
tonyalaribe 5492b6d
Update frame/assets/src/lib.rs
tonyalaribe bc20a6c
effect some changes from codereview
tonyalaribe 6ceb592
use NotFrozen error
tonyalaribe 2edac52
remove origin checks, as anyone can complete destruction after owner …
tonyalaribe aa2aecf
fix comments about Origin behaviour
tonyalaribe 10fdd90
add AssetStatus docs
tonyalaribe 139d2e9
modularize logic to allow calling logic in on_idle and on_initialize …
tonyalaribe 597f532
introduce simple migration for assets details
tonyalaribe a09e762
Merge remote-tracking branch 'origin/master' into aa/safely-destroy-l…
70930ac
reintroduce logging in the migrations
tonyalaribe dc4a243
move deposit_Event out of the mutate block
tonyalaribe 05d778d
Update frame/assets/src/functions.rs
tonyalaribe 62d4f13
Update frame/assets/src/migration.rs
tonyalaribe e9e108b
move AssetNotLive checkout out of the mutate blocks
tonyalaribe 5df4a62
Merge branch 'aa/safely-destroy-large-assets' of github.com:paritytec…
tonyalaribe 544ac52
rename RemoveKeysLimit to RemoveItemsLimit
tonyalaribe 9206acf
update docs
tonyalaribe 29c7745
fix event name in benchmark
tonyalaribe 2986f29
fix cargo fmt.
tonyalaribe 6ac84a1
fix lint in benchmarking
tonyalaribe 1f2930f
Merge branch 'master' of github.com:paritytech/substrate into aa/safe…
tonyalaribe 791aa7f
Empty commit to trigger CI
tonyalaribe 450a698
Update frame/assets/src/lib.rs
tonyalaribe 3daef79
Update frame/assets/src/lib.rs
tonyalaribe ea34cf5
Update frame/assets/src/functions.rs
tonyalaribe 84cbb47
Update frame/assets/src/functions.rs
tonyalaribe 082a10c
Update frame/assets/src/functions.rs
tonyalaribe e84d5ae
Update frame/assets/src/lib.rs
tonyalaribe 78cde15
Update frame/assets/src/functions.rs
tonyalaribe c6470fb
effect change suggested during code review
tonyalaribe 85e50b9
move limit to a single location
tonyalaribe c97e850
Merge branch 'master' of github.com:paritytech/substrate into aa/safe…
tonyalaribe df27e0b
Update frame/assets/src/functions.rs
tonyalaribe ca9fa2e
rename events
tonyalaribe 9609f13
fix weight typo, using rocksdb instead of T::DbWeight. Pending genera…
tonyalaribe 26d27cc
switch to using dead_account.len()
tonyalaribe 72ec8f8
rename event in the benchmarks
tonyalaribe ee30cf8
empty to retrigger CI
tonyalaribe 87a7877
Merge branch 'master' of github.com:paritytech/substrate into aa/safe…
tonyalaribe 773212e
trigger CI to check cumulus dependency
tonyalaribe a4be57f
trigger CI for dependent cumulus
tonyalaribe 111da0e
Merge branch 'master' into aa/safely-destroy-large-assets
tonyalaribe 9c06bb8
Update frame/assets/src/migration.rs
tonyalaribe 4eef069
move is-frozen to the assetStatus enum (#12547)
tonyalaribe 486814f
add pre and post migration hooks
tonyalaribe c185cb0
update do_transfer logic to add new assert for more correct error mes…
tonyalaribe 91095af
trigger CI
tonyalaribe 9ef3c2b
switch checking AssetStatus from checking Destroying state to checkin…
tonyalaribe 6165576
fix error type in tests from Frozen to AssetNotLive
tonyalaribe 7811b62
trigger CI
tonyalaribe cd3e28d
change ensure check for fn reducible_balance()
tonyalaribe aee596e
change the error type to Error:<T,I>::IncorrectStatus to be clearer
tonyalaribe 30814df
Trigger CI
tonyalaribe 9b653a5
Merge branch 'master' into aa/safely-destroy-large-assets
tonyalaribe 57fe747
Merge branch 'master' into aa/safely-destroy-large-assets
tonyalaribe 9580cea
reintroduce the destroy trait
tonyalaribe b6cca71
modify fungibles::Destroy public trait
tonyalaribe 9a18340
reintroduce trait
tonyalaribe 01f3fe3
update relevant trait documentation
tonyalaribe a408a3d
update docs formatting
tonyalaribe c4cb5d0
Merge branch 'master' into aa/reintroduce-destroy-into-fungibles-trait2
tonyalaribe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -267,25 +267,52 @@ pub trait Create<AccountId>: Inspect<AccountId> { | |
|
||
/// Trait for providing the ability to destroy existing fungible assets. | ||
pub trait Destroy<AccountId>: Inspect<AccountId> { | ||
/// The witness data needed to destroy an asset. | ||
type DestroyWitness; | ||
|
||
/// Provide the appropriate witness data needed to destroy an asset. | ||
fn get_destroy_witness(id: &Self::AssetId) -> Option<Self::DestroyWitness>; | ||
|
||
/// Destroy an existing fungible asset. | ||
/// * `id`: The `AssetId` to be destroyed. | ||
/// * `witness`: Any witness data that needs to be provided to complete the operation | ||
/// successfully. | ||
/// Start the destruction an existing fungible asset. | ||
/// * `id`: The `AssetId` to be destroyed. successfully. | ||
/// * `maybe_check_owner`: An optional account id that can be used to authorize the destroy | ||
/// command. If not provided, we will not do any authorization checks before destroying the | ||
/// command. If not provided, no authorization checks will be performed before destroying | ||
/// asset. | ||
fn start_destroy(id: Self::AssetId, maybe_check_owner: Option<AccountId>) -> DispatchResult; | ||
|
||
/// Destroy all accounts associated with a given asset. | ||
/// `destroy_accounts` should only be called after `start_destroy` has been called, and the | ||
/// asset is in a `Destroying` state | ||
/// | ||
/// If successful, this function will return the actual witness data from the destroyed asset. | ||
/// This may be different than the witness data provided, and can be used to refund weight. | ||
fn destroy( | ||
id: Self::AssetId, | ||
witness: Self::DestroyWitness, | ||
maybe_check_owner: Option<AccountId>, | ||
) -> Result<Self::DestroyWitness, DispatchError>; | ||
/// * `id`: The identifier of the asset to be destroyed. This must identify an existing asset. | ||
/// * `max_items`: The maximum number of accounts to be destroyed for a given call of the | ||
/// function. This value should be small enough to allow the operation fit into a logical | ||
/// block. | ||
/// | ||
/// Response: | ||
/// * u32: Total number of approvals which were actually destroyed | ||
/// | ||
/// Due to weight restrictions, this function may need to be called multiple | ||
/// times to fully destroy all approvals. It will destroy `max_items` approvals at a | ||
/// time. | ||
fn destroy_accounts(id: Self::AssetId, max_items: u32) -> Result<u32, DispatchError>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. left wondering what the u32 returned is. Is it the amount left or destroyed? Be good to be explicit about what it's returning. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's true. I'll add it to the comment. |
||
|
||
/// Destroy all approvals associated with a given asset up to the `max_items` | ||
/// `destroy_approvals` should only be called after `start_destroy` has been called, and the | ||
/// asset is in a `Destroying` state | ||
/// | ||
/// * `id`: The identifier of the asset to be destroyed. This must identify an existing asset. | ||
/// * `max_items`: The maximum number of accounts to be destroyed for a given call of the | ||
/// function. This value should be small enough to allow the operation fit into a logical | ||
/// block. | ||
/// | ||
/// Response: | ||
/// * u32: Total number of approvals which were actually destroyed | ||
/// | ||
/// Due to weight restrictions, this function may need to be called multiple | ||
/// times to fully destroy all approvals. It will destroy `max_items` approvals at a | ||
/// time. | ||
fn destroy_approvals(id: Self::AssetId, max_items: u32) -> Result<u32, DispatchError>; | ||
|
||
/// Complete destroying asset and unreserve currency. | ||
/// `finish_destroy` should only be called after `start_destroy` has been called, and the | ||
/// asset is in a `Destroying` state. All accounts or approvals should be destroyed before | ||
/// hand. | ||
/// | ||
/// * `id`: The identifier of the asset to be destroyed. This must identify an existing asset. | ||
fn finish_destroy(id: Self::AssetId) -> DispatchResult; | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
doc for max_items is missing
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.
Yeah, i've added that now. Thanks