From 393e3af69732edf730d031b5a0d7230e9efaab32 Mon Sep 17 00:00:00 2001 From: guibescos <59208140+guibescos@users.noreply.github.com> Date: Mon, 23 Oct 2023 14:23:41 +0100 Subject: [PATCH] Update vesting rounding (#248) * Update vesting rounding * Fix veting test * Fix clippy * Fix comment --- staking/programs/staking/src/state/vesting.rs | 90 ++++++++-------- staking/tests/vesting_test.ts | 102 +++++++++--------- 2 files changed, 94 insertions(+), 98 deletions(-) diff --git a/staking/programs/staking/src/state/vesting.rs b/staking/programs/staking/src/state/vesting.rs index 06349e68..516505c9 100644 --- a/staking/programs/staking/src/state/vesting.rs +++ b/staking/programs/staking/src/state/vesting.rs @@ -160,19 +160,19 @@ impl VestingSchedule { 0 } else { // Amount that vests per period is (initial_balance / num_periods), - // but again, we need to do the math in 128 bit precision and make sure - // we round the vested balance down, so as to round the unvested balance up. - - // Since we're in this branch, periods_passed <= num_periods, so vested <= - // initial_balance. Thus we know it can fit in a u64, so the unwrap - // can't fail. - let vested = (((periods_passed as u128) * (initial_balance as u128)) - / (num_periods as u128)) + // but again, we need to do the math in 128 bit precision. + + // Since we're in this branch, 0 <= periods_passed <= num_periods, so + // 0 <= remaining_periods <= num_periods. + // Additionally 0 <= initial_balance <= u64::MAX, so + // 0 <= unvested <= initial_balance <= u64::MAX + // therefore the unwrap can't fail. + // We round the unvested amount down, this makes the arithmetic for splitting accounts simpler. + let remaining_periods = num_periods.saturating_sub(periods_passed); + + (((remaining_periods as u128) * (initial_balance as u128)) / (num_periods as u128)) .try_into() - .unwrap(); - // We also know then 0 <= vested <= initial_balance, so this unwrap can't fail - // either I still feel safer with the unwrap though - initial_balance.checked_sub(vested).unwrap() + .unwrap() } } } @@ -211,24 +211,22 @@ impl VestingSchedule { ) .map_err(|_| error!(ErrorCode::GenericOverflow))?; - let current_vested: u64 = (((periods_passed as u128) * (initial_balance as u128)) - / (num_periods as u128)) - .try_into() - .map_err(|_| error!(ErrorCode::GenericOverflow))?; - - let periods_passed_incremented = periods_passed - .checked_add(1) - .ok_or_else(|| error!(ErrorCode::GenericOverflow))?; - - let next_period_vested: u64 = (((periods_passed_incremented as u128) - * (initial_balance as u128)) - / (num_periods as u128)) - .try_into() - .map_err(|_| error!(ErrorCode::GenericOverflow))?; + let current_unvested: u64 = Self::periodic_vesting_helper( + current_time, + initial_balance, + start_date, + period_duration, + num_periods, + ); + let next_period_unvested = Self::periodic_vesting_helper( + start_of_next_period, + initial_balance, + start_date, + period_duration, + num_periods, + ); - let amount: u64 = next_period_vested - .checked_sub(current_vested) - .ok_or_else(|| error!(ErrorCode::GenericOverflow))?; + let amount: u64 = current_unvested.saturating_sub(next_period_unvested); Ok(Some(VestingEvent { time: start_of_next_period, @@ -271,7 +269,7 @@ pub mod tests { v.get_next_vesting(t, None).unwrap(), Some(VestingEvent { time: 6, - amount: 3, + amount: 4, }) ); } else if t <= 10 { @@ -279,7 +277,7 @@ pub mod tests { let locked_float = 20.0 + (t - 5) as f64 * -20.0 / 6.0; assert_eq!( v.get_unvested_balance(t, None).unwrap(), - locked_float.ceil() as u64 + locked_float.floor() as u64 ); assert_eq!( v.get_next_vesting(t, None).unwrap(), @@ -342,7 +340,7 @@ pub mod tests { v.get_next_vesting(0, None).unwrap(), Some(VestingEvent { time: 8, - amount: 2, + amount: 3, }) ); assert_eq!(v.get_unvested_balance(5, None).unwrap(), 20); @@ -350,7 +348,7 @@ pub mod tests { v.get_next_vesting(5, None).unwrap(), Some(VestingEvent { time: 8, - amount: 2, + amount: 3, }) ); assert_eq!(v.get_unvested_balance(5 + 7 * 3, None).unwrap(), 0); @@ -362,7 +360,7 @@ pub mod tests { let locked_for_period = v.get_unvested_balance(t, None).unwrap(); // Linearly interpolate from (0, 20) to (7, 0) let locked_float = f64::max(20.0 * (1.0 - period as f64 / 7.0), 0.0); - assert_eq!(locked_for_period, locked_float.ceil() as u64); + assert_eq!(locked_for_period, locked_float.floor() as u64); for _t_in_period in 0..3 { assert_eq!(v.get_unvested_balance(t, None).unwrap(), locked_for_period); if period < 7 { @@ -394,12 +392,12 @@ pub mod tests { assert_eq!(v.get_unvested_balance(5 + 7 * 3, None).unwrap(), 20); assert_eq!(v.get_unvested_balance(4, Some(5)).unwrap(), 20); assert_eq!(v.get_unvested_balance(5 + 7 * 3, Some(5)).unwrap(), 0); - assert_eq!(v.get_unvested_balance(5 + 7 * 3, Some(6)).unwrap(), 3); + assert_eq!(v.get_unvested_balance(5 + 7 * 3, Some(6)).unwrap(), 2); assert_eq!( v.get_next_vesting(4 + 7 * 3, Some(5)).unwrap(), Some(VestingEvent { time: 26, - amount: 3, + amount: 2, }) ); assert_eq!(v.get_next_vesting(4 + 7 * 3, None).unwrap(), None); @@ -411,7 +409,7 @@ pub mod tests { let locked_for_period = v.get_unvested_balance(t, Some(5)).unwrap(); // Linearly interpolate from (0, 20) to (7, 0) let locked_float = f64::max(20.0 * (1.0 - period as f64 / 7.0), 0.0); - assert_eq!(locked_for_period, locked_float.ceil() as u64); + assert_eq!(locked_for_period, locked_float.floor() as u64); for _t_in_period in 0..3 { assert_eq!( v.get_unvested_balance(t, Some(5)).unwrap(), @@ -461,7 +459,7 @@ pub mod tests { v.get_next_vesting(1 << 60, None).unwrap(), Some(VestingEvent { time: (1 << 60) + 1, - amount: 0, + amount: 1, }) ) } @@ -476,15 +474,13 @@ pub mod tests { num_periods: 72, }; - let tokens_per_period = 1_000 / 72; - let value = v.get_unvested_balance(1, None).unwrap(); assert_eq!(value, 1000); assert_eq!( v.get_next_vesting(1, None).unwrap(), Some(VestingEvent { time: one_month.try_into().unwrap(), - amount: 13, + amount: 14, }) ); @@ -497,14 +493,14 @@ pub mod tests { .unwrap(), Some(VestingEvent { time: one_month.try_into().unwrap(), - amount: 13, + amount: 14, }) ); let value = v .get_unvested_balance(one_month.try_into().unwrap(), None) .unwrap(); - assert_eq!(value, 1000 - tokens_per_period); + assert_eq!(value, 986); assert_eq!( v.get_next_vesting(one_month.try_into().unwrap(), None) .unwrap(), @@ -517,7 +513,7 @@ pub mod tests { let value = v .get_unvested_balance((one_month * 2 - 1).try_into().unwrap(), None) .unwrap(); - assert_eq!(value, 1000 - tokens_per_period); + assert_eq!(value, 986); assert_eq!( v.get_next_vesting((one_month * 2 - 1).try_into().unwrap(), None) .unwrap(), @@ -530,7 +526,7 @@ pub mod tests { let value = v .get_unvested_balance((one_month * 2).try_into().unwrap(), None) .unwrap(); - assert_eq!(value, 973); + assert_eq!(value, 972); assert_eq!( v.get_next_vesting((one_month * 2).try_into().unwrap(), None) .unwrap(), @@ -543,14 +539,14 @@ pub mod tests { let value = v .get_unvested_balance((one_month * 72 - 1).try_into().unwrap(), None) .unwrap(); - assert_eq!(value, 14); + assert_eq!(value, 13); assert_eq!( v.get_next_vesting((one_month * 72 - 1).try_into().unwrap(), None) .unwrap(), Some(VestingEvent { time: (one_month * 72).try_into().unwrap(), - amount: 14, + amount: 13, }) ); diff --git a/staking/tests/vesting_test.ts b/staking/tests/vesting_test.ts index 23035f4b..bdd11387 100644 --- a/staking/tests/vesting_test.ts +++ b/staking/tests/vesting_test.ts @@ -133,7 +133,7 @@ describe("vesting", async () => { VestingAccountState.UnvestedTokensFullyLocked == stakeAccount.getVestingAccountState(await samConnection.getTime()) ); - const firstUnlock = Math.floor((100 * 10 ** 6) / 72) * 10 ** -6; + const firstUnlock = Math.ceil((100 * 10 ** 6) / 72) * 10 ** -6; assert( stakeAccount .getNetExcessGovernanceAtVesting(await samConnection.getTime()) @@ -211,9 +211,9 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locked: PythBalance.fromString("98.611112"), + locked: PythBalance.fromString("98.611111"), }, - locked: { locked: PythBalance.fromString("2.388888") }, + locked: { locked: PythBalance.fromString("2.388889") }, }, await samConnection.getTime() ); @@ -235,10 +235,10 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locked: PythBalance.fromString("98.611112"), + locked: PythBalance.fromString("98.611111"), }, locked: { - locked: PythBalance.fromString("2.388888"), + locked: PythBalance.fromString("2.388889"), locking: PythBalance.fromString("1"), }, }, @@ -263,11 +263,11 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locked: PythBalance.fromString("98.611112"), + locked: PythBalance.fromString("98.611111"), }, locked: { preunlocking: PythBalance.fromString("1"), - locked: PythBalance.fromString("1.388888"), + locked: PythBalance.fromString("1.388889"), locking: PythBalance.fromString("1"), }, }, @@ -277,7 +277,7 @@ describe("vesting", async () => { assert( samStakeAccount .getNetExcessGovernanceAtVesting(await samConnection.getTime()) - .eq(PythBalance.fromString("3.777777").toBN()) + .eq(PythBalance.fromString("3.777778").toBN()) ); }); @@ -303,12 +303,12 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locked: PythBalance.fromString("96.222223"), + locked: PythBalance.fromString("96.222222"), locking: PythBalance.fromString("1"), preunlocking: PythBalance.fromString("1.388889"), }, locked: { - preunlocking: PythBalance.fromString("3.388888"), + preunlocking: PythBalance.fromString("3.388889"), }, }, await samConnection.getTime() @@ -334,10 +334,10 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locked: PythBalance.fromString("97.222223"), + locked: PythBalance.fromString("97.222222"), unlocked: PythBalance.fromString("1.388889"), }, - withdrawable: PythBalance.fromString("3.388888"), + withdrawable: PythBalance.fromString("3.388889"), }, await samConnection.getTime() ); @@ -357,9 +357,9 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locked: PythBalance.fromString("97.222223"), + locked: PythBalance.fromString("97.222222"), }, - withdrawable: PythBalance.fromString("4.777777"), + withdrawable: PythBalance.fromString("4.777778"), }, await samConnection.getTime() ); @@ -399,10 +399,10 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locked: PythBalance.fromString("95.833334"), + locked: PythBalance.fromString("95.833333"), preunlocking: PythBalance.fromString("1.388889"), }, - withdrawable: PythBalance.fromString("4.777777"), + withdrawable: PythBalance.fromString("4.777778"), }, await samConnection.getTime() ); @@ -419,9 +419,9 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locked: PythBalance.fromString("95.833334"), + locked: PythBalance.fromString("95.833333"), }, - withdrawable: PythBalance.fromString("4.777777"), + withdrawable: PythBalance.fromString("4.777778"), locked: { unlocking: PythBalance.fromString("1.388889"), }, @@ -449,9 +449,9 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locked: PythBalance.fromString("95.833334"), + locked: PythBalance.fromString("95.833333"), }, - withdrawable: PythBalance.fromString("4.777777"), + withdrawable: PythBalance.fromString("4.777778"), locked: { unlocking: PythBalance.fromString("1.388889"), locking: PythBalance.fromString("1"), @@ -473,9 +473,9 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locked: PythBalance.fromString("95.833334"), + locked: PythBalance.fromString("95.833333"), }, - withdrawable: PythBalance.fromString("6.166666"), + withdrawable: PythBalance.fromString("6.166667"), locked: { locked: PythBalance.fromString("1"), }, @@ -497,9 +497,9 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - preunlocking: PythBalance.fromString("95.833334"), + preunlocking: PythBalance.fromString("95.833333"), }, - withdrawable: PythBalance.fromString("6.166666"), + withdrawable: PythBalance.fromString("6.166667"), locked: { preunlocking: PythBalance.fromString("1"), }, @@ -531,9 +531,9 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - unlocked: PythBalance.fromString("95.833334"), + unlocked: PythBalance.fromString("95.833333"), }, - withdrawable: PythBalance.fromString("7.166666"), + withdrawable: PythBalance.fromString("7.166667"), }, await samConnection.getTime() ); @@ -551,9 +551,9 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locking: PythBalance.fromString("95.833334"), + locking: PythBalance.fromString("95.833333"), }, - withdrawable: PythBalance.fromString("7.166666"), + withdrawable: PythBalance.fromString("7.166667"), }, await samConnection.getTime() ); @@ -565,10 +565,10 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locking: PythBalance.fromString("94.444445"), + locking: PythBalance.fromString("94.444444"), unlocked: PythBalance.fromString("1.388889"), }, - withdrawable: PythBalance.fromString("7.166666"), + withdrawable: PythBalance.fromString("7.166667"), }, await samConnection.getTime() ); @@ -586,10 +586,10 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locked: PythBalance.fromString("94.444445"), + locked: PythBalance.fromString("94.444444"), unlocked: PythBalance.fromString("1.388889"), }, - withdrawable: PythBalance.fromString("7.166666"), + withdrawable: PythBalance.fromString("7.166667"), }, await samConnection.getTime() ); @@ -608,10 +608,10 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locked: PythBalance.fromString("94.444445"), + locked: PythBalance.fromString("94.444444"), locking: PythBalance.fromString("1.388889"), }, - withdrawable: PythBalance.fromString("7.166666"), + withdrawable: PythBalance.fromString("7.166667"), }, await samConnection.getTime() ); @@ -628,11 +628,11 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locked: PythBalance.fromString("93.055556"), + locked: PythBalance.fromString("93.055555"), preunlocking: PythBalance.fromString("1.388889"), locking: PythBalance.fromString("1.388889"), }, - withdrawable: PythBalance.fromString("7.166666"), + withdrawable: PythBalance.fromString("7.166667"), }, await samConnection.getTime() ); @@ -653,10 +653,10 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locked: PythBalance.fromString("94.444445"), + locked: PythBalance.fromString("94.444444"), unlocked: PythBalance.fromString("1.388889"), }, - withdrawable: PythBalance.fromString("7.166666"), + withdrawable: PythBalance.fromString("7.166667"), }, await samConnection.getTime() ); @@ -680,10 +680,10 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - preunlocking: PythBalance.fromString("94.444445"), + preunlocking: PythBalance.fromString("94.444444"), unlocked: PythBalance.fromString("1.388889"), }, - withdrawable: PythBalance.fromString("7.166666"), + withdrawable: PythBalance.fromString("7.166667"), }, await samConnection.getTime() ); @@ -714,10 +714,10 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - locked: PythBalance.fromString("94.444445"), + locked: PythBalance.fromString("94.444444"), preunlocking: PythBalance.fromString("1.388889"), }, - withdrawable: PythBalance.fromString("7.166666"), + withdrawable: PythBalance.fromString("7.166667"), }, await samConnection.getTime() ); @@ -736,9 +736,9 @@ describe("vesting", async () => { sam.publicKey, { unvested: { - preunlocking: PythBalance.fromString("95.833334"), + preunlocking: PythBalance.fromString("95.833333"), }, - withdrawable: PythBalance.fromString("7.166666"), + withdrawable: PythBalance.fromString("7.166667"), }, await samConnection.getTime() ); @@ -824,9 +824,9 @@ describe("vesting", async () => { alice.publicKey, { unvested: { - unlocked: PythBalance.fromString("98.611112"), + unlocked: PythBalance.fromString("98.611111"), }, - withdrawable: PythBalance.fromString("1.388888"), + withdrawable: PythBalance.fromString("1.388889"), }, await aliceConnection.getTime() ); @@ -835,7 +835,7 @@ describe("vesting", async () => { await expectFailApi( aliceConnection.withdrawTokens( stakeAccount, - PythBalance.fromString("1.388889") + PythBalance.fromString("1.388890") ), "Amount exceeds withdrawable." ); @@ -850,9 +850,9 @@ describe("vesting", async () => { alice.publicKey, { unvested: { - unlocked: PythBalance.fromString("98.611112"), + unlocked: PythBalance.fromString("98.611111"), }, - withdrawable: PythBalance.fromString("0.388888"), + withdrawable: PythBalance.fromString("0.388889"), }, await aliceConnection.getTime() ); @@ -873,9 +873,9 @@ describe("vesting", async () => { alice.publicKey, { unvested: { - unlocked: PythBalance.fromString("97.222223"), + unlocked: PythBalance.fromString("97.222222"), }, - withdrawable: PythBalance.fromString("1.777777"), + withdrawable: PythBalance.fromString("1.777778"), }, await aliceConnection.getTime() );