Tvl pool staking

[PieWol]

What does this PR do?

• Introduced the TotalValueLocked storage for nomination-pools.
• introduced a slashing api in mock.rs
• additional test for tracking a slashing event towards a pool without sub-pools
• migration for the nomination-pools (V6 to V7) with VersionedMigration

Why are these changes needed?
this is the continuation of the work by @kianenigma in this PR

How were these changes implemented and what do they affect?

• It's an extra StorageValue that's modified whenever funds flow in or out of staking for any of the bonded_account of BondedPools
• The PoolSlashedevent is now emitted even when no SubPools are found

Closes #155
KSM: HHEEgVzcqL3kCXgsxSfJMbsTy8dxoTctuXtpY94n4s8F4pS

[PieWol]

The storage migration works with try-runtime on a recent Kusama snapshot:

[2023-09-03T17:39:55Z INFO  runtime::nomination-pools] [19343161] 🏊‍♂️ Upgraded 149 pools with a TVL of 61550133549019336
[2023-09-03T17:39:56Z INFO  try-runtime::cli] TryRuntime_on_runtime_upgrade executed without errors.

[kianenigma]

Thanks for the effort so far!

I have two broad questions:

1. Your comments and the try_state code indicate that the

TVL must be equal to or less than the total balance of all PoolMembers

But it is not clear when this inaccuracy can exist? the TVL, since it is tracking slashes as well, should always be accurate from what I can see.

2. You have slightly changed the definition of TVL in this PR. Let me elaborate a bit on how pools work: Pools take some balance from the user, transfer it to the pool account, and issue/burn equivalent points in return. You can imagine it as a bridge: You lock DOTs, and you get points in return. The only functions in which this bridging happens is the brun and issue.

DOT <-- burn/issue --> Points

The point of TVL in my mind was to track this process. It should track the total amount of DOTs that have been exchanged for points in the pool pallet.

If you agree with this, while your approach is also likely equivalent, it is different from what I have in mind. You don't need to query staking before and after unbond. You only need to track successful calls to burn and issue. This is more or less what the original PR was doing.

cc @Ank4n @gpestana would be good to get your eyes on this as well.

[PieWol]

For my understanding the perfect definition of value locked is exactly that what I would get back as a the amount when quering T::Staking::total_stake(). It's the actively bonded and currently unbonding funds. So the value that is currently locked

Thus the TVL of the nom-pools should just be in exact sync with the sum of all bonded_accounts of all nom-pools. Initially I also thought that this would be achievable by just doing operations on issue and dissolve but due to the fact that only active stake is read directly from staking but the UnbondingPool's are stored individuall y inside the pallet this doesn't always stay in check due to the one extrinsic called pool_withdraw_unbonded. This fn removes all unlocking chunks in T::Staking for the bonded_account permissionlessly.

With the effect of the T::Staking::total_stake() yielding afterwards less value for the bonded_account than you would expect when querying the total_balance of all PoolMembers because the nom-pool included UnbondingPools are not updated.

This is the explanation why I did it different. I was just happy this way as I could have everything stay in check with my opinionated definition of TVL that T::Staking already provides that I could also use as a way of verifying the TVL inside of do_try_state. Furthermore this is an easy and also exact migration.

This should also answer the first question. As total_balance() it's possible for the sum of total_balance of all PoolMembers to be higher than the T::Staking::total_stake() for the BondedPool.

I'm completely fine to just do it as you wish but I quickly wanted to write up my thoughts behind the change of concept. @kianenigma

[PieWol]

I do understand though, that there is also the way of looking at the TVL as the TVL in this very pallet.

[kianenigma]

Initially I also thought that this would be achievable by just doing operations on issue and dissolve but due to the fact that only active stake is read directly from staking but the UnbondingPool's are stored individually inside the pallet this doesn't always stay in check due to the one extrinsic called pool_withdraw_unbonded

Okay, I see a good point here and I will rephrase: The TVL that I suggested only tracks active_stake that is in the pallet, because once you unbond, it is already out of the TVL. Your definition has this as well.

If your approach was significantly more complicated, I would perhaps argue that we should look at the scope where this is used and see which one is more suitable, but your approach is also fine. The context where we need this is as the two scenarios below:

1. We want to mark the amount of funds that is locked in Pools, both active and unbonding, as T::Currency::deactivate, which will in turn make them effectless in the OpenGov curves.
2. Possibly, UIs can also benefit from this.

With that in mind, I am convinced that your definition of total value locked is better.

[PieWol]

@Ank4n @kianenigma I have adressed the latest comments, merged master recently and improved the CI failures. Are we ready to merge? 👀

[Ank4n]

bot fmt

[command-bot]

@Ank4n https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3859788 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". 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-52ded444-0e0f-47cf-b174-3c5a4cd7e17f to cancel this command or bot cancel to cancel all commands in this pull request.

[command-bot]

@Ank4n Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3859788 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/3859788/artifacts/download.

[kianenigma]

/tip medium

[substrate-tip-bot]

@PieWol Contributor did not properly post their account address.

Make sure the pull request description (or user bio) has: "{network} address: {address}".

[kianenigma]

/tip medium

[substrate-tip-bot]

@kianenigma A medium (5 KSM) tip was successfully submitted for @PieWol (HHEEgVzcqL3kCXgsxSfJMbsTy8dxoTctuXtpY94n4s8F4pS on kusama).

https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fkusama-rpc.polkadot.io#/referenda