From 78f8f708781da6fd340ef2a1fc288060a35295ad Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Thu, 10 Nov 2022 02:34:00 +0000 Subject: [PATCH] More testing and fuzzing and docs for pools (#12624) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * move pools fuzzing to hongfuzz * merge more small fixes * fix all tests * Update frame/nomination-pools/fuzzer/src/call.rs Co-authored-by: Gonçalo Pestana * remove transactional * fmt * fix CI * fmt * fix build again * fix CI Co-authored-by: Gonçalo Pestana --- Cargo.lock | 33 +- Cargo.toml | 1 + frame/balances/src/lib.rs | 2 +- frame/nomination-pools/Cargo.toml | 7 +- frame/nomination-pools/fuzzer/Cargo.toml | 33 ++ frame/nomination-pools/fuzzer/src/call.rs | 353 ++++++++++++++++++++ frame/nomination-pools/src/lib.rs | 167 +++++----- frame/nomination-pools/src/mock.rs | 23 +- frame/nomination-pools/src/tests.rs | 379 +++------------------- 9 files changed, 562 insertions(+), 436 deletions(-) create mode 100644 frame/nomination-pools/fuzzer/Cargo.toml create mode 100644 frame/nomination-pools/fuzzer/src/call.rs diff --git a/Cargo.lock b/Cargo.lock index 9fc220e099a5c..a13768a64a4ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2902,13 +2902,14 @@ dependencies = [ [[package]] name = "honggfuzz" -version = "0.5.54" +version = "0.5.55" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bea09577d948a98a5f59b7c891e274c4fb35ad52f67782b3d0cb53b9c05301f1" +checksum = "848e9c511092e0daa0a35a63e8e6e475a3e8f870741448b9f6028d69b142f18e" dependencies = [ "arbitrary", "lazy_static", - "memmap", + "memmap2", + "rustc_version 0.4.0", ] [[package]] @@ -4135,16 +4136,6 @@ dependencies = [ "rustix", ] -[[package]] -name = "memmap" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6585fd95e7bb50d6cc31e20d4cf9afb4e2ba16c5846fc76793f11218da9c475b" -dependencies = [ - "libc", - "winapi", -] - [[package]] name = "memmap2" version = "0.5.0" @@ -5735,7 +5726,6 @@ dependencies = [ "log", "pallet-balances", "parity-scale-codec", - "rand 0.8.5", "scale-info", "sp-core", "sp-io", @@ -5769,6 +5759,21 @@ dependencies = [ "sp-std", ] +[[package]] +name = "pallet-nomination-pools-fuzzer" +version = "2.0.0" +dependencies = [ + "frame-support", + "frame-system", + "honggfuzz", + "log", + "pallet-nomination-pools", + "rand 0.8.5", + "sp-io", + "sp-runtime", + "sp-tracing", +] + [[package]] name = "pallet-nomination-pools-runtime-api" version = "1.0.0-dev" diff --git a/Cargo.toml b/Cargo.toml index 45aa5f9854d25..4ba2663cdb409 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -116,6 +116,7 @@ members = [ "frame/preimage", "frame/proxy", "frame/nomination-pools", + "frame/nomination-pools/fuzzer", "frame/nomination-pools/benchmarking", "frame/nomination-pools/test-staking", "frame/nomination-pools/runtime-api", diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index 67976cbaf06ac..d3085152eba6c 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -477,7 +477,7 @@ pub mod pallet { VestingBalance, /// Account liquidity restrictions prevent withdrawal LiquidityRestrictions, - /// Balance too low to send value + /// Balance too low to send value. InsufficientBalance, /// Value too low to create account due to existential deposit ExistentialDeposit, diff --git a/frame/nomination-pools/Cargo.toml b/frame/nomination-pools/Cargo.toml index 459ee5f440a12..2db0b234b726d 100644 --- a/frame/nomination-pools/Cargo.toml +++ b/frame/nomination-pools/Cargo.toml @@ -26,13 +26,17 @@ sp-core = { version = "6.0.0", default-features = false, path = "../../primitive sp-io = { version = "6.0.0", default-features = false, path = "../../primitives/io" } log = { version = "0.4.0", default-features = false } +# Optional: usef for testing and/or fuzzing +pallet-balances = { version = "4.0.0-dev", path = "../balances", optional = true } +sp-tracing = { version = "5.0.0", path = "../../primitives/tracing", optional = true } + [dev-dependencies] pallet-balances = { version = "4.0.0-dev", path = "../balances" } sp-tracing = { version = "5.0.0", path = "../../primitives/tracing" } -rand = { version = "0.8.5", features = ["small_rng"] } [features] default = ["std"] +fuzzing = ["pallet-balances", "sp-tracing"] std = [ "codec/std", "scale-info/std", @@ -51,4 +55,3 @@ runtime-benchmarks = [ try-runtime = [ "frame-support/try-runtime" ] -fuzz-test = [] diff --git a/frame/nomination-pools/fuzzer/Cargo.toml b/frame/nomination-pools/fuzzer/Cargo.toml new file mode 100644 index 0000000000000..7dde8733e3f60 --- /dev/null +++ b/frame/nomination-pools/fuzzer/Cargo.toml @@ -0,0 +1,33 @@ +[package] +name = "pallet-nomination-pools-fuzzer" +version = "2.0.0" +authors = ["Parity Technologies "] +edition = "2021" +license = "Apache-2.0" +homepage = "https://substrate.io" +repository = "https://github.com/paritytech/substrate/" +description = "Fuzzer for fixed point arithmetic primitives." +documentation = "https://docs.rs/sp-arithmetic-fuzzer" +publish = false + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] + +[dependencies] +honggfuzz = "0.5.54" + +pallet-nomination-pools = { path = "..", features = ["fuzzing"] } + +frame-system = { path = "../../system" } +frame-support = { path = "../../support" } + +sp-runtime = { path = "../../../primitives/runtime" } +sp-io = { path = "../../../primitives/io" } +sp-tracing = { path = "../../../primitives/tracing" } + +rand = { version = "0.8.5", features = ["small_rng"] } +log = "0.4.17" + +[[bin]] +name = "call" +path = "src/call.rs" diff --git a/frame/nomination-pools/fuzzer/src/call.rs b/frame/nomination-pools/fuzzer/src/call.rs new file mode 100644 index 0000000000000..b07903609e8ab --- /dev/null +++ b/frame/nomination-pools/fuzzer/src/call.rs @@ -0,0 +1,353 @@ +// This file is part of Substrate. + +// Copyright (C) 2019-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! # Running +//! Running this fuzzer can be done with `cargo hfuzz run call`. `honggfuzz` CLI +//! options can be used by setting `HFUZZ_RUN_ARGS`, such as `-n 4` to use 4 threads. +//! +//! # Debugging a panic +//! Once a panic is found, it can be debugged with +//! `cargo hfuzz run-debug per_thing_rational hfuzz_workspace/call/*.fuzz`. + +use frame_support::{ + assert_ok, + traits::{Currency, GetCallName, UnfilteredDispatchable}, +}; +use honggfuzz::fuzz; +use pallet_nomination_pools::{ + log, + mock::*, + pallet as pools, + pallet::{BondedPools, Call as PoolsCall, Event as PoolsEvents, PoolMembers}, + BondExtra, BondedPool, LastPoolId, MaxPoolMembers, MaxPoolMembersPerPool, MaxPools, + MinCreateBond, MinJoinBond, PoolId, +}; +use rand::{seq::SliceRandom, Rng}; +use sp_runtime::{assert_eq_error_rate, Perquintill}; + +const ERA: BlockNumber = 1000; +const MAX_ED_MULTIPLE: Balance = 10_000; +const MIN_ED_MULTIPLE: Balance = 10; + +// not quite elegant, just to make it available in random_signed_origin. +const REWARD_AGENT_ACCOUNT: AccountId = 42; + +/// Grab random accounts, either known ones, or new ones. +fn random_signed_origin(rng: &mut R) -> (RuntimeOrigin, AccountId) { + let count = PoolMembers::::count(); + if rng.gen::() && count > 0 { + // take an existing account. + let skip = rng.gen_range(0..count as usize); + + // this is tricky: the account might be our reward agent, which we never want to be + // randomly chosen here. Try another one, or, if it is only our agent, return a random + // one nonetheless. + let candidate = PoolMembers::::iter_keys().skip(skip).take(1).next().unwrap(); + let acc = + if candidate == REWARD_AGENT_ACCOUNT { rng.gen::() } else { candidate }; + + (RuntimeOrigin::signed(acc), acc) + } else { + // create a new account + let acc = rng.gen::(); + (RuntimeOrigin::signed(acc), acc) + } +} + +fn random_ed_multiple(rng: &mut R) -> Balance { + let multiple = rng.gen_range(MIN_ED_MULTIPLE..MAX_ED_MULTIPLE); + ExistentialDeposit::get() * multiple +} + +fn fund_account(rng: &mut R, account: &AccountId) { + let target_amount = random_ed_multiple(rng); + if let Some(top_up) = target_amount.checked_sub(Balances::free_balance(account)) { + let _ = Balances::deposit_creating(account, top_up); + } + assert!(Balances::free_balance(account) >= target_amount); +} + +fn random_existing_pool(mut rng: &mut R) -> Option { + BondedPools::::iter_keys().collect::>().choose(&mut rng).map(|x| *x) +} + +fn random_call(mut rng: &mut R) -> (pools::Call, RuntimeOrigin) { + let op = rng.gen::(); + let mut op_count = as GetCallName>::get_call_names().len(); + // Exclude set_state, set_metadata, set_configs, update_roles and chill. + op_count -= 5; + + match op % op_count { + 0 => { + // join + let pool_id = random_existing_pool(&mut rng).unwrap_or_default(); + let (origin, who) = random_signed_origin(&mut rng); + fund_account(&mut rng, &who); + let amount = random_ed_multiple(&mut rng); + (PoolsCall::::join { amount, pool_id }, origin) + }, + 1 => { + // bond_extra + let (origin, who) = random_signed_origin(&mut rng); + let extra = if rng.gen::() { + BondExtra::Rewards + } else { + fund_account(&mut rng, &who); + let amount = random_ed_multiple(&mut rng); + BondExtra::FreeBalance(amount) + }; + (PoolsCall::::bond_extra { extra }, origin) + }, + 2 => { + // claim_payout + let (origin, _) = random_signed_origin(&mut rng); + (PoolsCall::::claim_payout {}, origin) + }, + 3 => { + // unbond + let (origin, who) = random_signed_origin(&mut rng); + let amount = random_ed_multiple(&mut rng); + (PoolsCall::::unbond { member_account: who, unbonding_points: amount }, origin) + }, + 4 => { + // pool_withdraw_unbonded + let pool_id = random_existing_pool(&mut rng).unwrap_or_default(); + let (origin, _) = random_signed_origin(&mut rng); + (PoolsCall::::pool_withdraw_unbonded { pool_id, num_slashing_spans: 0 }, origin) + }, + 5 => { + // withdraw_unbonded + let (origin, who) = random_signed_origin(&mut rng); + ( + PoolsCall::::withdraw_unbonded { member_account: who, num_slashing_spans: 0 }, + origin, + ) + }, + 6 => { + // create + let (origin, who) = random_signed_origin(&mut rng); + let amount = random_ed_multiple(&mut rng); + fund_account(&mut rng, &who); + let root = who; + let state_toggler = who; + let nominator = who; + (PoolsCall::::create { amount, root, state_toggler, nominator }, origin) + }, + 7 => { + // nominate + let (origin, _) = random_signed_origin(&mut rng); + let pool_id = random_existing_pool(&mut rng).unwrap_or_default(); + let validators = Default::default(); + (PoolsCall::::nominate { pool_id, validators }, origin) + }, + _ => unreachable!(), + } +} + +#[derive(Default)] +struct RewardAgent { + who: AccountId, + pool_id: Option, + expected_reward: Balance, +} + +// TODO: inject some slashes into the game. +impl RewardAgent { + fn new(who: AccountId) -> Self { + Self { who, ..Default::default() } + } + + fn join(&mut self) { + if self.pool_id.is_some() { + return + } + let pool_id = LastPoolId::::get(); + let amount = 10 * ExistentialDeposit::get(); + let origin = RuntimeOrigin::signed(self.who); + let _ = Balances::deposit_creating(&self.who, 10 * amount); + self.pool_id = Some(pool_id); + log::info!(target: "reward-agent", "🤖 reward agent joining in {} with {}", pool_id, amount); + assert_ok!(PoolsCall::join:: { amount, pool_id }.dispatch_bypass_filter(origin)); + } + + fn claim_payout(&mut self) { + // 10 era later, we claim our payout. We expect our income to be roughly what we + // calculated. + if !PoolMembers::::contains_key(&self.who) { + log!(warn, "reward agent is not in the pool yet, cannot claim"); + return + } + let pre = Balances::free_balance(&42); + let origin = RuntimeOrigin::signed(42); + assert_ok!(PoolsCall::::claim_payout {}.dispatch_bypass_filter(origin)); + let post = Balances::free_balance(&42); + + let income = post - pre; + log::info!( + target: "reward-agent", "🤖 CLAIM: actual: {}, expected: {}", + income, + self.expected_reward, + ); + assert_eq_error_rate!(income, self.expected_reward, 10); + self.expected_reward = 0; + } +} + +fn main() { + let mut reward_agent = RewardAgent::new(REWARD_AGENT_ACCOUNT); + sp_tracing::try_init_simple(); + let mut ext = sp_io::TestExternalities::new_empty(); + let mut events_histogram = Vec::<(PoolsEvents, u32)>::default(); + let mut iteration = 0 as BlockNumber; + let mut ok = 0; + let mut err = 0; + + let dot: Balance = (10 as Balance).pow(10); + ExistentialDeposit::set(dot); + BondingDuration::set(8); + + ext.execute_with(|| { + MaxPoolMembers::::set(Some(10_000)); + MaxPoolMembersPerPool::::set(Some(1000)); + MaxPools::::set(Some(1_000)); + + MinCreateBond::::set(10 * ExistentialDeposit::get()); + MinJoinBond::::set(5 * ExistentialDeposit::get()); + System::set_block_number(1); + }); + + loop { + fuzz!(|seed: [u8; 32]| { + use ::rand::{rngs::SmallRng, SeedableRng}; + let mut rng = SmallRng::from_seed(seed); + + ext.execute_with(|| { + let (call, origin) = random_call(&mut rng); + let outcome = call.clone().dispatch_bypass_filter(origin.clone()); + iteration += 1; + match outcome { + Ok(_) => ok += 1, + Err(_) => err += 1, + }; + + log!( + trace, + "iteration {}, call {:?}, origin {:?}, outcome: {:?}, so far {} ok {} err", + iteration, + call, + origin, + outcome, + ok, + err, + ); + + // possibly join the reward_agent + if iteration > ERA / 2 && BondedPools::::count() > 0 { + reward_agent.join(); + } + // and possibly roughly every 4 era, trigger payout for the agent. Doing this more + // frequent is also harmless. + if rng.gen_range(0..(4 * ERA)) == 0 { + reward_agent.claim_payout(); + } + + // execute sanity checks at a fixed interval, possibly on every block. + if iteration % + (std::env::var("SANITY_CHECK_INTERVAL") + .ok() + .and_then(|x| x.parse::().ok())) + .unwrap_or(1) == 0 + { + log!(info, "running sanity checks at {}", iteration); + Pools::do_try_state(u8::MAX).unwrap(); + } + + // collect and reset events. + System::events() + .into_iter() + .map(|r| r.event) + .filter_map(|e| { + if let pallet_nomination_pools::mock::RuntimeEvent::Pools(inner) = e { + Some(inner) + } else { + None + } + }) + .for_each(|e| { + if let Some((_, c)) = events_histogram + .iter_mut() + .find(|(x, _)| std::mem::discriminant(x) == std::mem::discriminant(&e)) + { + *c += 1; + } else { + events_histogram.push((e, 1)) + } + }); + System::reset_events(); + + // trigger an era change, and check the status of the reward agent. + if iteration % ERA == 0 { + CurrentEra::mutate(|c| *c += 1); + BondedPools::::iter().for_each(|(id, _)| { + let amount = random_ed_multiple(&mut rng); + let _ = + Balances::deposit_creating(&Pools::create_reward_account(id), amount); + // if we just paid out the reward agent, let's calculate how much we expect + // our reward agent to have earned. + if reward_agent.pool_id.map_or(false, |mid| mid == id) { + let all_points = BondedPool::::get(id).map(|p| p.points).unwrap(); + let member_points = + PoolMembers::::get(reward_agent.who).map(|m| m.points).unwrap(); + let agent_share = Perquintill::from_rational(member_points, all_points); + log::info!( + target: "reward-agent", + "🤖 REWARD = amount = {:?}, ratio: {:?}, share {:?}", + amount, + agent_share, + agent_share * amount, + ); + reward_agent.expected_reward += agent_share * amount; + } + }); + + log!( + info, + "iteration {}, {} pools, {} members, {} ok {} err, events = {:?}", + iteration, + BondedPools::::count(), + PoolMembers::::count(), + ok, + err, + events_histogram + .iter() + .map(|(x, c)| ( + format!("{:?}", x) + .split(" ") + .map(|x| x.to_string()) + .collect::>() + .first() + .cloned() + .unwrap(), + c, + )) + .collect::>(), + ); + } + }) + }) + } +} diff --git a/frame/nomination-pools/src/lib.rs b/frame/nomination-pools/src/lib.rs index 3661a7f70b48a..9ca9539b3dca8 100644 --- a/frame/nomination-pools/src/lib.rs +++ b/frame/nomination-pools/src/lib.rs @@ -24,9 +24,10 @@ //! //! * [Key terms](#key-terms) //! * [Usage](#usage) +//! * [Implementor's Guide](#implementors-guide) //! * [Design](#design) //! -//! ## Key terms +//! ## Key Terms //! //! * pool id: A unique identifier of each pool. Set to u32. //! * bonded pool: Tracks the distribution of actively staked funds. See [`BondedPool`] and @@ -88,7 +89,11 @@ //! in the aforementioned range of eras will be affected by the slash. A member is slashed pro-rata //! based on its stake relative to the total slash amount. //! -//! For design docs see the [slashing](#slashing) section. +//! Slashing does not change any single member's balance. Instead, the slash will only reduce the +//! balance associated with a particular pool. But, we never change the total *points* of a pool +//! because of slashing. Therefore, when a slash happens, the ratio of points to balance changes in +//! a pool. In other words, the value of one point, which is initially 1-to-1 against a unit of +//! balance, is now less than one balance because of the slash. //! //! ### Administration //! @@ -96,6 +101,10 @@ //! user must call [`Call::nominate`] to start nominating. [`Call::nominate`] can be called at //! anytime to update validator selection. //! +//! Similar to [`Call::nominate`], [`Call::chill`] will chill to pool in the staking system, and +//! [`Call::pool_withdraw_unbonded`] will withdraw any unbonding chunks of the pool bonded account. +//! The latter call is permissionless and can be called by anyone at any time. +//! //! To help facilitate pool administration the pool has one of three states (see [`PoolState`]): //! //! * Open: Anyone can join the pool and no members can be permissionlessly removed. @@ -121,10 +130,52 @@ //! //! 1. First, all members need to fully unbond and withdraw. If the pool state is set to //! `Destroying`, this can happen permissionlessly. -//! 2. The depositor itself fully unbonds and withdraws. Note that at this point, based on the -//! requirements of the staking system, the pool's bonded account's stake might not be able to ge -//! below a certain threshold as a nominator. At this point, the pool should `chill` itself to -//! allow the depositor to leave. +//! 2. The depositor itself fully unbonds and withdraws. +//! +//! > Note that at this point, based on the requirements of the staking system, the pool's bonded +//! > account's stake might not be able to ge below a certain threshold as a nominator. At this +//! > point, the pool should `chill` itself to allow the depositor to leave. See [`Call::chill`]. +//! +//! ## Implementor's Guide +//! +//! Some notes and common mistakes that wallets/apps wishing to implement this pallet should be +//! aware of: +//! +//! +//! ### Pool Members +//! +//! * In general, whenever a pool member changes their total point, the chain will automatically +//! claim all their pending rewards for them. This is not optional, and MUST happen for the reward +//! calculation to remain correct (see the documentation of `bond` as an example). So, make sure +//! you are warning your users about it. They might be surprised if they see that they bonded an +//! extra 100 DOTs, and now suddenly their 5.23 DOTs in pending reward is gone. It is not gone, it +//! has been paid out to you! +//! * Joining a pool implies transferring funds to the pool account. So it might be (based on which +//! wallet that you are using) that you no longer see the funds that are moved to the pool in your +//! “free balance” section. Make sure the user is aware of this, and not surprised by seeing this. +//! Also, the transfer that happens here is configured to to never accidentally destroy the sender +//! account. So to join a Pool, your sender account must remain alive with 1 DOT left in it. This +//! means, with 1 DOT as existential deposit, and 1 DOT as minimum to join a pool, you need at +//! least 2 DOT to join a pool. Consequently, if you are suggesting members to join a pool with +//! “Maximum possible value”, you must subtract 1 DOT to remain in the sender account to not +//! accidentally kill it. +//! * Points and balance are not the same! Any pool member, at any point in time, can have points in +//! either the bonded pool or any of the unbonding pools. The crucial fact is that in any of these +//! pools, the ratio of point to balance is different and might not be 1. Each pool starts with a +//! ratio of 1, but as time goes on, for reasons such as slashing, the ratio gets broken. Over +//! time, 100 points in a bonded pool can be worth 90 DOTs. Make sure you are either representing +//! points as points (not as DOTs), or even better, always display both: “You have x points in +//! pool y which is worth z DOTs”. See here and here for examples of how to calculate point to +//! balance ratio of each pool (it is almost trivial ;)) +//! +//! ### Pool Management +//! +//! * The pool will be seen from the perspective of the rest of the system as a single nominator. +//! Ergo, This nominator must always respect the `staking.minNominatorBond` limit. Similar to a +//! normal nominator, who has to first `chill` before fully unbonding, the pool must also do the +//! same. The pool’s bonded account will be fully unbonded only when the depositor wants to leave +//! and dismantle the pool. All that said, the message is: the depositor can only leave the chain +//! when they chill the pool first. //! //! ## Design //! @@ -277,14 +328,13 @@ use frame_support::{ Currency, Defensive, DefensiveOption, DefensiveResult, DefensiveSaturating, ExistenceRequirement, Get, }, - transactional, CloneNoBound, DefaultNoBound, RuntimeDebugNoBound, + DefaultNoBound, }; use scale_info::TypeInfo; use sp_core::U256; use sp_runtime::{ traits::{ - AccountIdConversion, Bounded, CheckedAdd, CheckedSub, Convert, Saturating, StaticLookup, - Zero, + AccountIdConversion, CheckedAdd, CheckedSub, Convert, Saturating, StaticLookup, Zero, }, FixedPointNumber, }; @@ -299,14 +349,14 @@ pub const LOG_TARGET: &'static str = "runtime::nomination-pools"; macro_rules! log { ($level:tt, $patter:expr $(, $values:expr)* $(,)?) => { log::$level!( - target: crate::LOG_TARGET, + target: $crate::LOG_TARGET, concat!("[{:?}] 🏊‍♂️ ", $patter), >::block_number() $(, $values)* ) }; } -#[cfg(test)] -mod mock; +#[cfg(any(test, feature = "fuzzing"))] +pub mod mock; #[cfg(test)] mod tests; @@ -322,8 +372,6 @@ pub type BalanceOf = /// Type used for unique identifier of each pool. pub type PoolId = u32; -type UnbondingPoolsWithEra = BoundedBTreeMap, TotalUnbondingPools>; - type AccountIdLookupOf = <::Lookup as StaticLookup>::Source; pub const POINTS_TO_BALANCE_INIT_RATIO: u32 = 1; @@ -398,16 +446,12 @@ impl PoolMember { // // rc * 10^20 / 10^18 = rc * 100 // - // meaning that as long as reward_counter's value is less than 1/100th of its max capacity - // (u128::MAX_VALUE), `checked_mul_int` won't saturate. - // - // given the nature of reward counter being 'pending_rewards / pool_total_point', the only - // (unrealistic) way that super high values can be achieved is for a pool to suddenly - // receive massive rewards with a very very small amount of stake. In all normal pools, as - // the points increase, so does the rewards. Moreover, as long as rewards are not - // accumulated for astronomically large durations, - // `current_reward_counter.defensive_saturating_sub(self.last_recorded_reward_counter)` - // won't be extremely big. + // the implementation of `multiply_by_rational_with_rounding` shows that it will only fail + // if the final division is not enough to fit in u128. In other words, if `rc * 100` is more + // than u128::max. Given that RC is interpreted as reward per unit of point, and unit of + // point is equal to balance (normally), and rewards are usually a proportion of the points + // in the pool, the likelihood of rc reaching near u128::MAX is near impossible. + (current_reward_counter.defensive_saturating_sub(self.last_recorded_reward_counter)) .checked_mul_int(self.active_points()) .ok_or(Error::::OverflowRisk) @@ -417,7 +461,7 @@ impl PoolMember { /// /// This is derived from the ratio of points in the pool to which the member belongs to. /// Might return different values based on the pool state for the same member and points. - fn active_stake(&self) -> BalanceOf { + fn active_balance(&self) -> BalanceOf { if let Some(pool) = BondedPool::::get(self.pool_id).defensive() { pool.points_to_balance(self.points) } else { @@ -594,7 +638,7 @@ impl BondedPool { } /// Get [`Self`] from storage. Returns `None` if no entry for `pool_account` exists. - fn get(id: PoolId) -> Option { + pub fn get(id: PoolId) -> Option { BondedPools::::try_get(id).ok().map(|inner| Self { id, inner }) } @@ -734,7 +778,7 @@ impl BondedPool { /// Whether or not the pool is ok to be in `PoolSate::Open`. If this returns an `Err`, then the /// pool is unrecoverable and should be in the destroying state. - fn ok_to_be_open(&self, new_funds: BalanceOf) -> Result<(), DispatchError> { + fn ok_to_be_open(&self) -> Result<(), DispatchError> { ensure!(!self.is_destroying(), Error::::CanNotChangeState); let bonded_balance = @@ -755,12 +799,6 @@ impl BondedPool { points_to_balance_ratio_floor < max_points_to_balance.into(), Error::::OverflowRisk ); - // while restricting the balance to `max_points_to_balance` of max total issuance, - let next_bonded_balance = bonded_balance.saturating_add(new_funds); - ensure!( - next_bonded_balance < BalanceOf::::max_value().div(max_points_to_balance.into()), - Error::::OverflowRisk - ); // then we can be decently confident the bonding pool points will not overflow // `BalanceOf`. Note that these are just heuristics. @@ -769,9 +807,9 @@ impl BondedPool { } /// Check that the pool can accept a member with `new_funds`. - fn ok_to_join(&self, new_funds: BalanceOf) -> Result<(), DispatchError> { + fn ok_to_join(&self) -> Result<(), DispatchError> { ensure!(self.state == PoolState::Open, Error::::NotOpen); - self.ok_to_be_open(new_funds)?; + self.ok_to_be_open()?; Ok(()) } @@ -791,7 +829,7 @@ impl BondedPool { target_member.active_points().saturating_sub(unbonding_points); let mut target_member_after_unbond = (*target_member).clone(); target_member_after_unbond.points = new_depositor_points; - target_member_after_unbond.active_stake() + target_member_after_unbond.active_balance() }; // any partial unbonding is only ever allowed if this unbond is permissioned. @@ -1073,7 +1111,7 @@ pub struct SubPools { /// older then `current_era - TotalUnbondingPools`. no_era: UnbondPool, /// Map of era in which a pool becomes unbonded in => unbond pools. - with_era: UnbondingPoolsWithEra, + with_era: BoundedBTreeMap, TotalUnbondingPools>, } impl SubPools { @@ -1105,7 +1143,7 @@ impl SubPools { } /// The sum of all unbonding balance, regardless of whether they are actually unlocked or not. - #[cfg(any(feature = "try-runtime", test, debug_assertions))] + #[cfg(any(feature = "try-runtime", feature = "fuzzing", test, debug_assertions))] fn sum_unbonding_balance(&self) -> BalanceOf { self.no_era.balance.saturating_add( self.with_era @@ -1122,8 +1160,8 @@ pub struct TotalUnbondingPools(PhantomData); impl Get for TotalUnbondingPools { fn get() -> u32 { // NOTE: this may be dangerous in the scenario bonding_duration gets decreased because - // we would no longer be able to decode `UnbondingPoolsWithEra`, which uses - // `TotalUnbondingPools` as the bound + // we would no longer be able to decode `BoundedBTreeMap::, + // TotalUnbondingPools>`, which uses `TotalUnbondingPools` as the bound T::Staking::bonding_duration() + T::PostUnbondingPoolsWindow::get() } } @@ -1469,7 +1507,6 @@ pub mod pallet { /// `existential deposit + amount` in their account. /// * Only a pool with [`PoolState::Open`] can be joined #[pallet::weight(T::WeightInfo::join())] - #[transactional] pub fn join( origin: OriginFor, #[pallet::compact] amount: BalanceOf, @@ -1482,7 +1519,7 @@ pub mod pallet { ensure!(!PoolMembers::::contains_key(&who), Error::::AccountBelongsToOtherPool); let mut bonded_pool = BondedPool::::get(pool_id).ok_or(Error::::PoolNotFound)?; - bonded_pool.ok_to_join(amount)?; + bonded_pool.ok_to_join()?; let mut reward_pool = RewardPools::::get(pool_id) .defensive_ok_or::>(DefensiveError::RewardPoolNotFound.into())?; @@ -1530,7 +1567,6 @@ pub mod pallet { T::WeightInfo::bond_extra_transfer() .max(T::WeightInfo::bond_extra_reward()) )] - #[transactional] pub fn bond_extra(origin: OriginFor, extra: BondExtra>) -> DispatchResult { let who = ensure_signed(origin)?; let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&who)?; @@ -1538,10 +1574,6 @@ pub mod pallet { // payout related stuff: we must claim the payouts, and updated recorded payout data // before updating the bonded pool points, similar to that of `join` transaction. reward_pool.update_records(bonded_pool.id, bonded_pool.points)?; - // TODO: optimize this to not touch the free balance of `who ` at all in benchmarks. - // Currently, bonding rewards is like a batch. In the same PR, also make this function - // take a boolean argument that make it either 100% pure (no storage update), or make it - // also emit event and do the transfer. #11671 let claimed = Self::do_reward_payout(&who, &mut member, &mut bonded_pool, &mut reward_pool)?; @@ -1552,8 +1584,9 @@ pub mod pallet { (bonded_pool.try_bond_funds(&who, claimed, BondType::Later)?, claimed), }; - bonded_pool.ok_to_be_open(bonded)?; - member.points = member.points.saturating_add(points_issued); + bonded_pool.ok_to_be_open()?; + member.points = + member.points.checked_add(&points_issued).ok_or(Error::::OverflowRisk)?; Self::deposit_event(Event::::Bonded { member: who.clone(), @@ -1573,7 +1606,6 @@ pub mod pallet { /// The member will earn rewards pro rata based on the members stake vs the sum of the /// members in the pools stake. Rewards do not "expire". #[pallet::weight(T::WeightInfo::claim_payout())] - #[transactional] pub fn claim_payout(origin: OriginFor) -> DispatchResult { let who = ensure_signed(origin)?; let (mut member, mut bonded_pool, mut reward_pool) = Self::get_member_with_pools(&who)?; @@ -1613,7 +1645,6 @@ pub mod pallet { /// there are too many unlocking chunks, the result of this call will likely be the /// `NoMoreChunks` error from the staking system. #[pallet::weight(T::WeightInfo::unbond())] - #[transactional] pub fn unbond( origin: OriginFor, member_account: AccountIdLookupOf, @@ -1689,7 +1720,6 @@ pub mod pallet { /// would probably see an error like `NoMoreChunks` emitted from the staking system when /// they attempt to unbond. #[pallet::weight(T::WeightInfo::pool_withdraw_unbonded(*num_slashing_spans))] - #[transactional] pub fn pool_withdraw_unbonded( origin: OriginFor, pool_id: PoolId, @@ -1726,7 +1756,6 @@ pub mod pallet { #[pallet::weight( T::WeightInfo::withdraw_unbonded_kill(*num_slashing_spans) )] - #[transactional] pub fn withdraw_unbonded( origin: OriginFor, member_account: AccountIdLookupOf, @@ -1749,7 +1778,7 @@ pub mod pallet { let withdrawn_points = member.withdraw_unlocked(current_era); ensure!(!withdrawn_points.is_empty(), Error::::CannotWithdrawAny); - // Before calculate the `balance_to_unbond`, with call withdraw unbonded to ensure the + // Before calculating the `balance_to_unbond`, we call withdraw unbonded to ensure the // `transferrable_balance` is correct. let stash_killed = T::Staking::withdraw_unbonded(bonded_pool.bonded_account(), num_slashing_spans)?; @@ -1778,13 +1807,13 @@ pub mod pallet { accumulator.saturating_add(sub_pools.no_era.dissolve(*unlocked_points)) } }) - // A call to this function may cause the pool's stash to get dusted. If this happens - // before the last member has withdrawn, then all subsequent withdraws will be 0. - // However the unbond pools do no get updated to reflect this. In the aforementioned - // scenario, this check ensures we don't try to withdraw funds that don't exist. - // This check is also defensive in cases where the unbond pool does not update its - // balance (e.g. a bug in the slashing hook.) We gracefully proceed in order to - // ensure members can leave the pool and it can be destroyed. + // A call to this transaction may cause the pool's stash to get dusted. If this + // happens before the last member has withdrawn, then all subsequent withdraws will + // be 0. However the unbond pools do no get updated to reflect this. In the + // aforementioned scenario, this check ensures we don't try to withdraw funds that + // don't exist. This check is also defensive in cases where the unbond pool does not + // update its balance (e.g. a bug in the slashing hook.) We gracefully proceed in + // order to ensure members can leave the pool and it can be destroyed. .min(bonded_pool.transferrable_balance()); T::Currency::transfer( @@ -1846,7 +1875,6 @@ pub mod pallet { /// In addition to `amount`, the caller will transfer the existential deposit; so the caller /// needs at have at least `amount + existential_deposit` transferrable. #[pallet::weight(T::WeightInfo::create())] - #[transactional] pub fn create( origin: OriginFor, #[pallet::compact] amount: BalanceOf, @@ -1871,7 +1899,6 @@ pub mod pallet { /// same as `create` with the inclusion of /// * `pool_id` - `A valid PoolId. #[pallet::weight(T::WeightInfo::create())] - #[transactional] pub fn create_with_pool_id( origin: OriginFor, #[pallet::compact] amount: BalanceOf, @@ -1896,7 +1923,6 @@ pub mod pallet { /// This directly forward the call to the staking pallet, on behalf of the pool bonded /// account. #[pallet::weight(T::WeightInfo::nominate(validators.len() as u32))] - #[transactional] pub fn nominate( origin: OriginFor, pool_id: PoolId, @@ -1919,7 +1945,6 @@ pub mod pallet { /// 2. if the pool conditions to be open are NOT met (as described by `ok_to_be_open`), and /// then the state of the pool can be permissionlessly changed to `Destroying`. #[pallet::weight(T::WeightInfo::set_state())] - #[transactional] pub fn set_state( origin: OriginFor, pool_id: PoolId, @@ -1931,9 +1956,7 @@ pub mod pallet { if bonded_pool.can_toggle_state(&who) { bonded_pool.set_state(state); - } else if bonded_pool.ok_to_be_open(Zero::zero()).is_err() && - state == PoolState::Destroying - { + } else if bonded_pool.ok_to_be_open().is_err() && state == PoolState::Destroying { // If the pool has bad properties, then anyone can set it as destroying bonded_pool.set_state(PoolState::Destroying); } else { @@ -1950,7 +1973,6 @@ pub mod pallet { /// The dispatch origin of this call must be signed by the state toggler, or the root role /// of the pool. #[pallet::weight(T::WeightInfo::set_metadata(metadata.len() as u32))] - #[transactional] pub fn set_metadata( origin: OriginFor, pool_id: PoolId, @@ -1982,7 +2004,6 @@ pub mod pallet { /// * `max_members` - Set [`MaxPoolMembers`]. /// * `max_members_per_pool` - Set [`MaxPoolMembersPerPool`]. #[pallet::weight(T::WeightInfo::set_configs())] - #[transactional] pub fn set_configs( origin: OriginFor, min_join_bond: ConfigOp>, @@ -2019,7 +2040,6 @@ pub mod pallet { /// It emits an event, notifying UIs of the role change. This event is quite relevant to /// most pool members and they should be informed of changes to pool roles. #[pallet::weight(T::WeightInfo::update_roles())] - #[transactional] pub fn update_roles( origin: OriginFor, pool_id: PoolId, @@ -2072,7 +2092,6 @@ pub mod pallet { /// This directly forward the call to the staking pallet, on behalf of the pool bonded /// account. #[pallet::weight(T::WeightInfo::chill())] - #[transactional] pub fn chill(origin: OriginFor, pool_id: PoolId) -> DispatchResult { let who = ensure_signed(origin)?; let bonded_pool = BondedPool::::get(pool_id).ok_or(Error::::PoolNotFound)?; @@ -2297,6 +2316,8 @@ impl Pallet { &bonded_pool.reward_account(), &member_account, pending_rewards, + // defensive: the depositor has put existential deposit into the pool and it stays + // untouched, reward account shall not die. ExistenceRequirement::AllowDeath, )?; @@ -2414,7 +2435,7 @@ impl Pallet { /// To cater for tests that want to escape parts of these checks, this function is split into /// multiple `level`s, where the higher the level, the more checks we performs. So, /// `try_state(255)` is the strongest sanity check, and `0` performs no checks. - #[cfg(any(feature = "try-runtime", test, debug_assertions))] + #[cfg(any(feature = "try-runtime", feature = "fuzzing", test, debug_assertions))] pub fn do_try_state(level: u8) -> Result<(), &'static str> { if level.is_zero() { return Ok(()) diff --git a/frame/nomination-pools/src/mock.rs b/frame/nomination-pools/src/mock.rs index e1af456e31fd4..99d521df3241b 100644 --- a/frame/nomination-pools/src/mock.rs +++ b/frame/nomination-pools/src/mock.rs @@ -259,44 +259,45 @@ impl Default for ExtBuilder { } } +#[cfg_attr(feature = "fuzzing", allow(dead_code))] impl ExtBuilder { // Add members to pool 0. - pub(crate) fn add_members(mut self, members: Vec<(AccountId, Balance)>) -> Self { + pub fn add_members(mut self, members: Vec<(AccountId, Balance)>) -> Self { self.members = members; self } - pub(crate) fn ed(self, ed: Balance) -> Self { + pub fn ed(self, ed: Balance) -> Self { ExistentialDeposit::set(ed); self } - pub(crate) fn min_bond(self, min: Balance) -> Self { + pub fn min_bond(self, min: Balance) -> Self { StakingMinBond::set(min); self } - pub(crate) fn min_join_bond(self, min: Balance) -> Self { + pub fn min_join_bond(self, min: Balance) -> Self { MinJoinBondConfig::set(min); self } - pub(crate) fn with_check(self, level: u8) -> Self { + pub fn with_check(self, level: u8) -> Self { CheckLevel::set(level); self } - pub(crate) fn max_members(mut self, max: Option) -> Self { + pub fn max_members(mut self, max: Option) -> Self { self.max_members = max; self } - pub(crate) fn max_members_per_pool(mut self, max: Option) -> Self { + pub fn max_members_per_pool(mut self, max: Option) -> Self { self.max_members_per_pool = max; self } - pub(crate) fn build(self) -> sp_io::TestExternalities { + pub fn build(self) -> sp_io::TestExternalities { sp_tracing::try_init_simple(); let mut storage = frame_system::GenesisConfig::default().build_storage::().unwrap(); @@ -339,7 +340,7 @@ impl ExtBuilder { } } -pub(crate) fn unsafe_set_state(pool_id: PoolId, state: PoolState) { +pub fn unsafe_set_state(pool_id: PoolId, state: PoolState) { BondedPools::::try_mutate(pool_id, |maybe_bonded_pool| { maybe_bonded_pool.as_mut().ok_or(()).map(|bonded_pool| { bonded_pool.state = state; @@ -354,7 +355,7 @@ parameter_types! { } /// All events of this pallet. -pub(crate) fn pool_events_since_last_call() -> Vec> { +pub fn pool_events_since_last_call() -> Vec> { let events = System::events() .into_iter() .map(|r| r.event) @@ -366,7 +367,7 @@ pub(crate) fn pool_events_since_last_call() -> Vec> { } /// All events of the `Balances` pallet. -pub(crate) fn balances_events_since_last_call() -> Vec> { +pub fn balances_events_since_last_call() -> Vec> { let events = System::events() .into_iter() .map(|r| r.event) diff --git a/frame/nomination-pools/src/tests.rs b/frame/nomination-pools/src/tests.rs index aadd4871d737e..7d5d418bbf2c8 100644 --- a/frame/nomination-pools/src/tests.rs +++ b/frame/nomination-pools/src/tests.rs @@ -25,7 +25,7 @@ macro_rules! unbonding_pools_with_era { ($($k:expr => $v:expr),* $(,)?) => {{ use sp_std::iter::{Iterator, IntoIterator}; let not_bounded: BTreeMap<_, _> = Iterator::collect(IntoIterator::into_iter([$(($k, $v),)*])); - UnbondingPoolsWithEra::try_from(not_bounded).unwrap() + BoundedBTreeMap::, TotalUnbondingPools>::try_from(not_bounded).unwrap() }}; } @@ -213,31 +213,30 @@ mod bonded_pool { // Simulate a 100% slashed pool StakingMock::set_bonded_balance(pool.bonded_account(), 0); - assert_noop!(pool.ok_to_join(0), Error::::OverflowRisk); + assert_noop!(pool.ok_to_join(), Error::::OverflowRisk); // Simulate a slashed pool at `MaxPointsToBalance` + 1 slashed pool StakingMock::set_bonded_balance( pool.bonded_account(), max_points_to_balance.saturating_add(1).into(), ); - assert_ok!(pool.ok_to_join(0)); + assert_ok!(pool.ok_to_join()); // Simulate a slashed pool at `MaxPointsToBalance` StakingMock::set_bonded_balance(pool.bonded_account(), max_points_to_balance); - assert_noop!(pool.ok_to_join(0), Error::::OverflowRisk); + assert_noop!(pool.ok_to_join(), Error::::OverflowRisk); StakingMock::set_bonded_balance( pool.bonded_account(), Balance::MAX / max_points_to_balance, ); - // New bonded balance would be over threshold of Balance type - assert_noop!(pool.ok_to_join(0), Error::::OverflowRisk); + // and a sanity check StakingMock::set_bonded_balance( pool.bonded_account(), Balance::MAX / max_points_to_balance - 1, ); - assert_ok!(pool.ok_to_join(0)); + assert_ok!(pool.ok_to_join()); }); } } @@ -437,7 +436,7 @@ mod join { roles: DEFAULT_ROLES, }, }; - ExtBuilder::default().build_and_execute(|| { + ExtBuilder::default().with_check(0).build_and_execute(|| { // Given Balances::make_free_balance_be(&11, ExistentialDeposit::get() + 2); assert!(!PoolMembers::::contains_key(&11)); @@ -545,7 +544,7 @@ mod join { // Balance needs to be gt Balance::MAX / `MaxPointsToBalance` assert_noop!( Pools::join(RuntimeOrigin::signed(11), 5, 123), - Error::::OverflowRisk + pallet_balances::Error::::InsufficientBalance, ); StakingMock::set_bonded_balance(Pools::create_bonded_account(1), max_points_to_balance); @@ -4283,7 +4282,7 @@ mod set_state { fn set_state_works() { ExtBuilder::default().build_and_execute(|| { // Given - assert_ok!(BondedPool::::get(1).unwrap().ok_to_be_open(0)); + assert_ok!(BondedPool::::get(1).unwrap().ok_to_be_open()); // Only the root and state toggler can change the state when the pool is ok to be open. assert_noop!( @@ -4831,6 +4830,41 @@ mod reward_counter_precision { }) } + #[test] + fn massive_reward_in_small_pool() { + let tiny_bond = 1000 * DOT; + ExtBuilder::default().ed(DOT).min_bond(tiny_bond).build_and_execute(|| { + assert_eq!( + pool_events_since_last_call(), + vec![ + Event::Created { depositor: 10, pool_id: 1 }, + Event::Bonded { member: 10, pool_id: 1, bonded: 10000000000000, joined: true } + ] + ); + + Balances::make_free_balance_be(&20, tiny_bond); + assert_ok!(Pools::join(RuntimeOrigin::signed(20), tiny_bond / 2, 1)); + + // Suddenly, add a shit ton of rewards. + assert_ok!( + Balances::mutate_account(&default_reward_account(), |a| a.free += inflation(1)) + ); + + // now claim. + assert_ok!(Pools::claim_payout(RuntimeOrigin::signed(10))); + assert_ok!(Pools::claim_payout(RuntimeOrigin::signed(20))); + + assert_eq!( + pool_events_since_last_call(), + vec![ + Event::Bonded { member: 20, pool_id: 1, bonded: 5000000000000, joined: true }, + Event::PaidOut { member: 10, pool_id: 1, payout: 7333333333333333333 }, + Event::PaidOut { member: 20, pool_id: 1, payout: 3666666666666666666 } + ] + ); + }) + } + #[test] fn reward_counter_calc_wont_fail_in_normal_polkadot_future() { // create a pool that has roughly half of the polkadot issuance in 10 years. @@ -5066,328 +5100,3 @@ mod reward_counter_precision { }); } } - -// NOTE: run this with debug_assertions, but in release mode. -#[cfg(feature = "fuzz-test")] -mod fuzz_test { - use super::*; - use crate::pallet::{Call as PoolsCall, Event as PoolsEvents}; - use frame_support::traits::UnfilteredDispatchable; - use rand::{seq::SliceRandom, thread_rng, Rng}; - use sp_runtime::{assert_eq_error_rate, Perquintill}; - - const ERA: BlockNumber = 1000; - const MAX_ED_MULTIPLE: Balance = 10_000; - const MIN_ED_MULTIPLE: Balance = 10; - - // not quite elegant, just to make it available in random_signed_origin. - const REWARD_AGENT_ACCOUNT: AccountId = 42; - - /// Grab random accounts, either known ones, or new ones. - fn random_signed_origin(rng: &mut R) -> (RuntimeOrigin, AccountId) { - let count = PoolMembers::::count(); - if rng.gen::() && count > 0 { - // take an existing account. - let skip = rng.gen_range(0..count as usize); - - // this is tricky: the account might be our reward agent, which we never want to be - // randomly chosen here. Try another one, or, if it is only our agent, return a random - // one nonetheless. - let candidate = PoolMembers::::iter_keys().skip(skip).take(1).next().unwrap(); - let acc = - if candidate == REWARD_AGENT_ACCOUNT { rng.gen::() } else { candidate }; - - (RuntimeOrigin::signed(acc), acc) - } else { - // create a new account - let acc = rng.gen::(); - (RuntimeOrigin::signed(acc), acc) - } - } - - fn random_ed_multiple(rng: &mut R) -> Balance { - let multiple = rng.gen_range(MIN_ED_MULTIPLE..MAX_ED_MULTIPLE); - ExistentialDeposit::get() * multiple - } - - fn fund_account(rng: &mut R, account: &AccountId) { - let target_amount = random_ed_multiple(rng); - if let Some(top_up) = target_amount.checked_sub(Balances::free_balance(account)) { - let _ = Balances::deposit_creating(account, top_up); - } - assert!(Balances::free_balance(account) >= target_amount); - } - - fn random_existing_pool(mut rng: &mut R) -> Option { - BondedPools::::iter_keys().collect::>().choose(&mut rng).map(|x| *x) - } - - fn random_call(mut rng: &mut R) -> (crate::pallet::Call, RuntimeOrigin) { - let op = rng.gen::(); - let mut op_count = - as frame_support::dispatch::GetCallName>::get_call_names() - .len(); - // Exclude set_state, set_metadata, set_configs, update_roles and chill. - op_count -= 5; - - match op % op_count { - 0 => { - // join - let pool_id = random_existing_pool(&mut rng).unwrap_or_default(); - let (origin, who) = random_signed_origin(&mut rng); - fund_account(&mut rng, &who); - let amount = random_ed_multiple(&mut rng); - (PoolsCall::::join { amount, pool_id }, origin) - }, - 1 => { - // bond_extra - let (origin, who) = random_signed_origin(&mut rng); - let extra = if rng.gen::() { - BondExtra::Rewards - } else { - fund_account(&mut rng, &who); - let amount = random_ed_multiple(&mut rng); - BondExtra::FreeBalance(amount) - }; - (PoolsCall::::bond_extra { extra }, origin) - }, - 2 => { - // claim_payout - let (origin, _) = random_signed_origin(&mut rng); - (PoolsCall::::claim_payout {}, origin) - }, - 3 => { - // unbond - let (origin, who) = random_signed_origin(&mut rng); - let amount = random_ed_multiple(&mut rng); - (PoolsCall::::unbond { member_account: who, unbonding_points: amount }, origin) - }, - 4 => { - // pool_withdraw_unbonded - let pool_id = random_existing_pool(&mut rng).unwrap_or_default(); - let (origin, _) = random_signed_origin(&mut rng); - (PoolsCall::::pool_withdraw_unbonded { pool_id, num_slashing_spans: 0 }, origin) - }, - 5 => { - // withdraw_unbonded - let (origin, who) = random_signed_origin(&mut rng); - ( - PoolsCall::::withdraw_unbonded { - member_account: who, - num_slashing_spans: 0, - }, - origin, - ) - }, - 6 => { - // create - let (origin, who) = random_signed_origin(&mut rng); - let amount = random_ed_multiple(&mut rng); - fund_account(&mut rng, &who); - let root = who.clone(); - let state_toggler = who.clone(); - let nominator = who.clone(); - (PoolsCall::::create { amount, root, state_toggler, nominator }, origin) - }, - 7 => { - // nominate - let (origin, _) = random_signed_origin(&mut rng); - let pool_id = random_existing_pool(&mut rng).unwrap_or_default(); - let validators = Default::default(); - (PoolsCall::::nominate { pool_id, validators }, origin) - }, - _ => unreachable!(), - } - } - - #[derive(Default)] - struct RewardAgent { - who: AccountId, - pool_id: Option, - expected_reward: Balance, - } - - // TODO: inject some slashes into the game. - impl RewardAgent { - fn new(who: AccountId) -> Self { - Self { who, ..Default::default() } - } - - fn join(&mut self) { - if self.pool_id.is_some() { - return - } - let pool_id = LastPoolId::::get(); - let amount = 10 * ExistentialDeposit::get(); - let origin = RuntimeOrigin::signed(self.who); - let _ = Balances::deposit_creating(&self.who, 10 * amount); - self.pool_id = Some(pool_id); - log::info!(target: "reward-agent", "🤖 reward agent joining in {} with {}", pool_id, amount); - assert_ok!(PoolsCall::join:: { amount, pool_id }.dispatch_bypass_filter(origin)); - } - - fn claim_payout(&mut self) { - // 10 era later, we claim our payout. We expect our income to be roughly what we - // calculated. - if !PoolMembers::::contains_key(&self.who) { - log!(warn, "reward agent is not in the pool yet, cannot claim"); - return - } - let pre = Balances::free_balance(&42); - let origin = RuntimeOrigin::signed(42); - assert_ok!(PoolsCall::::claim_payout {}.dispatch_bypass_filter(origin)); - let post = Balances::free_balance(&42); - - let income = post - pre; - log::info!( - target: "reward-agent", "🤖 CLAIM: actual: {}, expected: {}", - income, - self.expected_reward, - ); - assert_eq_error_rate!(income, self.expected_reward, 10); - self.expected_reward = 0; - } - } - - #[test] - fn fuzz_test() { - let mut reward_agent = RewardAgent::new(42); - sp_tracing::try_init_simple(); - // NOTE: use this to get predictable (non)randomness: - // use::{rngs::SmallRng, SeedableRng}; - // let mut rng = SmallRng::from_seed([0u8; 32]); - let mut rng = thread_rng(); - let mut ext = sp_io::TestExternalities::new_empty(); - // NOTE: sadly events don't fulfill the requirements of hashmap or btreemap. - let mut events_histogram = Vec::<(PoolsEvents, u32)>::default(); - let mut iteration = 0 as BlockNumber; - let mut ok = 0; - let mut err = 0; - - ext.execute_with(|| { - MaxPoolMembers::::set(Some(10_000)); - MaxPoolMembersPerPool::::set(Some(1000)); - MaxPools::::set(Some(1_000)); - - MinCreateBond::::set(10 * ExistentialDeposit::get()); - MinJoinBond::::set(5 * ExistentialDeposit::get()); - System::set_block_number(1); - }); - - ExistentialDeposit::set(10u128.pow(12u32)); - BondingDuration::set(8); - - loop { - ext.execute_with(|| { - iteration += 1; - let (call, origin) = random_call(&mut rng); - let outcome = call.clone().dispatch_bypass_filter(origin.clone()); - - match outcome { - Ok(_) => ok += 1, - Err(_) => err += 1, - }; - - log!( - debug, - "iteration {}, call {:?}, origin {:?}, outcome: {:?}, so far {} ok {} err", - iteration, - call, - origin, - outcome, - ok, - err, - ); - - // possibly join the reward_agent - if iteration > ERA / 2 && BondedPools::::count() > 0 { - reward_agent.join(); - } - // and possibly roughly every 4 era, trigger payout for the agent. Doing this more - // frequent is also harmless. - if rng.gen_range(0..(4 * ERA)) == 0 { - reward_agent.claim_payout(); - } - - // execute sanity checks at a fixed interval, possibly on every block. - if iteration % - (std::env::var("SANITY_CHECK_INTERVAL") - .ok() - .and_then(|x| x.parse::().ok())) - .unwrap_or(1) == 0 - { - log!(info, "running sanity checks at {}", iteration); - Pools::do_try_state(u8::MAX).unwrap(); - } - - // collect and reset events. - System::events() - .into_iter() - .map(|r| r.event) - .filter_map( - |e| if let mock::Event::Pools(inner) = e { Some(inner) } else { None }, - ) - .for_each(|e| { - if let Some((_, c)) = events_histogram - .iter_mut() - .find(|(x, _)| std::mem::discriminant(x) == std::mem::discriminant(&e)) - { - *c += 1; - } else { - events_histogram.push((e, 1)) - } - }); - System::reset_events(); - - // trigger an era change, and check the status of the reward agent. - if iteration % ERA == 0 { - CurrentEra::mutate(|c| *c += 1); - BondedPools::::iter().for_each(|(id, _)| { - let amount = random_ed_multiple(&mut rng); - let _ = - Balances::deposit_creating(&Pools::create_reward_account(id), amount); - // if we just paid out the reward agent, let's calculate how much we expect - // our reward agent to have earned. - if reward_agent.pool_id.map_or(false, |mid| mid == id) { - let all_points = BondedPool::::get(id).map(|p| p.points).unwrap(); - let member_points = - PoolMembers::::get(reward_agent.who).map(|m| m.points).unwrap(); - let agent_share = Perquintill::from_rational(member_points, all_points); - log::info!( - target: "reward-agent", - "🤖 REWARD = amount = {:?}, ratio: {:?}, share {:?}", - amount, - agent_share, - agent_share * amount, - ); - reward_agent.expected_reward += agent_share * amount; - } - }); - - log!( - info, - "iteration {}, {} pools, {} members, {} ok {} err, events = {:?}", - iteration, - BondedPools::::count(), - PoolMembers::::count(), - ok, - err, - events_histogram - .iter() - .map(|(x, c)| ( - format!("{:?}", x) - .split(" ") - .map(|x| x.to_string()) - .collect::>() - .first() - .cloned() - .unwrap(), - c, - )) - .collect::>(), - ); - } - }); - } - } -}