Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Assets pallet: Giving the asset owner the ability to set minimum balance #13486

Merged
merged 10 commits into from
Mar 2, 2023
Merged

Assets pallet: Giving the asset owner the ability to set minimum balance #13486

merged 10 commits into from
Mar 2, 2023

Conversation

Szegoo
Copy link
Contributor

@Szegoo Szegoo commented Feb 27, 2023 •
edited
Loading

Closes: #13402

Introduces a new extrinsic set_min_balance which allows the owner of the specific asset to change the min_balance required for an asset.

This is possible to do if there aren't any accounts holding the asset or if the new min balance is less than the old min balance.

Cumulus companion: paritytech/cumulus#2253

@Szegoo Szegoo changed the title set_min_balance Assets pallet: Giving the asset owner the ability to set minimum balance Feb 27, 2023
@Szegoo Szegoo marked this pull request as ready for review February 28, 2023 13:54
Szegoo
Szegoo commented Feb 28, 2023
View reviewed changes
frame/assets/src/lib.rs Outdated Show resolved Hide resolved
@Szegoo
Copy link
Contributor Author

Szegoo commented Feb 28, 2023

@jsidorenko @bkchr Could you review this?

jsidorenko
jsidorenko reviewed Feb 28, 2023
View reviewed changes
frame/assets/src/lib.rs Outdated Show resolved Hide resolved
@jsidorenko jsidorenko added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes and removed A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes labels Feb 28, 2023
bkchr
bkchr approved these changes Feb 28, 2023
View reviewed changes
frame/assets/src/lib.rs Outdated Show resolved Hide resolved
frame/assets/src/lib.rs Outdated Show resolved Hide resolved
@bkchr bkchr added A0-please_review Pull request needs code review. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. B1-note_worthy Changes should be noted in the release notes T1-runtime This PR/Issue is related to the topic “runtime”. labels Feb 28, 2023
@bkchr
Copy link
Member

bkchr commented Feb 28, 2023

@ggwpez could you please summon the mighty benchmark bot?

@bkchr bkchr added the C1-low PR touches the given topic and has a low impact on builders. label Feb 28, 2023
Szegoo and others added 2 commits February 28, 2023 20:53
@Szegoo @bkchr
Update frame/assets/src/lib.rs
8011299 
Co-authored-by: Bastian Köcher <git@kchr.de>
@Szegoo @bkchr
Update frame/assets/src/lib.rs
2f3103e 
Co-authored-by: Bastian Köcher <git@kchr.de>
@ggwpez
Copy link
Member

ggwpez commented Feb 28, 2023

bot bench $ pallet dev pallet_assets

@command-bot
Copy link

command-bot bot commented Feb 28, 2023 •
edited
Loading

@ggwpez https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2463794 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_assets. Check out https://gitlab.parity.io/parity/mirrors/substrate/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 113-144b5381-c26e-4d25-965a-c1e9e2f097c2 to cancel this command or bot cancel to cancel all commands in this pull request.

@ggwpez
Copy link
Member

ggwpez commented Feb 28, 2023 •
edited
Loading

bot help

☝️ we finally got it

@command-bot
Copy link

command-bot bot commented Feb 28, 2023

Here's a link to docs

@command-bot
Copy link

command-bot bot commented Feb 28, 2023

@ggwpez Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" pallet dev pallet_assets has finished. Result: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2463794 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2463794/artifacts/download.

@Szegoo Szegoo mentioned this pull request Feb 28, 2023
Merged
@jsidorenko
Copy link
Contributor

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 4aa213c into paritytech:master Mar 2, 2023
@IkerAlus
Copy link

IkerAlus commented Mar 2, 2023

Sorry for jumping in when this is already merged but I want to make sure this new extrinsic is not opening a door for blockchain state spamming:

Image the following: An asset is created on Statemint, the team behind the asset makes a gov proposal to make it sufficient with a reasonable min_balance similar to the ED in DOTs. The gov proposal is passed, then the team behind the asset (owner of the asset) lowers down the min_balance via this new extrinsic to a ridiculously small value. Attackers start spamming the blockchain with millions of super cheap transfer transactions to random accounts (fees will be low since they are proportional to min_balance value).

How is this scenario prevented? I was expecting to see a check for asset sufficiency to stop the update of the min_balance via the extrinsic.

@jsidorenko
Copy link
Contributor

@IkerAlus You're right! There should be a check for sufficiency in that call, as you'be described in the issue

@Szegoo Szegoo mentioned this pull request Mar 2, 2023
Merged
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-40/2468/1

@Garandor Garandor mentioned this pull request Apr 1, 2023
Draft
5 tasks
@kiltbot kiltbot mentioned this pull request Apr 3, 2023
Closed
@librelois librelois mentioned this pull request Apr 6, 2023
Closed
ukint-vs pushed a commit to gear-tech/substrate that referenced this pull request Apr 10, 2023
@Szegoo @bkchr @ukint-vs
Assets pallet: Giving the asset owner the ability to set minimum bala…
882440c 
…nce (paritytech#13486)

* set_min_balance

* allow when new_min_balance < old_min_balance

* add more specific event

* Update frame/assets/src/lib.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

* Update frame/assets/src/lib.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_assets

* use actual weight

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: command-bot <>
@kacperzuk-neti kacperzuk-neti mentioned this pull request Jun 23, 2023
Merged
15 tasks
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@Szegoo @bkchr @nathanwhit
Assets pallet: Giving the asset owner the ability to set minimum bala…
9e49a0d 
…nce (paritytech#13486)

* set_min_balance

* allow when new_min_balance < old_min_balance

* add more specific event

* Update frame/assets/src/lib.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

* Update frame/assets/src/lib.rs

Co-authored-by: Bastian Köcher <git@kchr.de>

* ".git/.scripts/commands/bench/bench.sh" pallet dev pallet_assets

* use actual weight

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: command-bot <>
@dmitrylavrenov dmitrylavrenov mentioned this pull request Nov 28, 2023
Closed
46 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkchr bkchr bkchr approved these changes

@jsidorenko jsidorenko jsidorenko approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow to update min_balance by asset owner for non-sufficient assets in assets pallet
6 participants
@Szegoo @bkchr @ggwpez @jsidorenko @IkerAlus @Polkadot-Forum