Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NPoS] Fix for Reward Deficit in the pool #1255

Merged
merged 116 commits into from
Sep 29, 2023
Merged

Conversation

Ank4n
Copy link
Contributor

@Ank4n Ank4n commented Aug 29, 2023

closes #158.
partially addresses #226.

Instead of fragile calculation of current balance by looking at free balance - ED, Nomination Pool now freezes ED in the pool reward account to restrict an account from going below minimum balance. This also has a nice side effect that if ED changes, we know how much is the imbalance in ED frozen in the pool and the current required ED. A pool operator can diligently top up the pool with the deficit in ED or vice versa, withdraw the excess they transferred to the pool.

Notable changes

  • New call adjust_pool_deposit: Allows to top up the deficit or withdraw the excess deposited funds to the pool.
  • Uses Fungible trait (instead of Currency trait). Since NP was not doing any locking/reserving previously, no migration is needed for this.
  • One time migration of freezing ED from each of the existing pools (not very PoV friendly but fine for relay chain).

@Ank4n Ank4n force-pushed the ankan/fix-np-reward-deficit branch from cd98a52 to 289d80b Compare August 29, 2023 12:07
@Ank4n Ank4n changed the title Fix for Reward Deficit in the pool [NPoS] Fix for Reward Deficit in the pool Aug 29, 2023
@Ank4n Ank4n force-pushed the ankan/fix-np-reward-deficit branch from 289d80b to 88f5139 Compare August 29, 2023 15:22
@Ank4n
Copy link
Contributor Author

Ank4n commented Sep 2, 2023

bot rebase

@Ank4n Ank4n added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Sep 7, 2023
athei and others added 12 commits September 8, 2023 20:37
* contracts: Update to wasmi 0.31

* ".git/.scripts/commands/bench/bench.sh" --subcommand=pallet --runtime=dev --target_dir=substrate --pallet=pallet_contracts

---------

Co-authored-by: command-bot <>
* substrate: chain-spec paths corrected in zombienet tests

* fix chain-spec path in cumulus test

* disable beefy on validator

---------

Co-authored-by: Javier Viola <javier@parity.io>
* feat: add futures api to `TransactionPool`

* fix clippy
Bumps [proc-macro-warning](https://github.com/ggwpez/proc-macro-warning) from 0.4.1 to 0.4.2.
- [Release notes](https://github.com/ggwpez/proc-macro-warning/releases)
- [Commits](ggwpez/proc-macro-warning@v0.4.1...v0.4.2)

---
updated-dependencies:
- dependency-name: proc-macro-warning
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps the known_good_semver group with 1 update: [syn](https://github.com/dtolnay/syn).

- [Release notes](https://github.com/dtolnay/syn/releases)
- [Commits](dtolnay/syn@2.0.29...2.0.31)

---
updated-dependencies:
- dependency-name: syn
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: known_good_semver
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Include bridges into fmt

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix some authors

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
* Add markdown linting

- add linter default rules
- adapt rules to current code
- fix the code for linting to pass
- add CI check

fix #1243

* Fix markdown for Substrate
* Fix tooling install
* Fix workflow
* Add documentation
* Remove trailing spaces
* Update .github/.markdownlint.yaml

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
* Fix mangled markdown/lists
* Fix captalization issues on known words
* bring back proper init

* refactor block cycle

* ".git/.scripts/commands/fmt/fmt.sh"

* Update cumulus/xcm/xcm-emulator/src/lib.rs

Co-authored-by: Squirrel <gilescope@gmail.com>

---------

Co-authored-by: command-bot <>
Co-authored-by: Giles Cope <gilescope@gmail.com>
* Delete stale adoc files

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Convert adoc to md

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Add adoc to gitignore

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Delete more random unmaintained files

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Markdown lint

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@command-bot
Copy link

command-bot bot commented Sep 22, 2023

@Ank4n https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3769806 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=runtime --runtime=westend --target_dir=polkadot --pallet=pallet_nomination_pools. Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 1-9ce7929f-dae4-41b3-b1e0-88925c6cb35d to cancel this command or bot cancel to cancel all commands in this pull request.

@Ank4n Ank4n added T2-pallets This PR/Issue is related to a particular pallet. and removed T1-FRAME This PR/Issue is related to core FRAME, the framework. labels Sep 22, 2023
…e=westend --target_dir=polkadot --pallet=pallet_nomination_pools
@command-bot
Copy link

command-bot bot commented Sep 22, 2023

@Ank4n Command "$PIPELINE_SCRIPTS_DIR/commands/bench/bench.sh" --subcommand=runtime --runtime=westend --target_dir=polkadot --pallet=pallet_nomination_pools has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3769806 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3769806/artifacts/download.

@Ank4n
Copy link
Contributor Author

Ank4n commented Sep 22, 2023

try-runtime on-runtime-upgrade results

Westend

[2023-09-22T13:18:07Z INFO  frame_support::migrations] Running NominationPools VersionedOnRuntimeUpgrade: version 5 to 6.
[2023-09-22T13:18:07Z INFO  runtime::nomination-pools] [17572294] 🏊‍♂️ Freezing ED succeeded for 216 pools

Kusama

[2023-09-22T13:07:59Z INFO  frame_support::migrations] Running NominationPools VersionedOnRuntimeUpgrade: version 5 to 6.
[2023-09-22T13:07:59Z INFO  runtime::nomination-pools] [19791330] 🏊‍♂️ Freezing ED succeeded for 154 pools

Polkadot

[2023-09-22T13:12:26Z INFO  frame_support::migrations] Running NominationPools VersionedOnRuntimeUpgrade: version 5 to 6.
[2023-09-22T13:12:26Z INFO  runtime::nomination-pools] [17402560] 🏊‍♂️ Freezing ED succeeded for 147 pools

@Ank4n
Copy link
Contributor Author

Ank4n commented Sep 22, 2023

It might be much simpler to just increment providers and remove the need for pool accounts to need ED. I will try the other approach and keep this PR parked for now.

@Ank4n Ank4n changed the title [NPoS] Fix for Reward Deficit in the pool [NPoS] [DNM] Fix for Reward Deficit in the pool Sep 22, 2023
@Ank4n Ank4n marked this pull request as draft September 22, 2023 14:51
@Ank4n Ank4n marked this pull request as ready for review September 26, 2023 19:51
@Ank4n
Copy link
Contributor Author

Ank4n commented Sep 26, 2023

It might be much simpler to just increment providers and remove the need for pool accounts to need ED. I will try the other approach and keep this PR parked for now.

Just incrementing provider can still mean an account can be dusted. Better we maintain the ED on the account.
Ref:

let maybe_dust = if account.free < ed && account.reserved.is_zero() {

@Ank4n Ank4n changed the title [NPoS] [DNM] Fix for Reward Deficit in the pool [NPoS] Fix for Reward Deficit in the pool Sep 27, 2023
@Ank4n Ank4n merged commit f820dc0 into master Sep 29, 2023
103 of 106 checks passed
@Ank4n Ank4n deleted the ankan/fix-np-reward-deficit branch September 29, 2023 15:48
ordian added a commit that referenced this pull request Oct 3, 2023
* master: (24 commits)
  Init System Parachain storage versions and add migration check jobs to CI (#1344)
  no-bound derives: Use absolute path for `core` (#1763)
  migrate alliance, fast-unstake and bags list to use derive-impl (#1636)
  Tvl pool staking (#1322)
  improve service error (#1734)
  frame-support: `RuntimeDebug\Eq\PartialEq` impls for `Imbalance` (#1717)
  Point documentation links to monorepo (#1741)
  [NPoS] Fix for Reward Deficit in the pool (#1255)
  Move import queue from `ChainSync` to `SyncingEngine` (#1736)
  Enable mocking contracts (#1331)
  Revert "fix(review-bot): pull secrets from `master` environment" (#1748)
  Remove kusama and polkadot runtime crates (#1731)
  Use `Extensions` to register offchain worker custom extensions (#1719)
  [RPC-Spec-V2] chainHead: use integer for block index and adjust RuntimeVersion JSON format (#1666)
  fix(review-bot): pull secrets from `master` environment (#1745)
  Fix `subkey inspect` output text padding (#1744)
  archive: Implement height, hashByHeight and call (#1582)
  rpc/client: Propagate `rpc_methods` method to reported methods (#1713)
  rococo-runtime: `RococoGenesisExt` removed (#1490)
  PVF: more filesystem sandboxing (#1373)
  ...
ordian added a commit that referenced this pull request Oct 10, 2023
* tsv-disabling-node-side: (24 commits)
  Init System Parachain storage versions and add migration check jobs to CI (#1344)
  no-bound derives: Use absolute path for `core` (#1763)
  migrate alliance, fast-unstake and bags list to use derive-impl (#1636)
  Tvl pool staking (#1322)
  improve service error (#1734)
  frame-support: `RuntimeDebug\Eq\PartialEq` impls for `Imbalance` (#1717)
  Point documentation links to monorepo (#1741)
  [NPoS] Fix for Reward Deficit in the pool (#1255)
  Move import queue from `ChainSync` to `SyncingEngine` (#1736)
  Enable mocking contracts (#1331)
  Revert "fix(review-bot): pull secrets from `master` environment" (#1748)
  Remove kusama and polkadot runtime crates (#1731)
  Use `Extensions` to register offchain worker custom extensions (#1719)
  [RPC-Spec-V2] chainHead: use integer for block index and adjust RuntimeVersion JSON format (#1666)
  fix(review-bot): pull secrets from `master` environment (#1745)
  Fix `subkey inspect` output text padding (#1744)
  archive: Implement height, hashByHeight and call (#1582)
  rpc/client: Propagate `rpc_methods` method to reported methods (#1713)
  rococo-runtime: `RococoGenesisExt` removed (#1490)
  PVF: more filesystem sandboxing (#1373)
  ...
svyatonik added a commit to svyatonik/runtimes that referenced this pull request Nov 3, 2023
svyatonik added a commit to svyatonik/runtimes that referenced this pull request Nov 9, 2023
svyatonik added a commit to svyatonik/runtimes that referenced this pull request Nov 22, 2023
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
closes paritytech#158.
partially addresses
paritytech#226.

Instead of fragile calculation of current balance by looking at `free
balance - ED`, Nomination Pool now freezes ED in the pool reward account
to restrict an account from going below minimum balance. This also has a
nice side effect that if ED changes, we know how much is the imbalance
in ED frozen in the pool and the current required ED. A pool operator
can diligently top up the pool with the deficit in ED or vice versa,
withdraw the excess they transferred to the pool.

## Notable changes
- New call `adjust_pool_deposit`: Allows to top up the deficit or
withdraw the excess deposited funds to the pool.
- Uses Fungible trait (instead of Currency trait). Since NP was not
doing any locking/reserving previously, no migration is needed for this.
- One time migration of freezing ED from each of the existing pools (not
very PoV friendly but fine for relay chain).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T2-pallets This PR/Issue is related to a particular pallet.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

nomination-pools try-state hook fails on Kusama