-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split accounts 3 #247
Split accounts 3 #247
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
) -> Result<(VestingSchedule, VestingSchedule)> { | ||
let remaining_amount = total_amount | ||
.checked_sub(transferred_amount) | ||
.ok_or(ErrorCode::GenericOverflow)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could even throw SplitTooManyTokens
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise that error is not used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the SplitTooManyTokens code implies that this method is only called in the split/accept methods. what happens if someone calls this somewhere else?
@@ -10,31 +10,27 @@ import { | |||
import path from "path"; | |||
import { Keypair, PublicKey, Transaction } from "@solana/web3.js"; | |||
import { StakeConnection, PythBalance, VestingAccountState } from "../app"; | |||
import { BN, Wallet } from "@project-serum/anchor"; | |||
import { assertBalanceMatches } from "./utils/api_utils"; | |||
import { ErrorCode, BN, Wallet, AnchorError } from "@project-serum/anchor"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some unused imports here
unlocked: PythBalance.fromString("33"), | ||
}, | ||
} | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonder if we should check more stuff about Alice's account
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a call to updateVoterWeight
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine as a test. You already have the tests in voter_weight_test
around how the different types of tokens are translated into voter weights, so not sure what it achieves to do effectively the same test here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol approving this myself
This PR has the full rust implementation.
Rust tests and more ts tests will be made subsequently.