From 80f7ffd6b27e59285a1045f5990f2e789c50f9ba Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Fri, 5 Aug 2022 18:17:35 +0000 Subject: [PATCH 1/7] Base test, clippy stopped working --- program/rust/build_utils.rs | 3 +- program/rust/src/lib.rs | 8 +- program/rust/src/log.rs | 1 - program/rust/src/rust_oracle.rs | 5 +- program/rust/src/tests/mod.rs | 2 + program/rust/src/tests/test_add_mapping.rs | 97 +++++++++++++++++++ .../test_init_mapping.rs} | 0 7 files changed, 107 insertions(+), 9 deletions(-) create mode 100644 program/rust/src/tests/mod.rs create mode 100644 program/rust/src/tests/test_add_mapping.rs rename program/rust/src/{test_oracle.rs => tests/test_init_mapping.rs} (100%) diff --git a/program/rust/build_utils.rs b/program/rust/build_utils.rs index 0be568b66..41ddf676c 100644 --- a/program/rust/build_utils.rs +++ b/program/rust/build_utils.rs @@ -24,8 +24,7 @@ impl<'a> DeriveAdderParserCallback<'a> { } //this is required to implement the callback trait -impl UnwindSafe for DeriveAdderParserCallback<'_> { -} +impl UnwindSafe for DeriveAdderParserCallback<'_> {} impl ParseCallbacks for DeriveAdderParserCallback<'_> { fn add_derives(&self, _name: &str) -> Vec { diff --git a/program/rust/src/lib.rs b/program/rust/src/lib.rs index 139950f48..74757fe9c 100644 --- a/program/rust/src/lib.rs +++ b/program/rust/src/lib.rs @@ -4,16 +4,18 @@ // Allow using the solana_program::entrypoint::deserialize function #![allow(clippy::not_unsafe_ptr_arg_deref)] -mod c_oracle_header; +pub mod c_oracle_header; mod deserialize; mod error; mod log; mod processor; -mod rust_oracle; -mod test_oracle; +pub mod rust_oracle; mod time_machine_types; mod utils; +#[cfg(test)] +mod tests; + use crate::c_oracle_header::SUCCESSFULLY_UPDATED_AGGREGATE; use crate::error::{ OracleError, diff --git a/program/rust/src/log.rs b/program/rust/src/log.rs index bc7e3dac5..ff32043bf 100644 --- a/program/rust/src/log.rs +++ b/program/rust/src/log.rs @@ -22,7 +22,6 @@ pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResu .try_into() .map_err(|_| OracleError::IntegerCastingError)?; - match instruction_id { command_t_e_cmd_upd_price | command_t_e_cmd_agg_price => { let instruction: cmd_upd_price = cmd_upd_price::try_from_slice(instruction_data)?; diff --git a/program/rust/src/rust_oracle.rs b/program/rust/src/rust_oracle.rs index b47be8aee..9a77af299 100644 --- a/program/rust/src/rust_oracle.rs +++ b/program/rust/src/rust_oracle.rs @@ -57,7 +57,6 @@ pub fn update_version( // Ok(SUCCESS) } - /// initialize the first mapping account in a new linked-list of mapping accounts /// accounts[0] funding account [signer writable] /// accounts[1] new mapping account [signer writable] @@ -187,7 +186,7 @@ pub fn load_account_as_mut<'a, T: Pod>( /// Mutably borrow the data in `account` as a mapping account, validating that the account /// is properly formatted. Any mutations to the returned value will be reflected in the /// account data. Use this to read already-initialized accounts. -fn load_mapping_account_mut<'a>( +pub fn load_mapping_account_mut<'a>( account: &'a AccountInfo, expected_version: u32, ) -> Result, ProgramError> { @@ -206,7 +205,7 @@ fn load_mapping_account_mut<'a>( /// Initialize account as a new mapping account. This function will zero out any existing data in /// the account. -fn initialize_mapping_account(account: &AccountInfo, version: u32) -> Result<(), ProgramError> { +pub fn initialize_mapping_account(account: &AccountInfo, version: u32) -> Result<(), ProgramError> { clear_account(account)?; let mut mapping_account = load_account_as_mut::(account)?; diff --git a/program/rust/src/tests/mod.rs b/program/rust/src/tests/mod.rs new file mode 100644 index 000000000..bee79ef71 --- /dev/null +++ b/program/rust/src/tests/mod.rs @@ -0,0 +1,2 @@ +mod test_add_mapping; +mod test_init_mapping; diff --git a/program/rust/src/tests/test_add_mapping.rs b/program/rust/src/tests/test_add_mapping.rs new file mode 100644 index 000000000..f339fc10c --- /dev/null +++ b/program/rust/src/tests/test_add_mapping.rs @@ -0,0 +1,97 @@ +mod test { + use crate::c_oracle_header::{ + cmd_hdr_t, + command_t_e_cmd_add_mapping, + pc_map_table_t, + PC_MAP_TABLE_SIZE, + PC_VERSION, + }; + use crate::rust_oracle::{ + add_mapping, + initialize_mapping_account, + load_mapping_account_mut, + }; + use bytemuck::bytes_of; + use solana_program::account_info::AccountInfo; + use solana_program::clock::Epoch; + use solana_program::native_token::LAMPORTS_PER_SOL; + use solana_program::pubkey::Pubkey; + use solana_program::rent::Rent; + use solana_program::system_program; + use std::mem::size_of; + + #[test] + fn test_add_mapping() { + let hdr: cmd_hdr_t = cmd_hdr_t { + ver_: PC_VERSION, + cmd_: command_t_e_cmd_add_mapping as i32, + }; + let instruction_data = bytes_of::(&hdr); + + let program_id = Pubkey::new_unique(); + let funding_key = Pubkey::new_unique(); + let cur_mapping_key = Pubkey::new_unique(); + let next_mapping_key = Pubkey::new_unique(); + let system_program = system_program::id(); + + let mut funding_balance = LAMPORTS_PER_SOL.clone(); + let funding_account = AccountInfo::new( + &funding_key, + true, + true, + &mut funding_balance, + &mut [], + &system_program, + false, + Epoch::default(), + ); + + let mut cur_mapping_balance = + Rent::minimum_balance(&Rent::default(), size_of::()); + let mut cur_mapping_raw_data = [0u8; size_of::()]; + + let cur_mapping = AccountInfo::new( + &cur_mapping_key, + true, + true, + &mut cur_mapping_balance, + &mut cur_mapping_raw_data, + &program_id, + false, + Epoch::default(), + ); + + initialize_mapping_account(&cur_mapping, PC_VERSION).unwrap(); + + { + let mut cur_mapping_data = load_mapping_account_mut(&cur_mapping, PC_VERSION).unwrap(); + cur_mapping_data.num_ = PC_MAP_TABLE_SIZE; + } + + let mut next_mapping_balance = + Rent::minimum_balance(&Rent::default(), size_of::()); + let mut next_mapping_raw_data = [0u8; size_of::()]; + + let next_mapping = AccountInfo::new( + &next_mapping_key, + true, + true, + &mut next_mapping_balance, + &mut next_mapping_raw_data, + &program_id, + false, + Epoch::default(), + ); + + assert!(add_mapping( + &program_id, + &[ + funding_account.clone(), + cur_mapping.clone(), + next_mapping.clone() + ], + instruction_data + ) + .is_ok()); + } +} diff --git a/program/rust/src/test_oracle.rs b/program/rust/src/tests/test_init_mapping.rs similarity index 100% rename from program/rust/src/test_oracle.rs rename to program/rust/src/tests/test_init_mapping.rs From de245df0d42b1d06a5854022419d1ca6485f2c08 Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Mon, 8 Aug 2022 15:43:42 +0000 Subject: [PATCH 2/7] Add publisher --- program/rust/build_utils.rs | 3 +- program/rust/src/c_oracle_header.rs | 18 ++++++ program/rust/src/lib.rs | 4 +- program/rust/src/processor.rs | 3 + program/rust/src/rust_oracle.rs | 95 +++++++++++++++++++++++++++++ 5 files changed, 120 insertions(+), 3 deletions(-) diff --git a/program/rust/build_utils.rs b/program/rust/build_utils.rs index 41ddf676c..0be568b66 100644 --- a/program/rust/build_utils.rs +++ b/program/rust/build_utils.rs @@ -24,7 +24,8 @@ impl<'a> DeriveAdderParserCallback<'a> { } //this is required to implement the callback trait -impl UnwindSafe for DeriveAdderParserCallback<'_> {} +impl UnwindSafe for DeriveAdderParserCallback<'_> { +} impl ParseCallbacks for DeriveAdderParserCallback<'_> { fn add_derives(&self, _name: &str) -> Vec { diff --git a/program/rust/src/c_oracle_header.rs b/program/rust/src/c_oracle_header.rs index 13648bbd7..c6a498ed5 100644 --- a/program/rust/src/c_oracle_header.rs +++ b/program/rust/src/c_oracle_header.rs @@ -64,6 +64,15 @@ unsafe impl Zeroable for cmd_add_price_t { unsafe impl Pod for cmd_add_price_t { } +#[cfg(target_endian = "little")] +unsafe impl Zeroable for cmd_add_publisher_t { +} + +#[cfg(target_endian = "little")] +unsafe impl Pod for cmd_add_publisher_t { +} + + #[cfg(target_endian = "little")] unsafe impl Zeroable for pc_pub_key_t { } @@ -71,3 +80,12 @@ unsafe impl Zeroable for pc_pub_key_t { #[cfg(target_endian = "little")] unsafe impl Pod for pc_pub_key_t { } + + +#[cfg(target_endian = "little")] +unsafe impl Zeroable for pc_price_comp_t { +} + +#[cfg(target_endian = "little")] +unsafe impl Pod for pc_price_comp_t { +} diff --git a/program/rust/src/lib.rs b/program/rust/src/lib.rs index 74757fe9c..79e770ca0 100644 --- a/program/rust/src/lib.rs +++ b/program/rust/src/lib.rs @@ -4,12 +4,12 @@ // Allow using the solana_program::entrypoint::deserialize function #![allow(clippy::not_unsafe_ptr_arg_deref)] -pub mod c_oracle_header; +mod c_oracle_header; mod deserialize; mod error; mod log; mod processor; -pub mod rust_oracle; +mod rust_oracle; mod time_machine_types; mod utils; diff --git a/program/rust/src/processor.rs b/program/rust/src/processor.rs index 2d8c967dc..7e29492fc 100644 --- a/program/rust/src/processor.rs +++ b/program/rust/src/processor.rs @@ -10,6 +10,7 @@ use crate::c_oracle_header::{ cmd_hdr, command_t_e_cmd_add_mapping, command_t_e_cmd_add_price, + command_t_e_cmd_add_publisher, command_t_e_cmd_agg_price, command_t_e_cmd_init_mapping, command_t_e_cmd_upd_account_version, @@ -24,6 +25,7 @@ use crate::error::{ use crate::rust_oracle::{ add_mapping, add_price, + add_publisher, init_mapping, update_price, update_version, @@ -63,6 +65,7 @@ pub fn process_instruction( command_t_e_cmd_add_price => add_price(program_id, accounts, instruction_data), command_t_e_cmd_init_mapping => init_mapping(program_id, accounts, instruction_data), command_t_e_cmd_add_mapping => add_mapping(program_id, accounts, instruction_data), + command_t_e_cmd_add_publisher => add_publisher(program_id, accounts, instruction_data), _ => c_entrypoint_wrapper(input), } } diff --git a/program/rust/src/rust_oracle.rs b/program/rust/src/rust_oracle.rs index 820309ef9..178103696 100644 --- a/program/rust/src/rust_oracle.rs +++ b/program/rust/src/rust_oracle.rs @@ -10,6 +10,7 @@ use std::mem::{ use bytemuck::{ bytes_of, + bytes_of_mut, try_from_bytes, try_from_bytes_mut, Pod, @@ -23,15 +24,18 @@ use solana_program::rent::Rent; use crate::c_oracle_header::{ cmd_add_price_t, + cmd_add_publisher_t, cmd_hdr_t, pc_acc, pc_map_table_t, + pc_price_comp, pc_price_t, pc_prod_t, pc_pub_key_t, PC_ACCTYPE_MAPPING, PC_ACCTYPE_PRICE, PC_ACCTYPE_PRODUCT, + PC_COMP_SIZE, PC_MAGIC, PC_MAP_TABLE_SIZE, PC_MAX_NUM_DECIMALS, @@ -172,6 +176,68 @@ pub fn add_price( Ok(SUCCESS) } + +// impl pc_price_t { +// add_publisher(key : &pc_pub_key_t) -> Result<(), ProgramError>{ +// Ok(()) +// } +// } +/// add a publisher to a price account +/// accounts[0] funding account [signer writable] +/// accounts[1] price account to add the publisher to [signer writable] +pub fn add_publisher( + program_id: &Pubkey, + accounts: &[AccountInfo], + instruction_data: &[u8], +) -> OracleResult { + let cmd_args = load::(instruction_data)?; + + if instruction_data.len() != size_of::() || pubkey_is_zero(&cmd_args.pub_) + { + return Err(ProgramError::InvalidArgument); + } + + let [_funding_account, price_account] = match accounts { + [x, y] + if valid_funding_account(x) + && valid_signable_account(program_id, y, size_of::()) => + { + Ok([x, y]) + } + _ => Err(ProgramError::InvalidArgument), + }?; + + let mut price_data = load_price_account_mut(price_account, cmd_args.ver_)?; + + if price_data.num_ >= PC_COMP_SIZE { + return Err(ProgramError::InvalidArgument); + } + + for i in 0..(price_data.num_ as usize) { + if pubkey_equal(&cmd_args.pub_, &price_data.comp_[i].pub_) { + return Err(ProgramError::InvalidArgument); + } + } + + let next_index = price_data.num_ as usize; + sol_memset( + bytes_of_mut(&mut price_data.comp_[next_index]), + 0, + size_of::(), + ); + pubkey_assign( + &mut price_data.comp_[next_index].pub_, + bytes_of(&cmd_args.pub_), + ); + price_data.num_ += 1; + + + price_data.size_ = (size_of::() - size_of_val(&price_data.comp_) + + (price_data.num_ as usize) * size_of::()) as u32; + Ok(SUCCESS) +} + + fn valid_funding_account(account: &AccountInfo) -> bool { account.is_signer && account.is_writable } @@ -230,6 +296,7 @@ pub fn load_account_as<'a, T: Pod>(account: &'a AccountInfo) -> Result( account: &'a AccountInfo, ) -> Result, ProgramError> { @@ -293,7 +360,35 @@ fn load_product_account_mut<'a>( Ok(product_data) } +/// Mutably borrow the data in `account` as a price account, validating that the account +/// is properly formatted. Any mutations to the returned value will be reflected in the +/// account data. Use this to read already-initialized accounts. +fn load_price_account_mut<'a>( + account: &'a AccountInfo, + expected_version: u32, +) -> Result, ProgramError> { + let price_data = load_account_as_mut::(account)?; + + pyth_assert( + price_data.magic_ == PC_MAGIC + && price_data.ver_ == expected_version + && price_data.type_ == PC_ACCTYPE_PRICE, + ProgramError::InvalidArgument, + )?; + + Ok(price_data) +} + + // Assign pubkey bytes from source to target, fails if source is not 32 bytes fn pubkey_assign(target: &mut pc_pub_key_t, source: &[u8]) { unsafe { target.k1_.copy_from_slice(source) } } + +fn pubkey_is_zero(key: &pc_pub_key_t) -> bool { + return unsafe { key.k8_.iter().all(|x| *x == 0) }; +} + +fn pubkey_equal(key1: &pc_pub_key_t, key2: &pc_pub_key_t) -> bool { + return unsafe { key1.k1_.iter().zip(&key2.k1_).all(|(x, y)| *x == *y) }; +} From d3af247c82a524bb03150e0906488f25a7c93f35 Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Mon, 8 Aug 2022 20:31:19 +0000 Subject: [PATCH 3/7] Remove comment --- program/rust/src/rust_oracle.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/program/rust/src/rust_oracle.rs b/program/rust/src/rust_oracle.rs index dc999c779..3e2ac54e0 100644 --- a/program/rust/src/rust_oracle.rs +++ b/program/rust/src/rust_oracle.rs @@ -175,12 +175,6 @@ pub fn add_price( Ok(SUCCESS) } - -// impl pc_price_t { -// add_publisher(key : &pc_pub_key_t) -> Result<(), ProgramError>{ -// Ok(()) -// } -// } /// add a publisher to a price account /// accounts[0] funding account [signer writable] /// accounts[1] price account to add the publisher to [signer writable] From 83260a855ce789ec5c70fd2be231d25117c5ad2c Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Mon, 8 Aug 2022 20:32:18 +0000 Subject: [PATCH 4/7] Restore test --- program/rust/src/tests/test_init_mapping.rs | 340 ++++++++++---------- 1 file changed, 168 insertions(+), 172 deletions(-) diff --git a/program/rust/src/tests/test_init_mapping.rs b/program/rust/src/tests/test_init_mapping.rs index 0e313cc49..6e0e40be5 100644 --- a/program/rust/src/tests/test_init_mapping.rs +++ b/program/rust/src/tests/test_init_mapping.rs @@ -1,182 +1,178 @@ -#[cfg(test)] -mod test { - use crate::c_oracle_header::{ - cmd_hdr_t, - command_t_e_cmd_init_mapping, - pc_map_table_t, - PC_ACCTYPE_MAPPING, - PC_MAGIC, - PC_VERSION, +use crate::c_oracle_header::{ + cmd_hdr_t, + command_t_e_cmd_init_mapping, + pc_map_table_t, + PC_ACCTYPE_MAPPING, + PC_MAGIC, + PC_VERSION, +}; +use crate::deserialize::load_account_as; +use crate::rust_oracle::{ + clear_account, + init_mapping, +}; +use bytemuck::bytes_of; +use solana_program::account_info::AccountInfo; +use solana_program::clock::Epoch; +use solana_program::native_token::LAMPORTS_PER_SOL; +use solana_program::program_error::ProgramError; +use solana_program::pubkey::Pubkey; +use solana_program::rent::Rent; +use solana_program::system_program; +use std::cell::RefCell; +use std::mem::size_of; +use std::rc::Rc; + +#[test] +fn test_init_mapping() { + let hdr: cmd_hdr_t = cmd_hdr_t { + ver_: PC_VERSION, + cmd_: command_t_e_cmd_init_mapping as i32, }; - use crate::rust_oracle::{ - clear_account, - init_mapping, - load_account_as, - }; - use bytemuck::bytes_of; - use solana_program::account_info::AccountInfo; - use solana_program::clock::Epoch; - use solana_program::native_token::LAMPORTS_PER_SOL; - use solana_program::program_error::ProgramError; - use solana_program::pubkey::Pubkey; - use solana_program::rent::Rent; - use solana_program::system_program; - use std::cell::RefCell; - use std::mem::size_of; - use std::rc::Rc; - - #[test] - fn test_init_mapping() { - let hdr: cmd_hdr_t = cmd_hdr_t { - ver_: PC_VERSION, - cmd_: command_t_e_cmd_init_mapping as i32, - }; - let instruction_data = bytes_of::(&hdr); - - let program_id = Pubkey::new_unique(); - let program_id_2 = Pubkey::new_unique(); - let funding_key = Pubkey::new_unique(); - let mapping_key = Pubkey::new_unique(); - let system_program = system_program::id(); - - let mut funding_balance = LAMPORTS_PER_SOL.clone(); - let mut funding_account = AccountInfo::new( - &funding_key, - true, - true, - &mut funding_balance, - &mut [], - &system_program, - false, - Epoch::default(), - ); - - let mut mapping_balance = - Rent::minimum_balance(&Rent::default(), size_of::()); - let mut mapping_raw_data = [0u8; size_of::()]; - - let mut mapping_account = AccountInfo::new( - &mapping_key, - true, - true, - &mut mapping_balance, - &mut mapping_raw_data, + let instruction_data = bytes_of::(&hdr); + + let program_id = Pubkey::new_unique(); + let program_id_2 = Pubkey::new_unique(); + let funding_key = Pubkey::new_unique(); + let mapping_key = Pubkey::new_unique(); + let system_program = system_program::id(); + + let mut funding_balance = LAMPORTS_PER_SOL.clone(); + let mut funding_account = AccountInfo::new( + &funding_key, + true, + true, + &mut funding_balance, + &mut [], + &system_program, + false, + Epoch::default(), + ); + + let mut mapping_balance = Rent::minimum_balance(&Rent::default(), size_of::()); + let mut mapping_raw_data = [0u8; size_of::()]; + + let mut mapping_account = AccountInfo::new( + &mapping_key, + true, + true, + &mut mapping_balance, + &mut mapping_raw_data, + &program_id, + false, + Epoch::default(), + ); + + assert!(init_mapping( + &program_id, + &[funding_account.clone(), mapping_account.clone()], + instruction_data + ) + .is_ok()); + + { + let mapping_data = load_account_as::(&mapping_account).unwrap(); + + assert_eq!(mapping_data.ver_, PC_VERSION); + assert_eq!(mapping_data.magic_, PC_MAGIC); + assert_eq!(mapping_data.type_, PC_ACCTYPE_MAPPING); + assert_eq!(mapping_data.size_, 56); + } + + assert_eq!( + init_mapping( + &program_id, + &[funding_account.clone(), mapping_account.clone()], + instruction_data + ), + Err(ProgramError::InvalidArgument) + ); + + clear_account(&mapping_account).unwrap(); + + assert_eq!( + init_mapping(&program_id, &[funding_account.clone()], instruction_data), + Err(ProgramError::InvalidArgument) + ); + + funding_account.is_signer = false; + + assert_eq!( + init_mapping( &program_id, - false, - Epoch::default(), - ); + &[funding_account.clone(), mapping_account.clone()], + instruction_data + ), + Err(ProgramError::InvalidArgument) + ); - assert!(init_mapping( + funding_account.is_signer = true; + mapping_account.is_signer = false; + + assert_eq!( + init_mapping( &program_id, &[funding_account.clone(), mapping_account.clone()], instruction_data - ) - .is_ok()); - - { - let mapping_data = load_account_as::(&mapping_account).unwrap(); - - assert_eq!(mapping_data.ver_, PC_VERSION); - assert_eq!(mapping_data.magic_, PC_MAGIC); - assert_eq!(mapping_data.type_, PC_ACCTYPE_MAPPING); - assert_eq!(mapping_data.size_, 56); - } - - assert_eq!( - init_mapping( - &program_id, - &[funding_account.clone(), mapping_account.clone()], - instruction_data - ), - Err(ProgramError::InvalidArgument) - ); - - clear_account(&mapping_account).unwrap(); - - assert_eq!( - init_mapping(&program_id, &[funding_account.clone()], instruction_data), - Err(ProgramError::InvalidArgument) - ); - - funding_account.is_signer = false; - - assert_eq!( - init_mapping( - &program_id, - &[funding_account.clone(), mapping_account.clone()], - instruction_data - ), - Err(ProgramError::InvalidArgument) - ); - - funding_account.is_signer = true; - mapping_account.is_signer = false; - - assert_eq!( - init_mapping( - &program_id, - &[funding_account.clone(), mapping_account.clone()], - instruction_data - ), - Err(ProgramError::InvalidArgument) - ); - - mapping_account.is_signer = true; - funding_account.is_writable = false; - - assert_eq!( - init_mapping( - &program_id, - &[funding_account.clone(), mapping_account.clone()], - instruction_data - ), - Err(ProgramError::InvalidArgument) - ); - - funding_account.is_writable = true; - mapping_account.is_writable = false; - - assert_eq!( - init_mapping( - &program_id, - &[funding_account.clone(), mapping_account.clone()], - instruction_data - ), - Err(ProgramError::InvalidArgument) - ); - - mapping_account.is_writable = true; - mapping_account.owner = &program_id_2; - - assert_eq!( - init_mapping( - &program_id, - &[funding_account.clone(), mapping_account.clone()], - instruction_data - ), - Err(ProgramError::InvalidArgument) - ); - - mapping_account.owner = &program_id; - let prev_data = mapping_account.data; - mapping_account.data = Rc::new(RefCell::new(&mut [])); - - assert_eq!( - init_mapping( - &program_id, - &[funding_account.clone(), mapping_account.clone()], - instruction_data - ), - Err(ProgramError::InvalidArgument) - ); - - mapping_account.data = prev_data; - - assert!(init_mapping( + ), + Err(ProgramError::InvalidArgument) + ); + + mapping_account.is_signer = true; + funding_account.is_writable = false; + + assert_eq!( + init_mapping( &program_id, &[funding_account.clone(), mapping_account.clone()], instruction_data - ) - .is_ok()); - } + ), + Err(ProgramError::InvalidArgument) + ); + + funding_account.is_writable = true; + mapping_account.is_writable = false; + + assert_eq!( + init_mapping( + &program_id, + &[funding_account.clone(), mapping_account.clone()], + instruction_data + ), + Err(ProgramError::InvalidArgument) + ); + + mapping_account.is_writable = true; + mapping_account.owner = &program_id_2; + + assert_eq!( + init_mapping( + &program_id, + &[funding_account.clone(), mapping_account.clone()], + instruction_data + ), + Err(ProgramError::InvalidArgument) + ); + + mapping_account.owner = &program_id; + let prev_data = mapping_account.data; + mapping_account.data = Rc::new(RefCell::new(&mut [])); + + assert_eq!( + init_mapping( + &program_id, + &[funding_account.clone(), mapping_account.clone()], + instruction_data + ), + Err(ProgramError::InvalidArgument) + ); + + mapping_account.data = prev_data; + + assert!(init_mapping( + &program_id, + &[funding_account.clone(), mapping_account.clone()], + instruction_data + ) + .is_ok()); } From 95a520c0847eaf0f81f8efab1cacda5233486cb3 Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Mon, 8 Aug 2022 20:37:36 +0000 Subject: [PATCH 5/7] Copy syntax from add product --- program/rust/src/rust_oracle.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/program/rust/src/rust_oracle.rs b/program/rust/src/rust_oracle.rs index 3e2ac54e0..e58798cf6 100644 --- a/program/rust/src/rust_oracle.rs +++ b/program/rust/src/rust_oracle.rs @@ -212,21 +212,20 @@ pub fn add_publisher( } } - let next_index = price_data.num_ as usize; + let current_index: usize = try_convert(price_data.num_)?; sol_memset( - bytes_of_mut(&mut price_data.comp_[next_index]), + bytes_of_mut(&mut price_data.comp_[current_index]), 0, size_of::(), ); pubkey_assign( - &mut price_data.comp_[next_index].pub_, + &mut price_data.comp_[current_index].pub_, bytes_of(&cmd_args.pub_), ); price_data.num_ += 1; - - - price_data.size_ = (size_of::() - size_of_val(&price_data.comp_) - + (price_data.num_ as usize) * size_of::()) as u32; + price_data.size_ = + try_convert::<_, u32>(size_of::() - size_of_val(&price_data.comp_))? + + price_data.num_ * try_convert::<_, u32>(size_of::())?; Ok(SUCCESS) } pub fn add_product( @@ -259,11 +258,10 @@ pub fn add_product( initialize_product_account(new_product_account, hdr.ver_)?; let current_index: usize = try_convert(mapping_data.num_)?; - unsafe { - mapping_data.prod_[current_index] - .k1_ - .copy_from_slice(&new_product_account.key.to_bytes()) - } + pubkey_assign( + &mut mapping_data.prod_[current_index], + bytes_of(&new_product_account.key.to_bytes()), + ); mapping_data.num_ += 1; mapping_data.size_ = try_convert::<_, u32>(size_of::() - size_of_val(&mapping_data.prod_))? From e598931016baf04d03c3b587c551fa01abc16c18 Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Mon, 8 Aug 2022 20:41:13 +0000 Subject: [PATCH 6/7] Add checker functions --- program/rust/src/rust_oracle.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/program/rust/src/rust_oracle.rs b/program/rust/src/rust_oracle.rs index e58798cf6..a0c9ca65f 100644 --- a/program/rust/src/rust_oracle.rs +++ b/program/rust/src/rust_oracle.rs @@ -191,15 +191,13 @@ pub fn add_publisher( } let [_funding_account, price_account] = match accounts { - [x, y] - if valid_funding_account(x) - && valid_signable_account(program_id, y, size_of::()) => - { - Ok([x, y]) - } + [x, y] => Ok([x, y]), _ => Err(ProgramError::InvalidArgument), }?; + check_valid_funding_account(_funding_account)?; + check_valid_signable_account(program_id, price_account, size_of::())?; + let mut price_data = load_price_account_mut(price_account, cmd_args.ver_)?; if price_data.num_ >= PC_COMP_SIZE { From 63d84946b76842146730c84cf0ad3a8b3f9d956a Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Mon, 8 Aug 2022 20:51:52 +0000 Subject: [PATCH 7/7] Use pubkey functions everywhere --- program/rust/src/rust_oracle.rs | 12 ++++++------ program/rust/src/tests/test_add_mapping.rs | 18 ++++++++---------- program/rust/src/tests/test_add_product.rs | 7 +------ 3 files changed, 15 insertions(+), 22 deletions(-) diff --git a/program/rust/src/rust_oracle.rs b/program/rust/src/rust_oracle.rs index a0c9ca65f..98314083e 100644 --- a/program/rust/src/rust_oracle.rs +++ b/program/rust/src/rust_oracle.rs @@ -118,8 +118,7 @@ pub fn add_mapping( let hdr = load::(instruction_data)?; let mut cur_mapping = load_mapping_account_mut(cur_mapping, hdr.ver_)?; pyth_assert( - cur_mapping.num_ == PC_MAP_TABLE_SIZE - && unsafe { cur_mapping.next_.k8_.iter().all(|x| *x == 0) }, + cur_mapping.num_ == PC_MAP_TABLE_SIZE && pubkey_is_zero(&cur_mapping.next_), ProgramError::InvalidArgument, )?; @@ -205,7 +204,7 @@ pub fn add_publisher( } for i in 0..(price_data.num_ as usize) { - if pubkey_equal(&cmd_args.pub_, &price_data.comp_[i].pub_) { + if pubkey_equal(&cmd_args.pub_, bytes_of(&price_data.comp_[i].pub_)) { return Err(ProgramError::InvalidArgument); } } @@ -415,13 +414,14 @@ pub fn pubkey_assign(target: &mut pc_pub_key_t, source: &[u8]) { unsafe { target.k1_.copy_from_slice(source) } } -fn pubkey_is_zero(key: &pc_pub_key_t) -> bool { +pub fn pubkey_is_zero(key: &pc_pub_key_t) -> bool { return unsafe { key.k8_.iter().all(|x| *x == 0) }; } -fn pubkey_equal(key1: &pc_pub_key_t, key2: &pc_pub_key_t) -> bool { - return unsafe { key1.k1_.iter().zip(&key2.k1_).all(|(x, y)| *x == *y) }; +pub fn pubkey_equal(target: &pc_pub_key_t, source: &[u8]) -> bool { + unsafe { target.k1_ == *source } } + /// Convert `x: T` into a `U`, returning the appropriate `OracleError` if the conversion fails. fn try_convert>(x: T) -> Result { // Note: the error here assumes we're only applying this function to integers right now. diff --git a/program/rust/src/tests/test_add_mapping.rs b/program/rust/src/tests/test_add_mapping.rs index 7422f5b50..5397bce55 100644 --- a/program/rust/src/tests/test_add_mapping.rs +++ b/program/rust/src/tests/test_add_mapping.rs @@ -13,6 +13,8 @@ use crate::rust_oracle::{ initialize_mapping_account, load_mapping_account_mut, pubkey_assign, + pubkey_equal, + pubkey_is_zero, }; use bytemuck::bytes_of; use solana_program::account_info::AccountInfo; @@ -102,15 +104,11 @@ fn test_add_mapping() { let next_mapping_data = load_mapping_account_mut(&next_mapping, PC_VERSION).unwrap(); let mut cur_mapping_data = load_mapping_account_mut(&cur_mapping, PC_VERSION).unwrap(); - assert!(unsafe { - cur_mapping_data - .next_ - .k1_ - .iter() - .zip(&next_mapping_key.to_bytes()) - .all(|(x, y)| *x == *y) - }); - assert!(unsafe { next_mapping_data.next_.k8_.iter().all(|x| *x == 0) }); + assert!(pubkey_equal( + &cur_mapping_data.next_, + &next_mapping_key.to_bytes() + )); + assert!(pubkey_is_zero(&next_mapping_data.next_)); pubkey_assign(&mut cur_mapping_data.next_, &Pubkey::default().to_bytes()); cur_mapping_data.num_ = 0; } @@ -132,7 +130,7 @@ fn test_add_mapping() { { let mut cur_mapping_data = load_mapping_account_mut(&cur_mapping, PC_VERSION).unwrap(); - assert!(unsafe { cur_mapping_data.next_.k8_.iter().all(|x| *x == 0) }); + assert!(pubkey_is_zero(&cur_mapping_data.next_)); cur_mapping_data.num_ = PC_MAP_TABLE_SIZE; cur_mapping_data.magic_ = 0; } diff --git a/program/rust/src/tests/test_add_product.rs b/program/rust/src/tests/test_add_product.rs index 7645cbfac..b1722d258 100644 --- a/program/rust/src/tests/test_add_product.rs +++ b/program/rust/src/tests/test_add_product.rs @@ -16,7 +16,6 @@ use crate::c_oracle_header::{ command_t_e_cmd_add_product, pc_map_table_t, pc_prod_t, - pc_pub_key_t, PC_ACCTYPE_MAPPING, PC_ACCTYPE_PRODUCT, PC_MAGIC, @@ -30,6 +29,7 @@ use crate::rust_oracle::{ clear_account, initialize_mapping_account, load_mapping_account_mut, + pubkey_equal, }; #[test] @@ -210,8 +210,3 @@ fn test_add_product() { let mapping_data = load_mapping_account_mut(&mapping_account, PC_VERSION).unwrap(); assert_eq!(mapping_data.num_, PC_MAP_TABLE_SIZE); } - -// Assign pubkey bytes from source to target, fails if source is not 32 bytes -fn pubkey_equal(target: &pc_pub_key_t, source: &[u8]) -> bool { - unsafe { target.k1_ == *source } -}