From fca6228cd2b5e190f69515566f0027478a9a1846 Mon Sep 17 00:00:00 2001 From: aalu1418 <50029043+aalu1418@users.noreply.github.com> Date: Thu, 9 Jan 2025 15:19:48 -0700 Subject: [PATCH] cleanup: address feedback on write index validation --- .../ccip-router/src/instructions/v1/pools.rs | 101 +++++++++++++++++- .../instructions/v1/token_admin_registry.rs | 15 +-- .../programs/ccip-router/src/token_context.rs | 96 ----------------- 3 files changed, 108 insertions(+), 104 deletions(-) diff --git a/chains/solana/contracts/programs/ccip-router/src/instructions/v1/pools.rs b/chains/solana/contracts/programs/ccip-router/src/instructions/v1/pools.rs index 895bc74b2..7a8658827 100644 --- a/chains/solana/contracts/programs/ccip-router/src/instructions/v1/pools.rs +++ b/chains/solana/contracts/programs/ccip-router/src/instructions/v1/pools.rs @@ -212,8 +212,9 @@ pub fn validate_and_parse_token_accounts<'info>( remaining_accounts.iter().map(|x| x.is_writable).collect(); expected_is_writable.append(&mut remaining_is_writable); for (i, is_writable) in expected_is_writable.iter().enumerate() { - require!( - token_admin_registry_account.is_writable(i as u8) == *is_writable, + require_eq!( + token_admin_registry_account.is_writable(i as u8), + *is_writable, CcipRouterError::InvalidInputsLookupTableAccountWritable ); } @@ -319,3 +320,99 @@ pub fn u64_to_le_u256(v: u64) -> [u8; 32] { out[..8].copy_from_slice(v.to_le_bytes().as_slice()); out } + +impl TokenAdminRegistry { + // set writable inserts bits from left to right + // index 0 is left-most bit + pub fn set_writable(&mut self, index: u8) { + match index < 128 { + true => { + self.writable_indexes[0] |= 1 << (127 - index); + } + false => { + self.writable_indexes[1] |= 1 << (255 - index); + } + } + } + + pub fn is_writable(&self, index: u8) -> bool { + match index < 128 { + true => self.writable_indexes[0] & 1 << (127 - index) != 0, + false => self.writable_indexes[1] & 1 << (255 - index) != 0, + } + } + + pub fn reset_writable(&mut self) { + self.writable_indexes = [0, 0]; + } +} + +#[cfg(test)] +mod tests { + use bytemuck::Zeroable; + use solana_program::pubkey::Pubkey; + + use crate::TokenAdminRegistry; + #[test] + fn set_writable() { + let mut state = TokenAdminRegistry { + version: 0, + administrator: Pubkey::zeroed(), + pending_administrator: Pubkey::zeroed(), + lookup_table: Pubkey::zeroed(), + writable_indexes: [0, 0], + }; + + state.set_writable(0); + state.set_writable(128); + assert_eq!(state.writable_indexes[0], 2u128.pow(127)); + assert_eq!(state.writable_indexes[1], 2u128.pow(127)); + + state.reset_writable(); + assert_eq!(state.writable_indexes[0], 0); + assert_eq!(state.writable_indexes[1], 0); + + state.set_writable(0); + state.set_writable(2); + state.set_writable(127); + state.set_writable(128); + state.set_writable(2 + 128); + state.set_writable(255); + assert_eq!( + state.writable_indexes[0], + 2u128.pow(127) + 2u128.pow(127 - 2) + 2u128.pow(0) + ); + assert_eq!( + state.writable_indexes[1], + 2u128.pow(127) + 2u128.pow(127 - 2) + 2u128.pow(0) + ); + } + + #[test] + fn check_writable() { + let state = TokenAdminRegistry { + version: 0, + administrator: Pubkey::zeroed(), + pending_administrator: Pubkey::zeroed(), + lookup_table: Pubkey::zeroed(), + writable_indexes: [ + 2u128.pow(127 - 7) + 2u128.pow(127 - 2) + 2u128.pow(127 - 4), + 2u128.pow(127 - 8) + 2u128.pow(127 - 56) + 2u128.pow(127 - 100), + ], + }; + + assert_eq!(state.is_writable(0), false); + assert_eq!(state.is_writable(128), false); + assert_eq!(state.is_writable(255), false); + + assert_eq!(state.writable_indexes[0].count_ones(), 3); + assert_eq!(state.writable_indexes[1].count_ones(), 3); + + assert_eq!(state.is_writable(7), true); + assert_eq!(state.is_writable(2), true); + assert_eq!(state.is_writable(4), true); + assert_eq!(state.is_writable(128 + 8), true); + assert_eq!(state.is_writable(128 + 56), true); + assert_eq!(state.is_writable(128 + 100), true); + } +} diff --git a/chains/solana/contracts/programs/ccip-router/src/instructions/v1/token_admin_registry.rs b/chains/solana/contracts/programs/ccip-router/src/instructions/v1/token_admin_registry.rs index c76389d9c..2c5a87b20 100644 --- a/chains/solana/contracts/programs/ccip-router/src/instructions/v1/token_admin_registry.rs +++ b/chains/solana/contracts/programs/ccip-router/src/instructions/v1/token_admin_registry.rs @@ -1,6 +1,5 @@ use anchor_lang::prelude::*; use anchor_spl::associated_token::get_associated_token_address_with_program_id; -use bytemuck::Zeroable; use solana_program::{address_lookup_table::state::AddressLookupTable, log::sol_log}; use crate::{ @@ -11,6 +10,8 @@ use crate::{ FEE_BILLING_TOKEN_CONFIG, TOKEN_ADMIN_REGISTRY_SEED, }; +const MINIMUM_TOKEN_POOL_ACCOUNTS: usize = 9; + pub fn register_token_admin_registry_via_get_ccip_admin( ctx: Context, mint: Pubkey, // should we validate that this is a real token program? @@ -78,14 +79,15 @@ pub fn set_pool( } // validate lookup table contains minimum required accounts if not zero address - if new_pool != Pubkey::zeroed() { + if new_pool != Pubkey::default() { // deserialize lookup table account let lookup_table_data = &mut &ctx.accounts.pool_lookuptable.data.borrow()[..]; let lookup_table_account: AddressLookupTable = AddressLookupTable::deserialize(lookup_table_data) .map_err(|_| CcipRouterError::InvalidInputsLookupTableAccounts)?; - require!( - lookup_table_account.addresses.len() >= 9, + require_gte!( + lookup_table_account.addresses.len(), + MINIMUM_TOKEN_POOL_ACCOUNTS, CcipRouterError::InvalidInputsLookupTableAccounts ); @@ -128,8 +130,9 @@ pub fn set_pool( sol_log(&acc.to_string()); sol_log(&lookup_table_account.addresses[i].to_string()); } - require!( - lookup_table_account.addresses[i] == *acc, + require_eq!( + lookup_table_account.addresses[i], + *acc, CcipRouterError::InvalidInputsLookupTableAccounts ); } diff --git a/chains/solana/contracts/programs/ccip-router/src/token_context.rs b/chains/solana/contracts/programs/ccip-router/src/token_context.rs index 80222139c..75888a041 100644 --- a/chains/solana/contracts/programs/ccip-router/src/token_context.rs +++ b/chains/solana/contracts/programs/ccip-router/src/token_context.rs @@ -18,32 +18,6 @@ pub struct TokenAdminRegistry { pub writable_indexes: [u128; 2], } -impl TokenAdminRegistry { - // set writable inserts bits from left to right - // index 0 is left-most bit - pub fn set_writable(&mut self, index: u8) { - match index < 128 { - true => { - self.writable_indexes[0] |= 1 << (127 - index); - } - false => { - self.writable_indexes[1] |= 1 << (255 - index); - } - } - } - - pub fn is_writable(&self, index: u8) -> bool { - match index < 128 { - true => self.writable_indexes[0] & 1 << (127 - index) != 0, - false => self.writable_indexes[1] & 1 << (255 - index) != 0, - } - } - - pub fn reset_writable(&mut self) { - self.writable_indexes = [0, 0]; - } -} - #[derive(Accounts)] #[instruction(mint: Pubkey)] pub struct RegisterTokenAdminRegistryViaGetCCIPAdmin<'info> { @@ -157,73 +131,3 @@ pub struct SetTokenBillingConfig<'info> { pub authority: Signer<'info>, pub system_program: Program<'info, System>, } - -#[cfg(test)] -mod tests { - use bytemuck::Zeroable; - use solana_program::pubkey::Pubkey; - - use crate::TokenAdminRegistry; - #[test] - fn set_writable() { - let mut state = TokenAdminRegistry { - version: 0, - administrator: Pubkey::zeroed(), - pending_administrator: Pubkey::zeroed(), - lookup_table: Pubkey::zeroed(), - writable_indexes: [0, 0], - }; - - state.set_writable(0); - state.set_writable(128); - assert_eq!(state.writable_indexes[0], 2u128.pow(127)); - assert_eq!(state.writable_indexes[1], 2u128.pow(127)); - - state.reset_writable(); - assert_eq!(state.writable_indexes[0], 0); - assert_eq!(state.writable_indexes[1], 0); - - state.set_writable(0); - state.set_writable(2); - state.set_writable(127); - state.set_writable(128); - state.set_writable(2 + 128); - state.set_writable(255); - assert_eq!( - state.writable_indexes[0], - 2u128.pow(127) + 2u128.pow(127 - 2) + 2u128.pow(0) - ); - assert_eq!( - state.writable_indexes[1], - 2u128.pow(127) + 2u128.pow(127 - 2) + 2u128.pow(0) - ); - } - - #[test] - fn check_writable() { - let state = TokenAdminRegistry { - version: 0, - administrator: Pubkey::zeroed(), - pending_administrator: Pubkey::zeroed(), - lookup_table: Pubkey::zeroed(), - writable_indexes: [ - 2u128.pow(127 - 7) + 2u128.pow(127 - 2) + 2u128.pow(127 - 4), - 2u128.pow(127 - 8) + 2u128.pow(127 - 56) + 2u128.pow(127 - 100), - ], - }; - - assert_eq!(state.is_writable(0), false); - assert_eq!(state.is_writable(128), false); - assert_eq!(state.is_writable(255), false); - - assert_eq!(state.writable_indexes[0].count_ones(), 3); - assert_eq!(state.writable_indexes[1].count_ones(), 3); - - assert_eq!(state.is_writable(7), true); - assert_eq!(state.is_writable(2), true); - assert_eq!(state.is_writable(4), true); - assert_eq!(state.is_writable(128 + 8), true); - assert_eq!(state.is_writable(128 + 56), true); - assert_eq!(state.is_writable(128 + 100), true); - } -}