From 4c0e0e071355c1048d75fba538c96c35ac743547 Mon Sep 17 00:00:00 2001 From: Sachin Charakhwal <40860699+SCJangra@users.noreply.github.com> Date: Fri, 22 Dec 2023 17:53:43 +0530 Subject: [PATCH] fix overflow in `balance_to_point` conversion (#2706) Closes #416 As I mentioned in the above issue `balance_to_points` conversion currently overflows the `Balance` types even before computing the actual points for the given tokens. This fixes that, --- substrate/frame/nomination-pools/src/lib.rs | 9 +++++--- substrate/frame/nomination-pools/src/tests.rs | 22 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/substrate/frame/nomination-pools/src/lib.rs b/substrate/frame/nomination-pools/src/lib.rs index 3a23b894ec8..c538f12e20b 100644 --- a/substrate/frame/nomination-pools/src/lib.rs +++ b/substrate/frame/nomination-pools/src/lib.rs @@ -2955,9 +2955,12 @@ impl<T: Config> Pallet<T> { }, (false, false) => { // Equivalent to (current_points / current_balance) * new_funds - balance(u256(current_points).saturating_mul(u256(new_funds))) - // We check for zero above - .div(current_balance) + balance( + u256(current_points) + .saturating_mul(u256(new_funds)) + // We check for zero above + .div(u256(current_balance)), + ) }, } } diff --git a/substrate/frame/nomination-pools/src/tests.rs b/substrate/frame/nomination-pools/src/tests.rs index 7fe1e704bb1..657b50e8594 100644 --- a/substrate/frame/nomination-pools/src/tests.rs +++ b/substrate/frame/nomination-pools/src/tests.rs @@ -292,6 +292,28 @@ mod bonded_pool { assert_ok!(pool.ok_to_join()); }); } + + #[test] + fn points_and_balance_conversions_are_safe() { + ExtBuilder::default().build_and_execute(|| { + let bonded_pool = BondedPool::<Runtime> { + id: 123123, + inner: BondedPoolInner { + commission: Commission::default(), + member_counter: 1, + points: u128::MAX, + roles: DEFAULT_ROLES, + state: PoolState::Open, + }, + }; + StakingMock::set_bonded_balance(bonded_pool.bonded_account(), u128::MAX); + + // Max out the points and balance of the pool and make sure the conversion works as + // expected and does not overflow. + assert_eq!(bonded_pool.balance_to_point(u128::MAX), u128::MAX); + assert_eq!(bonded_pool.points_to_balance(u128::MAX), u128::MAX); + }) + } } mod reward_pool { -- GitLab