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

Lower all Assets / NFT deposits by 10x #1332

Merged
merged 5 commits into from
Jun 14, 2022

Conversation

shawntabrizi
Copy link
Member

@shawntabrizi shawntabrizi commented Jun 6, 2022

This PR lowers all the deposits of Statemine, Statemint, Westmint, and Contracts-Rococo by 10x.

This was decided based on a high level analysis of the cost of NFT deposits compared to other platforms.

This can always be adjusted later back up if needed.

Also fixes an issue where contracts-rococo did not use Rococo deposits.

@paritytech-ci paritytech-ci requested review from a team June 6, 2022 10:42
@shawntabrizi
Copy link
Member Author

TODO on this PR is to also reduce the base deposits which are not controlled by this formula

@kianenigma
Copy link
Contributor

Have you made sure that all deposits in all pallets are recorded, so changing this does not break any operation?

@shawntabrizi
Copy link
Member Author

Base deposits are now also updated. This should be good to go.

@shawntabrizi
Copy link
Member Author

@kianenigma checked:

  • assets
  • multisig
  • proxy
  • uniques

And these seem to be all the affected pallets.

@joepetrowski
Copy link
Contributor

Would be good to add a fn redeposit to the Assets pallet similar to Uniques.

joepetrowski
joepetrowski previously approved these changes Jun 14, 2022
@joepetrowski joepetrowski dismissed their stale review June 14, 2022 14:25

Overlooked one deposit

@paritytech-ci paritytech-ci requested a review from a team June 14, 2022 14:25
Copy link
Contributor

@joepetrowski joepetrowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent of this PR is really not clear. The title says to lower all deposits, but the description focuses specifically on NFT creation. The code changes lower transaction fees, but not the ED. This really needs more clarity before merging.

@paritytech-ci paritytech-ci requested a review from a team June 14, 2022 15:11
@shawntabrizi shawntabrizi changed the title Lower all deposits by 10x Lower all Assets / NFT deposits by 10x Jun 14, 2022
Copy link
Contributor

@joepetrowski joepetrowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now and aligns Statemint/Statemine to have a more consistent experience.

@shawntabrizi
Copy link
Member Author

@joepetrowski updated to represent what I want. Specifically to lower all Assets and NFT deposits. To do that, I also modified a function which affects deposits of some other pallets, but that is fine imo.

@shawntabrizi
Copy link
Member Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 1120195 into master Jun 14, 2022
@paritytech-processbot paritytech-processbot bot deleted the shawntabrizi-lower-deposits branch June 14, 2022 17:13
@hbulgarini hbulgarini restored the shawntabrizi-lower-deposits branch June 14, 2022 17:38
@hbulgarini hbulgarini deleted the shawntabrizi-lower-deposits branch June 14, 2022 17:40
hbulgarini pushed a commit that referenced this pull request Jun 14, 2022
* lower all deposits by 10x

* undo rococo stuff

* Apply suggestions from code review

* update asset deposits

* align statemint deposit ratios

Co-authored-by: joepetrowski <joe@parity.io>
@hbulgarini hbulgarini mentioned this pull request Jun 14, 2022
joepetrowski added a commit that referenced this pull request Jun 14, 2022
* lower all deposits by 10x

* undo rococo stuff

* Apply suggestions from code review

* update asset deposits

* align statemint deposit ratios

Co-authored-by: joepetrowski <joe@parity.io>

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: joepetrowski <joe@parity.io>
IkerAlus added a commit to IkerAlus/polkadot-wiki that referenced this pull request Jul 20, 2022
[This PR](paritytech/cumulus#1332) updated the deposit constants needed to create an asset (and to add the metadata). Therefore the values used in the current version of this page are outdated and wrong.
DrW3RK pushed a commit to w3f/polkadot-wiki that referenced this pull request Jul 20, 2022
[This PR](paritytech/cumulus#1332) updated the deposit constants needed to create an asset (and to add the metadata). Therefore the values used in the current version of this page are outdated and wrong.
alfarok pushed a commit to w3f/polkadot-wiki that referenced this pull request Jul 28, 2022
[This PR](paritytech/cumulus#1332) updated the deposit constants needed to create an asset (and to add the metadata). Therefore the values used in the current version of this page are outdated and wrong.
tash-2s added a commit to tash-2s/open-emoji-battler that referenced this pull request Apr 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants