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

Make ED of zero (kind of) work #13655

Merged
merged 13 commits into from
Mar 24, 2023
Merged

Make ED of zero (kind of) work #13655

merged 13 commits into from
Mar 24, 2023

Conversation

gavofyork
Copy link
Member

@gavofyork gavofyork commented Mar 20, 2023

Make things work when existential deposit is zero. Note this means that once an account has a balance at all it will always have a provider, so naturally represents an account spam vector.

The problem of having an ED of zero is that the balances provider "always" exists because the account begins with a balance which satisfies >= ED. Therefore it never actually increments the provider ref, assuming it has already been incremented at some point in the past when the balance became >= ED.

So now we increment it either when it becomes newly >= ED or when it should already be incremented but there are zero provider refs.

When ED > 0, the latter condition can never be true since if balance >= ED is satisfied there will already be a provider ref in place.

@gavofyork gavofyork changed the title Minimum of 1 for ED Make ED balance of zero work Mar 20, 2023
@gavofyork gavofyork added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited and removed D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Mar 20, 2023
@gavofyork gavofyork changed the title Make ED balance of zero work Make ED of zero (kind of) work Mar 20, 2023
@ruseinov ruseinov self-requested a review March 20, 2023 17:50
Copy link
Contributor

@ruseinov ruseinov 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, just out of curiosity: why not enforce ED > 0 if it’s shooting us in the leg otherwise?

@gavofyork
Copy link
Member Author

There's at least one team who is unfortunately relying on it.

@gavofyork
Copy link
Member Author

Altered the PR to ban an ED of zero unless you enable it with zero_ed feature.

@gavofyork gavofyork requested a review from ggwpez March 21, 2023 13:36
@gavofyork
Copy link
Member Author

@ggwpez could you quickly re-review?

frame/balances/Cargo.toml Outdated Show resolved Hide resolved
frame/balances/src/lib.rs Outdated Show resolved Hide resolved
frame/balances/src/lib.rs Outdated Show resolved Hide resolved
frame/balances/src/lib.rs Outdated Show resolved Hide resolved
gavofyork and others added 4 commits March 21, 2023 17:40
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@gilescope
Copy link
Contributor

Could name the feature unsafe_zero_ed? Very hard for someone to enable it and then cry about it later with a name like that.

@gavofyork
Copy link
Member Author

bot rebase

@paritytech-processbot
Copy link

Rebased

@gavofyork gavofyork merged commit 770d237 into master Mar 24, 2023
@gavofyork gavofyork deleted the gav-ed-one branch March 24, 2023 17:48
breathx pushed a commit to gear-tech/substrate that referenced this pull request Apr 22, 2023
* Minimum of 1 for ED

* Avoid need for ED to be minimum one

* Docs

* Ban ED of zero unless feature enabled

* use integrity_test

* Docs

* Cleanup

* Update frame/balances/Cargo.toml

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update frame/balances/src/lib.rs

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update frame/balances/src/lib.rs

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Ensure dodgy code is disabled by default

* zero_ed -> insecure_zero_ed

---------

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: parity-processbot <>
@louismerlin louismerlin 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 May 4, 2023
@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-41-v0-9-42/2828/1

nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
* Minimum of 1 for ED

* Avoid need for ED to be minimum one

* Docs

* Ban ED of zero unless feature enabled

* use integrity_test

* Docs

* Cleanup

* Update frame/balances/Cargo.toml

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update frame/balances/src/lib.rs

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update frame/balances/src/lib.rs

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Ensure dodgy code is disabled by default

* zero_ed -> insecure_zero_ed

---------

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: parity-processbot <>
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. B0-silent Changes should not be mentioned in any release notes 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
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants