[wip] Token haver plugin #92
Conversation
| let registrar = &ctx.accounts.registrar; | ||
| let voter_weight_record = &mut ctx.accounts.voter_weight_record; | ||
|
|
||
| let governance_program_id = ctx.accounts.token_owner_record.owner; |
There was a problem hiding this comment.
governance_program_id must be taken from Registrar and not from token_owner_record.owner to ensure the provided TOR belongs to the configured program.
Otherwise you could deploy your own program and manufacture any TOR which would pass this condition and exploit it.
get_token_owner_record_data() asserts that token_owner_record.owner == governance_program_id
|
|
||
| let governance_program_id = ctx.accounts.token_owner_record.owner; | ||
|
|
||
| let token_owner_record = token_owner_record::get_token_owner_record_data( |
There was a problem hiding this comment.
We have to ensure the provided token_owner_record matches the configured realm and governing_token_mint and belongs to the configured governance_program_id For that reason get_token_owner_record_data_for_realm_and_governing_mint(), which does the referential integrity checks, should be used to deserialise the account.
Hint: It's a good idea to draw a referential integrity graph between all instruction accounts and check if their relationship are enforced.
You can see the referential integrity for VoterWeightRecord is checked for realm and governing_token_mint in the Anchor declarative way, plus the owner_program is also checked implicitly by Anchor
TokenAccount.owner is checked against token_owner_record.governing_token_owner explicitly in the code.
| let nonzero_token_accounts: Vec<Account<TokenAccount>> = ctx | ||
| .remaining_accounts | ||
| .iter() | ||
| .filter(|account| !account.data_is_empty()) // filter out empty accounts, so we don't need to check them in UI |
There was a problem hiding this comment.
I would expect Account::<TokenAccount>::try_from in the next statement to fail if the account data is empty and I would rather throw an error for invalid accounts then silently ignoring them
| voter_weight_record.voter_weight_expiry = Some(Clock::get()?.slot); | ||
|
|
||
| // Set action and target to None to indicate the weight is valid for any action and target | ||
| voter_weight_record.weight_action = None; |
There was a problem hiding this comment.
These are default values and never changed by the program and the assignment is not required here
|
|
||
| // Set action and target to None to indicate the weight is valid for any action and target | ||
| voter_weight_record.weight_action = None; | ||
| voter_weight_record.weight_action_target = None; |
| // Throw an error if a token account's owner doesnt match token_owner_record.governing_token_owner | ||
| require_eq!( | ||
| account.owner, | ||
| token_owner_record.governing_token_owner, |
There was a problem hiding this comment.
I realised we actually don't need token_owner_record account for this instruction at all because the token_account.owner check can be done against voter_weight_record.governing_token_owner
| } | ||
|
|
||
| // Setup voter_weight | ||
| voter_weight_record.voter_weight = nonzero_token_accounts.len() as u64; |
There was a problem hiding this comment.
Maybe we could parametrise this formula in Registrar configuration to make this plugin more generic for frozen accounts? The options I'm thinking about would be:
- weight = Number of frozen accounts - the default as implemented now
- weight = sum(amounts) - this would require additional mint amount adjustments in case they would have different precisions (see VSR for reference)
I'm not sure if the extra effort would be justified for this use case
I also add a devcontainer w/ Dockerfile to be used for Codespaces and verifiable builds.