Split accounts test #258
Split accounts test #258
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -71,6 +71,9 @@ pub enum ErrorCode { | |||
| SplitTooManyTokens, | |||
| #[msg("Sanity check failed")] // 6033 | |||
| SanityCheckFailed, | |||
| #[msg("Other")] //6031 | |||
| #[msg("Other")] //6034 | |||
There was a problem hiding this comment.
I'd just move Other to be the last error
staking/programs/staking/src/lib.rs
Outdated
| // This check allows us to create an empty positions account on behalf of the recipient and | ||
| // not worry about moving positions from the source account to the new account. | ||
| require!( | ||
| source_stake_account_positions.num_positions()? == 0, |
There was a problem hiding this comment.
I think it's better to use source_stake_account_metadata.next_index == 0
| // all of these fields should be preserved in the result | ||
| start_date: 203, | ||
| period_duration: 100, | ||
| num_periods: 12, |
There was a problem hiding this comment.
Perhaps use the parameters we're going to use in the real vesting schedules for this test
There was a problem hiding this comment.
I didn't do this one because it makes the invariant test below much more expensive. I don't think the length of the start_date or duration should matter for the splitting logic though, so i think it's better to have the invariant test and not have this.
| match (remaining_schedule, transferred_schedule) { | ||
| ( | ||
| PeriodicVesting { | ||
| initial_balance: r, .. |
There was a problem hiding this comment.
I'd like to see a test that for an arbitrary t
if schedule.get_unvested_balance(t) <= total then
transfered_schedule.get_unvested_balance(t) <= transferred
and transfered_schedule.get_unvested_balance(t) <= total - transferred
This proves that the invariants are preserved by this transformation.
| @@ -709,6 +714,28 @@ pub mod tests { | |||
| } | |||
| } | |||
|
|
|||
| for timestamp in 0..(START_TIMESTAMP + (PERIOD_DURATION * NUM_PERIODS + 1) as i64) { | |||
There was a problem hiding this comment.
You could change the increment of this for loop to be PERIOD_DURATION and then it would run fast for big values of PERIOD_DURATION
* Draft * Add authority * Clippy * Checkpoint * Do it * Rename error * Update stuff * Use epoch_of_snapshot * Checkpoint * Scaffolding * Checkpoint * Cleanup * Cleanup test * Another round * Cleanup * Restore all tests * add todos * Cleanup idls * Throw error since it's not implemented * Add some comments * Box everything * Add soruce * Add more comments * Add another comment * First implementation * Delete current request * add bumps * Fix bug * Tests works * Update idls * Clippy * Clippy * Add actual tests * Clippy * Cleanup * Fix tests * Split accounts test (#258) * vesting tests * more tests * better invariant tests * minor cleanups * cleanup implementation * idl * ok fix the ts tests * refactor * refactor * fix * cleanup * pr comments * minor * Cleanup * Comment * Add another test with full amount * Sorry --------- Co-authored-by: Jayant Krishnamurthy <[email protected]>
two changes per our conversation:
I didn't add the check that all tokens in the account are locked because it turns out that check doesn't help with the vesting account splitting logic.