Commit Graph

2 Commits

Author SHA1 Message Date
Liam Aharon 46090ff114 Unbalanced and Balanced fungible conformance tests, and fungible fixes (#1296)
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>
2024-01-15 12:30:51 +00:00
Liam Aharon 9c53fd7299 fungible conformance tests: Inspect and Mutate (#13852)
* typo

* - create test files for each fungile trait
- begin implementing tests for the Inspect trait

* wrap inspect tests in a macro

* first run of mutate tests

* move test implementation out of ballances

* make tests more generic

* transfer tests

* combine inspect and mutate tests

* set balance failing tests

* can_deposit tests

* can_withdraw tests

* test reducible_balance

* remove balanced stub

* revert set_balance return val fix

* typo

* macro and dust trap tests

* disable test when it doesn't make sense

* remove debug comment

* reduce macro boilerplate

* improved var naming

* improve variable naming

* remove redundant comment

* remove placeholder tests

* remove placeholder tests

* simplify macro

* Update frame/balances/src/tests/fungible_conformance_tests.rs

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* use Balance from T

* fix copyright

* add test doc comments

* improve test naming

* clippy

* fix rustdoc errors

* fix rustdoc

* improve macro

* improve variable naming

* remove redundant comment

* use path

---------

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
2023-05-05 02:21:40 +10:00