From afbc057ddb7bb13c9b7765287b60d3e15441a5ff Mon Sep 17 00:00:00 2001 From: Jayant Krishnamurthy Date: Sun, 21 Aug 2022 15:20:54 -0700 Subject: [PATCH 1/3] first commit of del_product --- program/c/src/oracle/oracle.h | 6 +++ program/rust/src/processor.rs | 3 ++ program/rust/src/rust_oracle.rs | 60 ++++++++++++++++++++++ program/rust/src/tests/mod.rs | 1 + program/rust/src/tests/pyth_simulator.rs | 24 +++++++++ program/rust/src/tests/test_del_product.rs | 52 +++++++++++++++++++ program/rust/src/utils.rs | 9 ++++ 7 files changed, 155 insertions(+) create mode 100644 program/rust/src/tests/test_del_product.rs diff --git a/program/c/src/oracle/oracle.h b/program/c/src/oracle/oracle.h index 320d1e84e..4ffc0667b 100644 --- a/program/c/src/oracle/oracle.h +++ b/program/c/src/oracle/oracle.h @@ -275,6 +275,12 @@ typedef enum { // key[1] product account [signer writable] // key[2] price account [signer writable] e_cmd_del_price, + + // deletes a product account + // key[0] funding account [signer writable] + // key[1] mapping account [signer writable] + // key[2] product account [signer writable] + e_cmd_del_product, } command_t; typedef struct cmd_hdr diff --git a/program/rust/src/processor.rs b/program/rust/src/processor.rs index 595f1e915..ebc0c5ad4 100644 --- a/program/rust/src/processor.rs +++ b/program/rust/src/processor.rs @@ -6,6 +6,7 @@ use crate::c_oracle_header::{ command_t_e_cmd_add_publisher, command_t_e_cmd_agg_price, command_t_e_cmd_del_price, + command_t_e_cmd_del_product, command_t_e_cmd_del_publisher, command_t_e_cmd_init_mapping, command_t_e_cmd_init_price, @@ -29,6 +30,7 @@ use crate::rust_oracle::{ add_product, add_publisher, del_price, + del_product, del_publisher, init_mapping, init_price, @@ -75,6 +77,7 @@ pub fn process_instruction( command_t_e_cmd_upd_product => upd_product(program_id, accounts, instruction_data), command_t_e_cmd_set_min_pub => set_min_pub(program_id, accounts, instruction_data), command_t_e_cmd_del_price => del_price(program_id, accounts, instruction_data), + command_t_e_cmd_del_product => del_product(program_id, accounts, instruction_data), _ => Err(OracleError::UnrecognizedInstruction.into()), } } diff --git a/program/rust/src/rust_oracle.rs b/program/rust/src/rust_oracle.rs index 3bf306b1e..a8faf4cae 100644 --- a/program/rust/src/rust_oracle.rs +++ b/program/rust/src/rust_oracle.rs @@ -62,6 +62,7 @@ use crate::utils::{ check_valid_writable_account, is_component_update, pubkey_assign, + pubkey_clear, pubkey_equal, pubkey_is_zero, pyth_assert, @@ -705,3 +706,62 @@ pub fn set_min_pub( Ok(()) } + +/// The deleted product account must not have any price accounts. +/// Note that this function allows you to delete products from non-tail mapping accounts. +pub fn del_product( + program_id: &Pubkey, + accounts: &[AccountInfo], + instruction_data: &[u8], +) -> ProgramResult { + let [funding_account, mapping_account, product_account] = match accounts { + [w, x, y] => Ok([w, x, y]), + _ => Err(ProgramError::InvalidArgument), + }?; + + check_valid_funding_account(funding_account)?; + check_valid_signable_account(program_id, mapping_account, size_of::())?; + check_valid_signable_account(program_id, product_account, PC_PROD_ACC_SIZE as usize)?; + + { + let cmd_args = load::(instruction_data)?; + let mut mapping_data = load_checked::(mapping_account, cmd_args.ver_)?; + let product_data = load_checked::(product_account, cmd_args.ver_)?; + + // This assertion is just to make the subtractions below simpler + pyth_assert(mapping_data.num_ >= 1, ProgramError::InvalidArgument)?; + pyth_assert( + pubkey_is_zero(&product_data.px_acc_), + ProgramError::InvalidArgument, + )?; + + let product_key = product_account.key.to_bytes(); + let product_index = mapping_data + .prod_ + .iter() + .position(|x| pubkey_equal(x, &product_key)) + .ok_or(ProgramError::InvalidArgument)?; + + let last_index: usize = try_convert(mapping_data.num_ - 1)?; + + let last_key_bytes = mapping_data.prod_[last_index]; + pubkey_assign( + &mut mapping_data.prod_[product_index], + bytes_of(&last_key_bytes), + ); + pubkey_clear(&mut mapping_data.prod_[last_index]); + mapping_data.num_ -= 1; + mapping_data.size_ = + try_convert::<_, u32>(size_of::() - size_of_val(&mapping_data.prod_))? + + mapping_data.num_ * try_convert::<_, u32>(size_of::())?; + } + + // Zero out the balance of the price account to delete it. + // Note that you can't use the system program's transfer instruction to do this operation, as + // that instruction fails if the source account has any data. + let lamports = product_account.lamports(); + **product_account.lamports.borrow_mut() = 0; + **funding_account.lamports.borrow_mut() += lamports; + + Ok(()) +} diff --git a/program/rust/src/tests/mod.rs b/program/rust/src/tests/mod.rs index 164ebcf80..9746dde91 100644 --- a/program/rust/src/tests/mod.rs +++ b/program/rust/src/tests/mod.rs @@ -4,6 +4,7 @@ mod test_add_price; mod test_add_product; mod test_add_publisher; mod test_del_price; +mod test_del_product; mod test_del_publisher; mod test_init_mapping; mod test_init_price; diff --git a/program/rust/src/tests/pyth_simulator.rs b/program/rust/src/tests/pyth_simulator.rs index 194c9336f..d89809473 100644 --- a/program/rust/src/tests/pyth_simulator.rs +++ b/program/rust/src/tests/pyth_simulator.rs @@ -32,6 +32,7 @@ use crate::c_oracle_header::{ command_t_e_cmd_add_price, command_t_e_cmd_add_product, command_t_e_cmd_del_price, + command_t_e_cmd_del_product, command_t_e_cmd_init_mapping, pc_map_table_t, pc_price_t, @@ -161,6 +162,29 @@ impl PythSimulator { .map(|_| product_keypair) } + pub async fn del_product( + &mut self, + mapping_keypair: &Keypair, + product_keypair: &Keypair, + ) -> Result<(), BanksClientError> { + let cmd = cmd_hdr_t { + ver_: PC_VERSION, + cmd_: command_t_e_cmd_del_product as i32, + }; + let instruction = Instruction::new_with_bytes( + self.program_id, + bytes_of(&cmd), + vec![ + AccountMeta::new(self.payer.pubkey(), true), + AccountMeta::new(mapping_keypair.pubkey(), true), + AccountMeta::new(product_keypair.pubkey(), true), + ], + ); + + self.process_ix(instruction, &vec![&mapping_keypair, &product_keypair]) + .await + } + /// Initialize a price account and add it to an existing product account (using the add_price /// instruction). Returns the keypair associated with the newly-created account. pub async fn add_price( diff --git a/program/rust/src/tests/test_del_product.rs b/program/rust/src/tests/test_del_product.rs new file mode 100644 index 000000000..04a15736a --- /dev/null +++ b/program/rust/src/tests/test_del_product.rs @@ -0,0 +1,52 @@ +use solana_sdk::signer::Signer; + +use crate::c_oracle_header::pc_map_table_t; +use crate::tests::pyth_simulator::PythSimulator; +use crate::utils::{ + pubkey_equal, + pubkey_is_zero, +}; + +#[tokio::test] +async fn test_del_product() { + let mut sim = PythSimulator::new().await; + let mapping_keypair = sim.init_mapping().await.unwrap(); + let _product1 = sim.add_product(&mapping_keypair).await.unwrap(); + let product2 = sim.add_product(&mapping_keypair).await.unwrap(); + let product3 = sim.add_product(&mapping_keypair).await.unwrap(); + let product4 = sim.add_product(&mapping_keypair).await.unwrap(); + let product5 = sim.add_product(&mapping_keypair).await.unwrap(); + let _price3 = sim.add_price(&product3, -8).await.unwrap(); + + let mapping_keypair2 = sim.init_mapping().await.unwrap(); + let product1_2 = sim.add_product(&mapping_keypair2).await.unwrap(); + + assert!(sim.get_account(product2.pubkey()).await.is_some()); + assert!(sim.get_account(product4.pubkey()).await.is_some()); + + // Can't delete a product with a price account + assert!(sim.del_product(&mapping_keypair, &product3).await.is_err()); + // Can't delete mismatched product/mapping accounts + assert!(sim + .del_product(&mapping_keypair, &product1_2) + .await + .is_err()); + + assert!(sim.del_product(&mapping_keypair, &product2).await.is_ok()); + + let mapping_data = sim + .get_account_data_as::(mapping_keypair.pubkey()) + .await + .unwrap(); + assert_eq!(mapping_data.num_, 4); + + assert!(pubkey_equal( + &mapping_data.prod_[1], + &product5.pubkey().to_bytes() + )); + assert!(pubkey_equal( + &mapping_data.prod_[3], + &product4.pubkey().to_bytes() + )); + assert!(pubkey_is_zero(&mapping_data.prod_[4])); +} diff --git a/program/rust/src/utils.rs b/program/rust/src/utils.rs index b56effd88..209ab987a 100644 --- a/program/rust/src/utils.rs +++ b/program/rust/src/utils.rs @@ -106,6 +106,15 @@ pub fn pubkey_equal(target: &pc_pub_key_t, source: &[u8]) -> bool { unsafe { target.k1_ == *source } } +/// Zero out the bytes of `key`. +pub fn pubkey_clear(key: &mut pc_pub_key_t) { + unsafe { + for i in 0..key.k8_.len() { + key.k8_[i] = 0; + } + } +} + /// Convert `x: T` into a `U`, returning the appropriate `OracleError` if the conversion fails. pub fn try_convert>(x: T) -> Result { // Note: the error here assumes we're only applying this function to integers right now. From 96a8da5dc5eb0041083817cb93b33fe8af15fb16 Mon Sep 17 00:00:00 2001 From: Jayant Krishnamurthy Date: Mon, 22 Aug 2022 09:07:59 -0700 Subject: [PATCH 2/3] cleanup --- program/rust/src/rust_oracle.rs | 7 ++++++- program/rust/src/tests/pyth_simulator.rs | 1 + program/rust/src/tests/test_del_product.rs | 19 ++++++++++++++++++- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/program/rust/src/rust_oracle.rs b/program/rust/src/rust_oracle.rs index a8faf4cae..64f691509 100644 --- a/program/rust/src/rust_oracle.rs +++ b/program/rust/src/rust_oracle.rs @@ -707,8 +707,13 @@ pub fn set_min_pub( Ok(()) } +/// Delete a product account and remove it from the product list of its associated mapping account. /// The deleted product account must not have any price accounts. -/// Note that this function allows you to delete products from non-tail mapping accounts. +/// +/// This function allows you to delete products from non-tail mapping accounts. This ability is a +/// little weird, as it allows you to construct a list of multiple mapping accounts where non-tail +/// accounts have empty space. This is fine however; users should simply add new products to the +/// first available spot. pub fn del_product( program_id: &Pubkey, accounts: &[AccountInfo], diff --git a/program/rust/src/tests/pyth_simulator.rs b/program/rust/src/tests/pyth_simulator.rs index d89809473..08df0ebac 100644 --- a/program/rust/src/tests/pyth_simulator.rs +++ b/program/rust/src/tests/pyth_simulator.rs @@ -162,6 +162,7 @@ impl PythSimulator { .map(|_| product_keypair) } + /// Delete a product account (using the del_product instruction). pub async fn del_product( &mut self, mapping_keypair: &Keypair, diff --git a/program/rust/src/tests/test_del_product.rs b/program/rust/src/tests/test_del_product.rs index 04a15736a..9a5c6fc55 100644 --- a/program/rust/src/tests/test_del_product.rs +++ b/program/rust/src/tests/test_del_product.rs @@ -1,6 +1,14 @@ +use std::mem::{ + size_of, + size_of_val, +}; + use solana_sdk::signer::Signer; -use crate::c_oracle_header::pc_map_table_t; +use crate::c_oracle_header::{ + pc_map_table_t, + pc_pub_key_t, +}; use crate::tests::pyth_simulator::PythSimulator; use crate::utils::{ pubkey_equal, @@ -34,6 +42,8 @@ async fn test_del_product() { assert!(sim.del_product(&mapping_keypair, &product2).await.is_ok()); + assert!(sim.get_account(product2.pubkey()).await.is_none()); + let mapping_data = sim .get_account_data_as::(mapping_keypair.pubkey()) .await @@ -44,9 +54,16 @@ async fn test_del_product() { &mapping_data.prod_[1], &product5.pubkey().to_bytes() )); + assert!(sim.get_account(product5.pubkey()).await.is_some()); assert!(pubkey_equal( &mapping_data.prod_[3], &product4.pubkey().to_bytes() )); assert!(pubkey_is_zero(&mapping_data.prod_[4])); + + assert_eq!( + mapping_data.size_, + (size_of::() - size_of_val(&mapping_data.prod_)) as u32 + + 4 * size_of::() as u32 + ); } From 88347bd6a97767281e243e1809b2b276c7e7f953 Mon Sep 17 00:00:00 2001 From: Jayant Krishnamurthy Date: Mon, 22 Aug 2022 11:40:25 -0700 Subject: [PATCH 3/3] PR comments --- program/rust/src/rust_oracle.rs | 13 +++-- program/rust/src/tests/test_del_product.rs | 62 +++++++++++++++------- 2 files changed, 52 insertions(+), 23 deletions(-) diff --git a/program/rust/src/rust_oracle.rs b/program/rust/src/rust_oracle.rs index 64f691509..e7dbb7b15 100644 --- a/program/rust/src/rust_oracle.rs +++ b/program/rust/src/rust_oracle.rs @@ -747,15 +747,20 @@ pub fn del_product( .position(|x| pubkey_equal(x, &product_key)) .ok_or(ProgramError::InvalidArgument)?; - let last_index: usize = try_convert(mapping_data.num_ - 1)?; + let num_after_removal: usize = try_convert( + mapping_data + .num_ + .checked_sub(1) + .ok_or(ProgramError::InvalidArgument)?, + )?; - let last_key_bytes = mapping_data.prod_[last_index]; + let last_key_bytes = mapping_data.prod_[num_after_removal]; pubkey_assign( &mut mapping_data.prod_[product_index], bytes_of(&last_key_bytes), ); - pubkey_clear(&mut mapping_data.prod_[last_index]); - mapping_data.num_ -= 1; + pubkey_clear(&mut mapping_data.prod_[num_after_removal]); + mapping_data.num_ = try_convert::<_, u32>(num_after_removal)?; mapping_data.size_ = try_convert::<_, u32>(size_of::() - size_of_val(&mapping_data.prod_))? + mapping_data.num_ * try_convert::<_, u32>(size_of::())?; diff --git a/program/rust/src/tests/test_del_product.rs b/program/rust/src/tests/test_del_product.rs index 9a5c6fc55..07e1f2697 100644 --- a/program/rust/src/tests/test_del_product.rs +++ b/program/rust/src/tests/test_del_product.rs @@ -3,6 +3,7 @@ use std::mem::{ size_of_val, }; +use solana_program::pubkey::Pubkey; use solana_sdk::signer::Signer; use crate::c_oracle_header::{ @@ -10,16 +11,13 @@ use crate::c_oracle_header::{ pc_pub_key_t, }; use crate::tests::pyth_simulator::PythSimulator; -use crate::utils::{ - pubkey_equal, - pubkey_is_zero, -}; +use crate::utils::pubkey_equal; #[tokio::test] async fn test_del_product() { let mut sim = PythSimulator::new().await; let mapping_keypair = sim.init_mapping().await.unwrap(); - let _product1 = sim.add_product(&mapping_keypair).await.unwrap(); + let product1 = sim.add_product(&mapping_keypair).await.unwrap(); let product2 = sim.add_product(&mapping_keypair).await.unwrap(); let product3 = sim.add_product(&mapping_keypair).await.unwrap(); let product4 = sim.add_product(&mapping_keypair).await.unwrap(); @@ -48,22 +46,48 @@ async fn test_del_product() { .get_account_data_as::(mapping_keypair.pubkey()) .await .unwrap(); - assert_eq!(mapping_data.num_, 4); - - assert!(pubkey_equal( - &mapping_data.prod_[1], - &product5.pubkey().to_bytes() + assert!(mapping_product_list_equals( + &mapping_data, + vec![ + product1.pubkey(), + product5.pubkey(), + product3.pubkey(), + product4.pubkey() + ] )); assert!(sim.get_account(product5.pubkey()).await.is_some()); - assert!(pubkey_equal( - &mapping_data.prod_[3], - &product4.pubkey().to_bytes() + + + assert!(sim.del_product(&mapping_keypair, &product4).await.is_ok()); + let mapping_data = sim + .get_account_data_as::(mapping_keypair.pubkey()) + .await + .unwrap(); + + assert!(mapping_product_list_equals( + &mapping_data, + vec![product1.pubkey(), product5.pubkey(), product3.pubkey()] )); - assert!(pubkey_is_zero(&mapping_data.prod_[4])); +} + +/// Returns true if the list of products in `mapping_data` contains the keys in `expected` (in the +/// same order). Also checks `mapping_data.num_` and `size_`. +fn mapping_product_list_equals(mapping_data: &pc_map_table_t, expected: Vec) -> bool { + if mapping_data.num_ != expected.len() as u32 { + return false; + } + + let expected_size = (size_of::() - size_of_val(&mapping_data.prod_) + + expected.len() * size_of::()) as u32; + if mapping_data.size_ != expected_size { + return false; + } + + for i in 0..expected.len() { + if !pubkey_equal(&mapping_data.prod_[i], &expected[i].to_bytes()) { + return false; + } + } - assert_eq!( - mapping_data.size_, - (size_of::() - size_of_val(&mapping_data.prod_)) as u32 - + 4 * size_of::() as u32 - ); + return true; }