From 5baf3c38a0ff913f3cd2b532f54bff4dd1386979 Mon Sep 17 00:00:00 2001 From: Jayant Krishnamurthy Date: Mon, 22 Aug 2022 13:02:41 -0700 Subject: [PATCH] Command to delete a product (#262) * first commit of del_product * cleanup * PR comments Co-authored-by: Jayant Krishnamurthy --- program/c/src/oracle/oracle.h | 6 ++ program/rust/src/processor.rs | 3 + program/rust/src/rust_oracle.rs | 70 ++++++++++++++++ program/rust/src/tests/mod.rs | 1 + program/rust/src/tests/pyth_simulator.rs | 25 ++++++ program/rust/src/tests/test_del_product.rs | 93 ++++++++++++++++++++++ program/rust/src/utils.rs | 9 +++ 7 files changed, 207 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..e7dbb7b15 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,72 @@ 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. +/// +/// 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], + 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 num_after_removal: usize = try_convert( + mapping_data + .num_ + .checked_sub(1) + .ok_or(ProgramError::InvalidArgument)?, + )?; + + 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_[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::())?; + } + + // 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..08df0ebac 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,30 @@ impl PythSimulator { .map(|_| product_keypair) } + /// Delete a product account (using the del_product instruction). + 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..07e1f2697 --- /dev/null +++ b/program/rust/src/tests/test_del_product.rs @@ -0,0 +1,93 @@ +use std::mem::{ + size_of, + size_of_val, +}; + +use solana_program::pubkey::Pubkey; +use solana_sdk::signer::Signer; + +use crate::c_oracle_header::{ + pc_map_table_t, + pc_pub_key_t, +}; +use crate::tests::pyth_simulator::PythSimulator; +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 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()); + + assert!(sim.get_account(product2.pubkey()).await.is_none()); + + 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(), + product4.pubkey() + ] + )); + assert!(sim.get_account(product5.pubkey()).await.is_some()); + + + 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()] + )); +} + +/// 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; + } + } + + return true; +} 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.