-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Assets: Remove zombies, introduce approvals #8220
Conversation
…/substrate into gav-self-sufficient-ref-count
@shawntabrizi should be good to go. |
Finished benchmark for branch: gav-assets-approve-api Benchmark: Benchmark Runtime Pallet cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_assets --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/assets/src/weights.rs --template=./.maintain/frame-weight-template.hbs ResultsPallet: "pallet_assets", Extrinsic: "create", Lowest values: [], Highest values: [], Steps: [50], Repeat: 20
|
…/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_assets --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/assets/src/weights.rs --template=./.maintain/frame-weight-template.hbs
frame/assets/src/weights.rs
Outdated
.saturating_add(T::DbWeight::get().reads(2 as Weight)) | ||
.saturating_add(T::DbWeight::get().writes(2 as Weight)) | ||
} | ||
fn transfer() -> Weight { | ||
(42_211_000 as Weight) | ||
(71_531_000 as Weight) |
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.
This still doesn't look good. Increase of 75% in time to execute...
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.
As far as I can tell, it's down to the fact that the new code always bumps reference in the system account, whereas the old code didn't need to if it was a zombie (which is the case with the old benchmarking). odd that reading a storage item and bumping a reference would cost so much.
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.
Introduced transfer_keep_alive
which should have a lower weight since it will never result in the source account getting killed.
frame/assets/src/weights.rs
Outdated
.saturating_add(T::DbWeight::get().reads(2 as Weight)) | ||
.saturating_add(T::DbWeight::get().writes(2 as Weight)) | ||
} | ||
fn burn() -> Weight { | ||
(29_245_000 as Weight) | ||
(47_042_000 as Weight) |
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.
suspicious.
frame/assets/src/weights.rs
Outdated
} | ||
fn mint() -> Weight { | ||
(32_995_000 as Weight) | ||
(46_912_000 as Weight) |
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.
suspicious
…ate into gav-assets-approve-api
/benchmark runtime pallet pallet_assets |
Finished benchmark for branch: gav-assets-approve-api Benchmark: Benchmark Runtime Pallet cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_assets --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/assets/src/weights.rs --template=./.maintain/frame-weight-template.hbs ResultsCompiling node-runtime v2.0.1 (/home/shawntabrizi/bench-bot/git/substrate/bin/node/runtime) error[E0308]: mismatched types error[E0308]: mismatched types error: aborting due to 3 previous errors Some errors have detailed explanations: E0308, E0599. To learn more, run the command again with --verbose. |
/benchmark runtime pallet pallet_assets |
Finished benchmark for branch: gav-assets-approve-api Benchmark: Benchmark Runtime Pallet cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_assets --extrinsic="*" --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/assets/src/weights.rs --template=./.maintain/frame-weight-template.hbs ResultsPallet: "pallet_assets", Extrinsic: "create", Lowest values: [], Highest values: [], Steps: [50], Repeat: 20
|
…/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_assets --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/assets/src/weights.rs --template=./.maintain/frame-weight-template.hbs
bot merge |
Trying merge. |
Merge failed: |
Removed the concept of zombies; assets may be self-sufficient (in which case their min-balance actually provides enough value lockup to allow for the storage) or otherwise all non-zero account balances must have a consumer reference (so the account must have some primary balance ED or other provider reference).
Also added the concept of approvals; these work in a similar way to ERC-20 style approvals and allow for an asset owner to distribute assets without needing to pre-fund each account with an existential deposit. A deposit (in the primary token) must be made for each asset distributed, but that may be retracted at any time and the deposit freed.
Also adds
transfer_keep_alive
, which has a similiar purpose to its namesake in Balances pallet.TODO:
Includes #8221