Address unaligned references, add padding

- Make Registrar zero-copy. While it was using borsh for serialization,
  the array of voting mint configs couldn't be serialized without
  unaligned references.
- Reorganize all zero_copy fields such that switching to repr(C) would
  not make a difference. (just for safety in case that happens)
- Add static asserts on the sizes of all account structs, so any changes
  are visible very early.
- Add padding to structs that didn't have it yet.
This commit is contained in:
Christian Kamm 2021-12-09 08:22:22 +01:00
parent ddf37c4de0
commit decdd1230c
20 changed files with 87 additions and 59 deletions

1
Cargo.lock generated
View File

@ -3223,6 +3223,7 @@ dependencies = [
"spl-associated-token-account",
"spl-governance",
"spl-token 3.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
"static_assertions",
]
[[package]]

View File

@ -27,6 +27,7 @@ anchor-spl = { git = "https://github.com/microwavedcola1/anchor.git", branch = "
# programs/voter-stake-registry/tests/fixtures/spl_governance.so is built from.
spl-governance = { git = "https://github.com/solana-labs/solana-program-library", rev = "75ddd9bb229396427977fb679a0763630e83bce6", features = ["no-entrypoint"] }
solana-program = "^1.8.1"
static_assertions = "1.1"
[dev-dependencies]
solana-sdk = "^1.8.1"

View File

@ -7,7 +7,7 @@ use anchor_spl::token::{Token, TokenAccount};
#[derive(Accounts)]
pub struct Clawback<'info> {
#[account(has_one = clawback_authority)]
pub registrar: Box<Account<'info, Registrar>>,
pub registrar: AccountLoader<'info, Registrar>,
pub clawback_authority: Signer<'info>,
// checking the PDA address it just an extra precaution,
@ -65,7 +65,7 @@ impl<'info> Clawback<'info> {
/// that have already vested in place.
pub fn clawback(ctx: Context<Clawback>, deposit_entry_index: u8) -> Result<()> {
// Load the accounts.
let registrar = &ctx.accounts.registrar;
let registrar = &ctx.accounts.registrar.load()?;
let voter = &mut ctx.accounts.voter.load_mut()?;
let deposit_entry = voter.active_deposit_mut(deposit_entry_index)?;
require!(

View File

@ -9,7 +9,7 @@ use anchor_spl::token::{Mint, Token, TokenAccount};
#[derive(Accounts)]
pub struct ConfigureVotingMint<'info> {
#[account(mut, has_one = realm_authority)]
pub registrar: Box<Account<'info, Registrar>>,
pub registrar: AccountLoader<'info, Registrar>,
pub realm_authority: Signer<'info>,
/// Token account that all funds for this mint will be stored in
@ -91,7 +91,7 @@ pub fn configure_voting_mint(
grant_authority: Option<Pubkey>,
) -> Result<()> {
require!(lockup_saturation_secs > 0, LockupSaturationMustBePositive);
let registrar = &mut ctx.accounts.registrar;
let registrar = &mut ctx.accounts.registrar.load_mut()?;
require!(
(idx as usize) < registrar.voting_mints.len(),
OutOfBoundsVotingMintConfigIndex
@ -107,6 +107,7 @@ pub fn configure_voting_mint(
lockup_scaled_factor,
lockup_saturation_secs,
grant_authority: grant_authority.unwrap_or(Pubkey::new_from_array([0; 32])),
padding: [0; 31],
};
// Check for overflow in vote weight

View File

@ -5,7 +5,7 @@ use anchor_spl::token::Mint;
#[derive(Accounts)]
pub struct CreateDepositEntry<'info> {
pub registrar: Box<Account<'info, Registrar>>,
pub registrar: AccountLoader<'info, Registrar>,
// checking the PDA address it just an extra precaution,
// the other constraints must be exhaustive
@ -39,7 +39,7 @@ pub fn create_deposit_entry(
allow_clawback: bool,
) -> Result<()> {
// Load accounts.
let registrar = &ctx.accounts.registrar;
let registrar = &ctx.accounts.registrar.load()?;
let voter = &mut ctx.accounts.voter.load_mut()?;
// Get the exchange rate entry associated with this deposit.

View File

@ -17,7 +17,7 @@ pub struct CreateRegistrar<'info> {
payer = payer,
space = 8 + size_of::<Registrar>()
)]
pub registrar: Box<Account<'info, Registrar>>,
pub registrar: AccountLoader<'info, Registrar>,
/// An spl-governance realm
///
@ -52,7 +52,7 @@ pub struct CreateRegistrar<'info> {
/// To use the registrar, call ConfigVotingMint to register token mints that may be
/// used for voting.
pub fn create_registrar(ctx: Context<CreateRegistrar>, registrar_bump: u8) -> Result<()> {
let registrar = &mut ctx.accounts.registrar;
let registrar = &mut ctx.accounts.registrar.load_init()?;
registrar.bump = registrar_bump;
registrar.governance_program_id = ctx.accounts.governance_program_id.key();
registrar.realm = ctx.accounts.realm.key();

View File

@ -8,7 +8,7 @@ use std::mem::size_of;
#[derive(Accounts)]
#[instruction(voter_bump: u8, voter_weight_record_bump: u8)]
pub struct CreateVoter<'info> {
pub registrar: Box<Account<'info, Registrar>>,
pub registrar: AccountLoader<'info, Registrar>,
#[account(
init_if_needed,
@ -78,7 +78,7 @@ pub fn create_voter(
}
// Load accounts.
let registrar = &ctx.accounts.registrar;
let registrar = &ctx.accounts.registrar.load()?;
let voter_authority = ctx.accounts.voter_authority.key();
// Init the voter if is hasn't been already.

View File

@ -6,7 +6,7 @@ use std::convert::TryFrom;
#[derive(Accounts)]
pub struct Deposit<'info> {
pub registrar: Box<Account<'info, Registrar>>,
pub registrar: AccountLoader<'info, Registrar>,
// checking the PDA address it just an extra precaution,
// the other constraints must be exhaustive
@ -59,7 +59,7 @@ pub fn deposit(ctx: Context<Deposit>, deposit_entry_index: u8, amount: u64) -> R
return Ok(());
}
let registrar = &ctx.accounts.registrar;
let registrar = &ctx.accounts.registrar.load()?;
let voter = &mut ctx.accounts.voter.load_mut()?;
let d_entry = voter.active_deposit_mut(deposit_entry_index)?;
@ -101,13 +101,15 @@ pub fn deposit(ctx: Context<Deposit>, deposit_entry_index: u8, amount: u64) -> R
d_entry.amount_deposited_native += amount;
d_entry.amount_initially_locked_native += amount;
let start_ts = d_entry.lockup.start_ts;
let end_ts = d_entry.lockup.end_ts;
msg!(
"Deposited amount {} at deposit index {} with lockup kind {:?}, and start ts {}, end ts {}",
amount,
deposit_entry_index,
d_entry.lockup.kind,
d_entry.lockup.start_ts,
d_entry.lockup.end_ts,
start_ts,
end_ts,
);
Ok(())

View File

@ -16,7 +16,7 @@ use std::mem::size_of;
amount: u64,
)]
pub struct Grant<'info> {
pub registrar: Box<Account<'info, Registrar>>,
pub registrar: AccountLoader<'info, Registrar>,
#[account(
init_if_needed,
@ -89,7 +89,7 @@ pub fn grant(
amount: u64,
) -> Result<()> {
// Load accounts.
let registrar = &ctx.accounts.registrar;
let registrar = &ctx.accounts.registrar.load()?;
let voter_authority = ctx.accounts.voter_authority.key();
// Get the exchange rate entry associated with this deposit.
@ -145,13 +145,15 @@ pub fn grant(
d_entry.amount_deposited_native = amount;
d_entry.amount_initially_locked_native = amount;
let start_ts = d_entry.lockup.start_ts;
let end_ts = d_entry.lockup.end_ts;
msg!(
"Granted amount {} at deposit index {} with lockup kind {:?}, and start ts {}, end ts {}",
amount,
free_entry_idx,
d_entry.lockup.kind,
d_entry.lockup.start_ts,
d_entry.lockup.end_ts,
start_ts,
end_ts,
);
Ok(())

View File

@ -6,7 +6,7 @@ use anchor_lang::prelude::*;
pub struct ResetLockup<'info> {
// checking the PDA address it just an extra precaution,
// the other constraints must be exhaustive
pub registrar: Box<Account<'info, Registrar>>,
pub registrar: AccountLoader<'info, Registrar>,
#[account(
mut,
seeds = [registrar.key().as_ref(), b"voter".as_ref(), voter_authority.key().as_ref()],
@ -25,7 +25,7 @@ pub fn reset_lockup(
deposit_entry_index: u8,
periods: u32,
) -> Result<()> {
let registrar = &ctx.accounts.registrar;
let registrar = &ctx.accounts.registrar.load()?;
let voter = &mut ctx.accounts.voter.load_mut()?;
let d = voter.active_deposit_mut(deposit_entry_index)?;

View File

@ -7,14 +7,14 @@ use std::str::FromStr;
#[instruction(time_offset: i64)]
pub struct SetTimeOffset<'info> {
#[account(mut, has_one = realm_authority)]
pub registrar: Box<Account<'info, Registrar>>,
pub registrar: AccountLoader<'info, Registrar>,
pub realm_authority: Signer<'info>,
}
/// A debug-only instruction that advances the time.
pub fn set_time_offset(ctx: Context<SetTimeOffset>, time_offset: i64) -> Result<()> {
let allowed_program = Pubkey::from_str("GovernanceProgram11111111111111111111111111").unwrap();
let registrar = &mut ctx.accounts.registrar;
let registrar = &mut ctx.accounts.registrar.load_mut()?;
require!(
registrar.governance_program_id == allowed_program,
ErrorCode::DebugInstruction

View File

@ -6,7 +6,7 @@ use anchor_lang::prelude::*;
// exchange rates.
#[derive(Accounts)]
pub struct UpdateMaxVoteWeight<'info> {
pub registrar: Box<Account<'info, Registrar>>,
pub registrar: AccountLoader<'info, Registrar>,
// TODO: SPL governance has not yet implemented this.
pub max_vote_weight_record: UncheckedAccount<'info>,
}
@ -19,7 +19,7 @@ pub struct UpdateMaxVoteWeight<'info> {
/// all tokens fits into a u64 *after* converting into common decimals, as
/// defined by the registrar's `rate_decimal` field.
pub fn update_max_vote_weight<'info>(ctx: Context<UpdateMaxVoteWeight>) -> Result<()> {
let registrar = &ctx.accounts.registrar;
let registrar = &ctx.accounts.registrar.load()?;
let _max_vote_weight = registrar.max_vote_weight(ctx.remaining_accounts)?;
// TODO: SPL governance has not yet implemented this feature.
// When it has, probably need to write the result into an account,

View File

@ -4,7 +4,7 @@ use anchor_lang::prelude::*;
#[derive(Accounts)]
pub struct UpdateVoterWeightRecord<'info> {
pub registrar: Box<Account<'info, Registrar>>,
pub registrar: AccountLoader<'info, Registrar>,
// checking the PDA address it just an extra precaution,
// the other constraints must be exhaustive
@ -18,9 +18,9 @@ pub struct UpdateVoterWeightRecord<'info> {
mut,
seeds = [registrar.key().as_ref(), b"voter-weight-record".as_ref(), voter.load()?.voter_authority.key().as_ref()],
bump = voter.load()?.voter_weight_record_bump,
constraint = voter_weight_record.realm == registrar.realm,
constraint = voter_weight_record.realm == registrar.load()?.realm,
constraint = voter_weight_record.governing_token_owner == voter.load()?.voter_authority,
constraint = voter_weight_record.governing_token_mint == registrar.realm_governing_token_mint,
constraint = voter_weight_record.governing_token_mint == registrar.load()?.realm_governing_token_mint,
)]
pub voter_weight_record: Account<'info, VoterWeightRecord>,
@ -34,7 +34,7 @@ pub struct UpdateVoterWeightRecord<'info> {
/// This "revise" instruction must be called immediately before voting, in
/// the same transaction.
pub fn update_voter_weight_record(ctx: Context<UpdateVoterWeightRecord>) -> Result<()> {
let registrar = &ctx.accounts.registrar;
let registrar = &ctx.accounts.registrar.load()?;
let voter = ctx.accounts.voter.load()?;
let record = &mut ctx.accounts.voter_weight_record;
record.voter_weight = voter.weight(&registrar)?;

View File

@ -5,7 +5,7 @@ use anchor_spl::token::{self, Token, TokenAccount};
#[derive(Accounts)]
pub struct Withdraw<'info> {
pub registrar: Box<Account<'info, Registrar>>,
pub registrar: AccountLoader<'info, Registrar>,
// checking the PDA address it just an extra precaution,
// the other constraints must be exhaustive
@ -36,9 +36,9 @@ pub struct Withdraw<'info> {
mut,
seeds = [registrar.key().as_ref(), b"voter-weight-record".as_ref(), voter_authority.key().as_ref()],
bump = voter.load()?.voter_weight_record_bump,
constraint = voter_weight_record.realm == registrar.realm,
constraint = voter_weight_record.realm == registrar.load()?.realm,
constraint = voter_weight_record.governing_token_owner == voter.load()?.voter_authority,
constraint = voter_weight_record.governing_token_mint == registrar.realm_governing_token_mint,
constraint = voter_weight_record.governing_token_mint == registrar.load()?.realm_governing_token_mint,
)]
pub voter_weight_record: Account<'info, VoterWeightRecord>,
@ -74,7 +74,7 @@ impl<'info> Withdraw<'info> {
/// `amount` is in units of the native currency being withdrawn.
pub fn withdraw(ctx: Context<Withdraw>, deposit_entry_index: u8, amount: u64) -> Result<()> {
// Load the accounts.
let registrar = &ctx.accounts.registrar;
let registrar = &ctx.accounts.registrar.load()?;
let voter = &mut ctx.accounts.voter.load_mut()?;
// Governance may forbid withdraws, for example when engaged in a vote.
@ -110,13 +110,15 @@ pub fn withdraw(ctx: Context<Withdraw>, deposit_entry_index: u8, amount: u64) ->
amount,
)?;
let start_ts = deposit_entry.lockup.start_ts;
let end_ts = deposit_entry.lockup.end_ts;
msg!(
"Withdrew amount {} at deposit index {} with lockup kind {:?}, and start ts {}, end ts {}",
amount,
deposit_entry_index,
deposit_entry.lockup.kind,
deposit_entry.lockup.start_ts,
deposit_entry.lockup.end_ts,
start_ts,
end_ts,
);
// Update the voter weight record

View File

@ -7,6 +7,9 @@ mod error;
mod instructions;
pub mod state;
#[macro_use]
extern crate static_assertions;
// The program address.
declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS");

View File

@ -9,11 +9,8 @@ use std::convert::TryFrom;
#[zero_copy]
#[derive(Default)]
pub struct DepositEntry {
// True if the deposit entry is being used.
pub is_used: bool,
// Points to the VotingMintConfig this deposit uses.
pub voting_mint_config_idx: u8,
// Locked state.
pub lockup: Lockup,
/// Amount in deposited, in native currency. Withdraws of vested tokens
/// directly reduce this amount.
@ -32,11 +29,18 @@ pub struct DepositEntry {
/// which should not change due to withdraws.
pub amount_initially_locked_native: u64,
// True if the deposit entry is being used.
pub is_used: bool,
/// If the clawback authority is allowed to extract locked tokens.
pub allow_clawback: bool,
// Locked state.
pub lockup: Lockup,
// Points to the VotingMintConfig this deposit uses.
pub voting_mint_config_idx: u8,
pub padding: [u8; 13],
}
const_assert!(std::mem::size_of::<DepositEntry>() == 32 + 2 * 8 + 3 + 13);
impl DepositEntry {
/// # Voting Power Caclulation

View File

@ -21,9 +21,7 @@ pub const SECS_PER_MONTH: i64 = 10;
pub const SECS_PER_MONTH: i64 = 365 * SECS_PER_DAY / 12;
#[zero_copy]
#[derive(AnchorSerialize, AnchorDeserialize)]
pub struct Lockup {
pub kind: LockupKind,
/// Start of the lockup.
///
/// Note, that if start_ts is in the future, the funds are nevertheless
@ -32,12 +30,17 @@ pub struct Lockup {
/// Similarly vote power computations don't care about start_ts and always
/// assume the full interval from now to end_ts.
pub start_ts: i64,
// End of the lockup.
/// End of the lockup.
pub end_ts: i64,
/// Type of lockup.
pub kind: LockupKind,
// Empty bytes for future upgrades.
// TODO: what kinds of upgrades do we foresee?
pub padding: [u8; 16],
pub padding: [u8; 15],
}
const_assert!(std::mem::size_of::<Lockup>() == 2 * 8 + 1 + 15);
impl Default for Lockup {
fn default() -> Self {
@ -45,7 +48,7 @@ impl Default for Lockup {
kind: LockupKind::None,
start_ts: 0,
end_ts: 0,
padding: [0; 16],
padding: [0; 15],
}
}
}
@ -59,7 +62,7 @@ impl Lockup {
end_ts: start_ts
.checked_add(i64::from(periods).checked_mul(kind.period_secs()).unwrap())
.unwrap(),
padding: [0; 16],
padding: [0; 15],
})
}
@ -651,7 +654,7 @@ mod tests {
kind: LockupKind::Cliff,
start_ts,
end_ts,
padding: [0u8; 16],
padding: [0u8; 15],
};
let days_left = l.periods_left(curr_ts)?;
assert_eq!(days_left, t.expected_days_left);
@ -666,7 +669,7 @@ mod tests {
kind: LockupKind::Monthly,
start_ts,
end_ts,
padding: [0u8; 16],
padding: [0u8; 15],
};
let months_left = l.periods_left(curr_ts)?;
assert_eq!(months_left, t.expected_months_left);
@ -686,8 +689,9 @@ mod tests {
start_ts,
end_ts,
kind: t.kind,
padding: [0u8; 16],
padding: [0u8; 15],
},
padding: [0; 13],
};
let curr_ts = start_ts + days_to_secs(t.curr_day);
let power = d.voting_power_locked(curr_ts, t.amount_deposited, MAX_SECS_LOCKED)?;

View File

@ -4,7 +4,7 @@ use anchor_lang::prelude::*;
use anchor_spl::token::Mint;
/// Instance of a voting rights distributor.
#[account]
#[account(zero_copy)]
#[derive(Default)]
pub struct Registrar {
pub governance_program_id: Pubkey,
@ -12,13 +12,15 @@ pub struct Registrar {
pub realm_governing_token_mint: Pubkey,
pub realm_authority: Pubkey,
pub clawback_authority: Pubkey,
pub bump: u8,
// The length should be adjusted for one's use case.
pub voting_mints: [VotingMintConfig; 2],
/// Debug only: time offset, to allow tests to move forward in time.
pub time_offset: i64,
pub bump: u8,
pub padding: [u8; 31],
}
const_assert!(std::mem::size_of::<Registrar>() == 5 * 32 + 2 * 120 + 8 + 1 + 31);
impl Registrar {
pub fn clock_unix_timestamp(&self) -> i64 {

View File

@ -9,10 +9,12 @@ use spl_governance::state::token_owner_record;
pub struct Voter {
pub voter_authority: Pubkey,
pub registrar: Pubkey,
pub deposits: [DepositEntry; 32],
pub voter_bump: u8,
pub voter_weight_record_bump: u8,
pub deposits: [DepositEntry; 32],
pub padding: [u8; 30],
}
const_assert!(std::mem::size_of::<Voter>() == 2 * 32 + 32 * 64 + 2 + 30);
impl Voter {
pub fn weight(&self, registrar: &Registrar) -> Result<u64> {

View File

@ -10,13 +10,13 @@ const SCALED_FACTOR_BASE: u64 = 1_000_000_000;
/// See documentation of configure_voting_mint for details on how
/// native token amounts convert to vote weight.
#[zero_copy]
#[derive(AnchorSerialize, AnchorDeserialize, Default)]
#[derive(Default)]
pub struct VotingMintConfig {
/// Mint for this entry.
pub mint: Pubkey,
/// Number of digits to shift native amounts, applying a 10^digit_shift factor.
pub digit_shift: i8,
/// The authority that is allowed to push grants into voters
pub grant_authority: Pubkey,
/// Vote weight factor for deposits, in 1/SCALED_FACTOR_BASE units.
pub deposit_scaled_factor: u64,
@ -27,9 +27,13 @@ pub struct VotingMintConfig {
/// Number of seconds of lockup needed to reach the maximum lockup bonus.
pub lockup_saturation_secs: u64,
/// The authority that is allowed to push grants into voters
pub grant_authority: Pubkey,
/// Number of digits to shift native amounts, applying a 10^digit_shift factor.
pub digit_shift: i8,
// Empty bytes for future upgrades.
pub padding: [u8; 31],
}
const_assert!(std::mem::size_of::<VotingMintConfig>() == 2 * 32 + 3 * 8 + 1 + 31);
impl VotingMintConfig {
/// Converts an amount in this voting mints's native currency