Original PR https://github.com/paritytech/substrate/pull/14655
---
Partial https://github.com/paritytech/polkadot-sdk/issues/225
- [x] Adds conformance tests for Unbalanced
- [x] Adds conformance tests for Balanced
- Several minor fixes to fungible default implementations and the
Balances pallet
- [x] `Unbalanced::decrease_balance` can reap account when
`Preservation` is `Preserve`
- [x] `Balanced::pair` can return pairs of imbalances which do not
cancel each other out
- [x] Balances pallet `active_issuance` 'underflow'
- [x] Refactors the conformance test file structure to match the
fungible file structure: tests for traits in regular.rs go into a test
file named regular.rs, tests for traits in freezes.rs go into a test
file named freezes.rs, etc.
- [x] Improve doc comments
- [x] Simplify macros
## Fixes
### `Unbalanced::decrease_balance` can reap account when called with
`Preservation::Preserve`
There is a potential issue in the default implementation of
`Unbalanced::decrease_balance`. The implementation can delete an account
even when it is called with `preservation: Preservation::Preserve`. This
seems to contradict the documentation of `Preservation::Preserve`:
```rust
/// The account may not be killed and our provider reference must remain (in the context of
/// tokens, this means that the account may not be dusted).
Preserve,
```
I updated `Unbalanced::decrease_balance` to return
`Err(TokenError::BelowMinimum)` when a withdrawal would cause the
account to be reaped and `preservation: Preservation::Preserve`.
- [ ] TODO Confirm with @gavofyork that this is correct behavior
Test for this behavior:
https://github.com/paritytech/polkadot-sdk/blob/e5c876dd6b59e2b7dbacaa4538cb42c802db3730/substrate/frame/support/src/traits/tokens/fungible/conformance_tests/regular.rs#L912-L937
### `Balanced::pair` returning non-canceling pairs
`Balanced::pair` is supposed to create a pair of imbalances that cancel
each other out. However this is not the case when the method is called
with an amount greater than the total supply.
In the existing default implementation, `Balanced::pair` creates a pair
by first rescinding the balance, creating `Debt`, and then issuing the
balance, creating `Credit`.
When creating `Debt`, if the amount to create exceeds the
`total_supply`, `total_supply` units of `Debt` are created *instead* of
`amount` units of `Debt`. This can lead to non-canceling amount of
`Credit` and `Debt` being created.
To address this, I create the credit and debt directly in the method
instead of calling `issue` and `rescind`.
Test for this behavior:
https://github.com/paritytech/polkadot-sdk/blob/e5c876dd6b59e2b7dbacaa4538cb42c802db3730/substrate/frame/support/src/traits/tokens/fungible/conformance_tests/regular.rs#L1323-L1346
### `Balances` pallet `active_issuance` 'underflow'
This PR resolves an issue in the `Balances` pallet that can lead to odd
behavior of `active_issuance`.
Currently, the Balances pallet doesn't check if `InactiveIssuance`
remains less than or equal to `TotalIssuance` when supply is
deactivated. This allows `InactiveIssuance` to be greater than
`TotalIssuance`, which can result in unexpected behavior from the
perspective of the fungible API.
`active_issuance` is derived from
`TotalIssuance.saturating_sub(InactiveIssuance)`.
If an `amount` is deactivated that causes `InactiveIssuance` to become
greater TotalIssuance, `active_issuance` will return 0. However once in
that state, reactivating an amount will not increase `active_issuance`
by the reactivated `amount` as expected.
Consider this test where the last assertion would fail due to this
issue:
https://github.com/paritytech/polkadot-sdk/blob/e5c876dd6b59e2b7dbacaa4538cb42c802db3730/substrate/frame/support/src/traits/tokens/fungible/conformance_tests/regular.rs#L1036-L1071
To address this, I've modified the `deactivate` function to ensure
`InactiveIssuance` never surpasses `TotalIssuance`.
---------
Co-authored-by: Muharem <ismailov.m.h@gmail.com>
Changes:
- Change the fungible(s) logic to treat a self-transfer as No-OP (as
long as all pre-checks pass).
Note that the self-transfer case will not emit an event since no state
was changed.
---------
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
closes https://github.com/paritytech/polkadot-sdk/issues/1882
## Breaking Changes
This PR introduces a new item to `pallet_balances::Config`:
```diff
trait Config {
++ type RuntimeFreezeReasons;
}
```
This value is only used to check it against `type MaxFreeze`. A similar
check has been added for `MaxHolds` against `RuntimeHoldReasons`, which
is already given to `pallet_balances`.
In all contexts, you should pass the real `RuntimeFreezeReasons`
generated by `construct_runtime` to `type RuntimeFreezeReasons`. Passing
`()` would also work, but it would imply that the runtime uses no
freezes at all.
---------
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
* replace Index by Nonce
* replace Index by Nonce
* replace Index by Nonce
* replace Index by Nonce
* replace Index by Nonce
* wip
* remove index in lieu of nonce
* wip
* remove accountnonce in lieu of nonce
* add minor improvement
* rebase and merge conflicts
* HoldReason: Improve usage
`HoldReason` was switched recently to use the `composite_enum` attribute that will merge the enums
from all pallets in the runtime to `RuntimeHoldReason`. `pallet-nis` was still requiring that the
variant was passed as constant to call `hold`. The proper implementation is to use the `HoldReason`
from inside the pallet directly when calling `hold`. This is done by adding a `RuntimeHoldReason` as
type to the `Config` trait and requiring that `Currency` is using the same reason. Besides that the
pr changes the name `HoldIdentifier` in `pallet_balances::Config` to `RuntimeHoldReason`.
* Update frame/nis/src/lib.rs
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
* Review comment
* Fixes
---------
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
* Fix: Incorrect implementation of can_reserve check
* Fix: Incorrect migration of consumer counting for existing accounts with frozen amounts
* Fix: Inconsistent implementation between assets can_deposit and new_account
* Fixes
* Fixes
* Another fix
* Update tests.rs
* Update fungible_tests.rs
* Use `can_accrue_consumers` in the body of `can_inc_consumer`
---------
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: parity-processbot <>
* 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 <>
Oliver Tale-Yazdi