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

Self-sufficient account ref-counting #8221

Merged
merged 15 commits into from
Mar 3, 2021
Merged

Conversation

gavofyork
Copy link
Member

@gavofyork gavofyork commented Feb 27, 2021

Introduces a new ref-counted account type next to providers and consumers, called self-sufficient references. These allow an account to exist, but do not in themselves allow consumers to exist (for that there must be at least one provider reference on the account).

This is useful for times where you know that an aspect of an account is enough to pay for its basic existence, but where you want to guarantee that the reference can be yanked (probably by someone other than the account holder) without concern of there being consumers which depend on it.

Depended on by #8220

Requires audit.

Includes migration!

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Feb 27, 2021
@gavofyork gavofyork added B7-runtimenoteworthy C1-low PR touches the given topic and has a low impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Feb 27, 2021
@gavofyork
Copy link
Member Author

CC @xlc

frame/system/src/lib.rs Outdated Show resolved Hide resolved
Co-authored-by: Jaco Greeff <jacogr@gmail.com>
migrations::migrate_to_dual_ref_count::<T>()
if !UpgradedToTripleRefCount::<T>::get() {
UpgradedToTripleRefCount::<T>::put(true);
migrations::migrate_to_triple_ref_count::<T>()
Copy link
Member

Choose a reason for hiding this comment

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

Can we not call the migration explicitly in the particular runtimes?

Copy link
Member Author

Choose a reason for hiding this comment

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

sure - i'm just concerned about it being forgotten.

@xlc
Copy link
Contributor

xlc commented Feb 27, 2021

I think I understand the purpose of this but still think the whole account ref system is under utilized & under documented. Will be helpful to have some guides to explain how it should be used along with examples. #5255

frame/system/src/lib.rs Show resolved Hide resolved
frame/system/src/lib.rs Show resolved Hide resolved
frame/system/src/lib.rs Show resolved Hide resolved
frame/system/src/lib.rs Show resolved Hide resolved
frame/system/src/lib.rs Outdated Show resolved Hide resolved
gavofyork and others added 3 commits March 2, 2021 13:22
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>
Copy link
Member

@shawntabrizi shawntabrizi left a comment

Choose a reason for hiding this comment

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

lgtm

frame/system/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

Logic within the PR lgtm, haven't looked at the linked PR yet. I do agree with @xlc that a guide system refernces would be needed. I also have some doubts about why and how we increment them in some pallets and not in others (e.g. #7305)

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
@gavofyork gavofyork merged commit e2d74ac into master Mar 3, 2021
@gavofyork gavofyork deleted the gav-self-sufficient-ref-count branch March 3, 2021 21:28
arjanz added a commit to polkascan/py-scale-codec that referenced this pull request Mar 5, 2021
@shawntabrizi shawntabrizi added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Apr 8, 2021
gguoss pushed a commit to chainx-org/ChainX that referenced this pull request Feb 25, 2022
* Remove VestingAccount

* Fix chain specs

* Update rust-toolchain

* Update make test and benchmark

* Update CI

* Run `make format` and `make clippy`

* Update CI

* Quick fix

* Support try-runtime

* Remove pallet-randomness-collective-flip

* Migrate elections-phragmen
(paritytech/substrate#7040)

* Migrate PalletVersion to StorageVersion
(paritytech/substrate#9165)

* Migrate pallet-babe epoch config
(paritytech/substrate#8072)

* Migrate frame-system AccountInfo to AccountInfoWithTripleRefCount
(paritytech/substrate#8221)

* Migrate prefix `GrandpaFinality` -> `Grandpa`

* Migrate prefix `Instance1Collective` -> `Council`

* Migrate prefix `Instance2Collective` -> `TechnicalCommittee`

* Migrate prefix `Instance1Membership` -> `TechnicalMembership`

* Migrate pallet-tips prefix from `Treasury` -> `Tips`

* Migrate pallet-bounties prefix from `Treasury` -> `Bounties`

* Migrate prefix from `PhragmenElection` -> `Elections`

* Revert `dev` spec_name

* Confirm migrations order

* Use ChainX substrate patch for system migration

* Run `make format`

* Reset spec_name `dev` -> `chainx`

* Add migrations.rs for mainnet and testnet

* Bump ChainX version to `4.0.0`

* Bump ChainX version to `4.0.0`

* Update governance parameters for dev and malan

* Update malan chainspec

* Quick fix

* Quick fix

* Update generate_keys.sh

* Adjust malan parameters

* Update malan chainspec

* Update malan chainspec

* Update malan chainspec

* Rename malan testnet name

* Add bootnodes url

* Run `make format`

* Regenerate weights

* Disable pre_release.yml CI

* Regenerate benchmark weights

* Run `make clippy`

* Run `make format`

Co-authored-by: icodezjb <icodezjb@users.noreply.github.com>
gguoss pushed a commit to chainx-org/ChainX that referenced this pull request Feb 25, 2022
* Remove VestingAccount

* Fix chain specs

* Update rust-toolchain

* Update make test and benchmark

* Update CI

* Run `make format` and `make clippy`

* Update CI

* Quick fix

* Support try-runtime

* Remove pallet-randomness-collective-flip

* Migrate elections-phragmen
(paritytech/substrate#7040)

* Migrate PalletVersion to StorageVersion
(paritytech/substrate#9165)

* Migrate pallet-babe epoch config
(paritytech/substrate#8072)

* Migrate frame-system AccountInfo to AccountInfoWithTripleRefCount
(paritytech/substrate#8221)

* Migrate prefix `GrandpaFinality` -> `Grandpa`

* Migrate prefix `Instance1Collective` -> `Council`

* Migrate prefix `Instance2Collective` -> `TechnicalCommittee`

* Migrate prefix `Instance1Membership` -> `TechnicalMembership`

* Migrate pallet-tips prefix from `Treasury` -> `Tips`

* Migrate pallet-bounties prefix from `Treasury` -> `Bounties`

* Migrate prefix from `PhragmenElection` -> `Elections`

* Revert `dev` spec_name

* Confirm migrations order

* Use ChainX substrate patch for system migration

* Run `make format`

* Reset spec_name `dev` -> `chainx`

* Add migrations.rs for mainnet and testnet

* Bump ChainX version to `4.0.0`

* Bump ChainX version to `4.0.0`

* Update governance parameters for dev and malan

* Update malan chainspec

* Quick fix

* Quick fix

* Update generate_keys.sh

* Adjust malan parameters

* Update malan chainspec

* Update malan chainspec

* Update malan chainspec

* Rename malan testnet name

* Add bootnodes url

* Run `make format`

* Regenerate weights

* Disable pre_release.yml CI

* Regenerate benchmark weights

* Run `make clippy`

* Run `make format`

Co-authored-by: icodezjb <icodezjb@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants