refactor: address feedback

Riateche · Riateche · commit 2f3c39c7646d · 2024-08-09T12:44:21.000+01:00
diff --git a/program/rust/src/accounts.rs b/program/rust/src/accounts.rs
@@ -59,6 +59,7 @@ pub use {
         PriceEma,
         PriceInfo,
         PythOracleSerialize,
+        MAX_FEED_INDEX,
     },
     product::{
         update_product_metadata,
diff --git a/program/rust/src/accounts/permission.rs b/program/rust/src/accounts/permission.rs
@@ -47,9 +47,6 @@ pub struct PermissionAccount {
 }
 
 impl PermissionAccount {
-    pub const MIN_SIZE_WITH_LAST_FEED_INDEX: usize =
-        size_of::<PermissionAccount>() + size_of::<u32>();
-
     pub fn is_authorized(&self, key: &Pubkey, command: OracleCommand) -> bool {
         #[allow(clippy::match_like_matches_macro)]
         match (*key, command) {
@@ -66,7 +63,7 @@ impl PermissionAccount {
     ) -> Result<RefMut<'a, u32>, ProgramError> {
         let start = size_of::<PermissionAccount>();
         let end = start + size_of::<u32>();
-        assert_eq!(Self::MIN_SIZE_WITH_LAST_FEED_INDEX, end);
+        assert_eq!(Self::NEW_ACCOUNT_SPACE, end);
         if account.data_len() < end {
             return Err(ProgramError::AccountDataTooSmall);
         }
@@ -78,6 +75,6 @@ impl PermissionAccount {
 
 impl PythAccount for PermissionAccount {
     const ACCOUNT_TYPE: u32 = PC_ACCTYPE_PERMISSIONS;
-    const INITIAL_SIZE: u32 = Self::MIN_SIZE_WITH_LAST_FEED_INDEX as u32;
-    const NEW_ACCOUNT_SPACE: usize = Self::MIN_SIZE_WITH_LAST_FEED_INDEX;
+    const NEW_ACCOUNT_SPACE: usize = size_of::<PermissionAccount>() + size_of::<u32>();
+    const INITIAL_SIZE: u32 = Self::NEW_ACCOUNT_SPACE as u32;
 }
diff --git a/program/rust/src/accounts/price.rs b/program/rust/src/accounts/price.rs
@@ -70,7 +70,7 @@ mod price_pythnet {
         /// Various flags
         pub flags:              PriceAccountFlags,
         /// Globally unique price feed index used for publishing.
-        /// Limited to 28 bites.
+        /// Limited to 28 bites so that it can be packed together with trading status in a single u32.
         pub feed_index:         u32,
         /// Corresponding product account
         pub product_account:    Pubkey,
@@ -95,6 +95,10 @@ mod price_pythnet {
         pub price_cumulative:   PriceCumulative,
     }
 
+    // Feed index is limited to 28 bites so that it can be packed
+    // together with trading status in a single u32.
+    pub const MAX_FEED_INDEX: u32 = (1 << 28) - 1;
+
     bitflags! {
         #[repr(C)]
         #[derive(Copy, Clone, Pod, Zeroable)]
diff --git a/program/rust/src/processor.rs b/program/rust/src/processor.rs
@@ -3,6 +3,8 @@ use {
         accounts::{
             AccountHeader,
             PermissionAccount,
+            PythAccount,
+            MAX_FEED_INDEX,
         },
         deserialize::load_account_as_mut,
         error::OracleError,
@@ -108,8 +110,8 @@ pub fn process_instruction(
 }
 
 fn reserve_new_price_feed_index(permissions_account: &AccountInfo) -> Result<u32, ProgramError> {
-    if permissions_account.data_len() < PermissionAccount::MIN_SIZE_WITH_LAST_FEED_INDEX {
-        let new_size = PermissionAccount::MIN_SIZE_WITH_LAST_FEED_INDEX;
+    if permissions_account.data_len() < PermissionAccount::NEW_ACCOUNT_SPACE {
+        let new_size = PermissionAccount::NEW_ACCOUNT_SPACE;
         let rent = Rent::get()?;
         let new_minimum_balance = rent.minimum_balance(new_size);
         pyth_assert(
@@ -124,7 +126,7 @@ fn reserve_new_price_feed_index(permissions_account: &AccountInfo) -> Result<u32
     let mut last_feed_index = PermissionAccount::load_last_feed_index_mut(permissions_account)?;
     *last_feed_index += 1;
     pyth_assert(
-        *last_feed_index < (1 << 28),
+        *last_feed_index <= MAX_FEED_INDEX,
         OracleError::MaxLastFeedIndexReached.into(),
     )?;
     Ok(*last_feed_index)
diff --git a/program/rust/src/processor/add_price.rs b/program/rust/src/processor/add_price.rs
@@ -50,6 +50,7 @@ pub fn add_price(
         ProgramError::InvalidArgument,
     )?;
 
+
     let (funding_account, product_account, price_account, permissions_account) = match accounts {
         [x, y, z, p] => Ok((x, y, z, p)),
         _ => Err(OracleError::InvalidNumberOfAccounts),
@@ -75,7 +76,6 @@ pub fn add_price(
     let mut product_data =
         load_checked::<ProductAccount>(product_account, cmd_args.header.version)?;
 
-    // TODO: where is it created???
     let mut price_data = PriceAccount::initialize(price_account, cmd_args.header.version)?;
     price_data.exponent = cmd_args.exponent;
     price_data.price_type = cmd_args.price_type;
diff --git a/program/rust/src/processor/add_product.rs b/program/rust/src/processor/add_product.rs
@@ -65,6 +65,7 @@ pub fn add_product(
         hdr,
     )?;
 
+
     let mut mapping_data = load_checked::<MappingAccount>(tail_mapping_account, hdr.version)?;
     // The mapping account must have free space to add the product account
     pyth_assert(
diff --git a/program/rust/src/processor/del_product.rs b/program/rust/src/processor/del_product.rs
@@ -68,6 +68,7 @@ pub fn del_product(
         cmd_args,
     )?;
 
+
     {
         let mut mapping_data = load_checked::<MappingAccount>(mapping_account, cmd_args.version)?;
         let product_data = load_checked::<ProductAccount>(product_account, cmd_args.version)?;
diff --git a/program/rust/src/processor/init_price.rs b/program/rust/src/processor/init_price.rs
@@ -55,6 +55,7 @@ pub fn init_price(
         &cmd_args.header,
     )?;
 
+
     let mut price_data = load_checked::<PriceAccount>(price_account, cmd_args.header.version)?;
     pyth_assert(
         price_data.price_type == cmd_args.price_type,
diff --git a/program/rust/src/processor/set_min_pub.rs b/program/rust/src/processor/set_min_pub.rs
@@ -51,6 +51,7 @@ pub fn set_min_pub(
         &cmd.header,
     )?;
 
+
     let mut price_account_data = load_checked::<PriceAccount>(price_account, cmd.header.version)?;
     price_account_data.min_pub_ = cmd.minimum_publishers;
 
diff --git a/program/rust/src/processor/upd_permissions.rs b/program/rust/src/processor/upd_permissions.rs
@@ -61,6 +61,7 @@ pub fn upd_permissions(
         OracleError::InvalidSystemAccount.into(),
     )?;
 
+
     // Create PermissionAccount if it doesn't exist
     PermissionAccount::initialize_pda(
         permissions_account,
diff --git a/program/rust/src/processor/upd_product.rs b/program/rust/src/processor/upd_product.rs
@@ -47,6 +47,7 @@ pub fn upd_product(
         hdr,
     )?;
 
+
     {
         // Validate that product_account contains the appropriate account header
         let mut _product_data = load_checked::<ProductAccount>(product_account, hdr.version)?;
diff --git a/program/rust/src/tests/mod.rs b/program/rust/src/tests/mod.rs
@@ -28,5 +28,6 @@ mod test_upd_price_with_validator;
 mod test_upd_product;
 mod test_utils;
 
+
 mod test_twap;
 mod test_upd_price_v2;
diff --git a/program/rust/src/tests/test_add_product.rs b/program/rust/src/tests/test_add_product.rs
@@ -38,6 +38,7 @@ use {
     std::mem::size_of,
 };
 
+
 #[test]
 fn test_add_product() {
     let mut instruction_data = [0u8; PC_PROD_ACC_SIZE as usize];
@@ -200,6 +201,7 @@ fn test_add_product() {
     assert_eq!(mapping_data.number_of_products, PC_MAP_TABLE_SIZE);
 }
 
+
 // Create an add_product instruction that sets the product metadata to strings
 pub fn populate_instruction(instruction_data: &mut [u8], strings: &[&str]) -> usize {
     {
diff --git a/program/rust/src/tests/test_del_price.rs b/program/rust/src/tests/test_del_price.rs
@@ -36,6 +36,7 @@ async fn test_del_price() {
         .unwrap();
     assert!(product1_data.first_price_account == Pubkey::default());
 
+
     // price2_1 is the 2nd item in the linked list since price2_2 got added after t.
     assert!(sim.del_price(&product2, &price2_1).await.is_err());
     // Can delete the accounts in the opposite order though
diff --git a/program/rust/src/tests/test_del_product.rs b/program/rust/src/tests/test_del_product.rs
@@ -11,6 +11,7 @@ use {
     },
 };
 
+
 #[tokio::test]
 async fn test_del_product() {
     let mut sim = PythSimulator::new().await;
@@ -55,6 +56,7 @@ async fn test_del_product() {
     ));
     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::<MappingAccount>(mapping_keypair.pubkey())
diff --git a/program/rust/src/tests/test_ema.rs b/program/rust/src/tests/test_ema.rs
@@ -18,6 +18,7 @@ use {
     test_generator::test_resources,
 };
 
+
 #[test_resources("program/rust/test_data/ema/*.csv")]
 fn test_ema(input_path_raw: &str) {
     let (inputs, expected_outputs) = read_test_data(input_path_raw);
@@ -109,6 +110,7 @@ fn run_ema_test(inputs: &[InputRecord], expected_outputs: &[OutputRecord]) {
     }
 }
 
+
 // TODO: put these functions somewhere more accessible
 pub fn upd_aggregate(
     price_account: &mut PriceAccount,
@@ -128,6 +130,7 @@ pub fn upd_twap(price_account: &mut PriceAccount, nslots: i64) {
     unsafe { c_upd_twap((price_account as *mut PriceAccount) as *mut u8, nslots) }
 }
 
+
 #[derive(Serialize, Deserialize, Debug)]
 struct InputRecord {
     price:  i64,
diff --git a/program/rust/src/tests/test_full_publisher_set.rs b/program/rust/src/tests/test_full_publisher_set.rs
@@ -36,6 +36,7 @@ async fn test_full_publisher_set() -> Result<(), Box<dyn std::error::Error>> {
         .await;
     let price = price_accounts["LTC"];
 
+
     let n_pubs = pub_keypairs.len();
 
     // Divide publishers into two even parts (assuming the max PC_NUM_COMP size is even)
diff --git a/program/rust/src/tests/test_message.rs b/program/rust/src/tests/test_message.rs
@@ -44,6 +44,7 @@ fn test_twap_message_roundtrip(input: TwapMessage) -> bool {
     }
 }
 
+
 fn prop_publisher_caps_message_roundtrip(input: PublisherStakeCapsMessage) -> bool {
     let reconstructed = from_slice::<BigEndian, Message>(&input.clone().to_bytes()).unwrap();
 
diff --git a/program/rust/src/tests/test_upd_aggregate.rs b/program/rust/src/tests/test_upd_aggregate.rs
@@ -67,6 +67,7 @@ fn test_upd_aggregate() {
         corp_act_status_: 0,
     };
 
+
     let mut instruction_data = [0u8; size_of::<UpdPriceArgs>()];
     populate_instruction(&mut instruction_data, 42, 2, 1);
 
diff --git a/program/rust/src/tests/test_upd_permissions.rs b/program/rust/src/tests/test_upd_permissions.rs
@@ -1,6 +1,9 @@
 use {
     crate::{
-        accounts::PermissionAccount,
+        accounts::{
+            PermissionAccount,
+            PythAccount,
+        },
         deserialize::load,
         error::OracleError,
         instruction::{
@@ -66,7 +69,7 @@ async fn test_upd_permissions() {
 
     assert_eq!(
         permission_account.data.len(),
-        PermissionAccount::MIN_SIZE_WITH_LAST_FEED_INDEX
+        PermissionAccount::NEW_ACCOUNT_SPACE
     );
     assert_eq!(
         Rent::default().minimum_balance(permission_account.data.len()),
@@ -88,6 +91,7 @@ async fn test_upd_permissions() {
     master_authority = Pubkey::new_unique();
     security_authority = Pubkey::new_unique();
 
+
     // Should fail because payer is not the authority
     assert_eq!(
         sim.upd_permissions(
diff --git a/program/rust/src/tests/test_upd_price_no_fail_on_error.rs b/program/rust/src/tests/test_upd_price_no_fail_on_error.rs
@@ -52,6 +52,7 @@ fn test_upd_price_no_fail_on_error_no_fail_on_error() {
 
     update_clock_slot(&mut clock_account, 1);
 
+
     // Check that the normal upd_price fails
     populate_instruction(&mut instruction_data, 42, 9, 1, true);
 
@@ -68,6 +69,7 @@ fn test_upd_price_no_fail_on_error_no_fail_on_error() {
         Err(OracleError::PermissionViolation.into())
     );
 
+
     populate_instruction(&mut instruction_data, 42, 9, 1, false);
     // We haven't permissioned the publish account for the price account
     // yet, so any update should fail silently and have no effect. The
@@ -83,6 +85,7 @@ fn test_upd_price_no_fail_on_error_no_fail_on_error() {
     )
     .is_ok());
 
+
     {
         let mut price_data = load_checked::<PriceAccount>(&price_account, PC_VERSION).unwrap();
         assert_eq!(price_data.comp_[0].latest_.price_, 0);
@@ -166,6 +169,7 @@ fn test_upd_price_no_fail_on_error_no_fail_on_error() {
     }
 }
 
+
 // Create an upd_price_no_fail_on_error or upd_price instruction with the provided parameters
 fn populate_instruction(
     instruction_data: &mut [u8],
diff --git a/program/rust/src/tests/test_upd_price_v2.rs b/program/rust/src/tests/test_upd_price_v2.rs
@@ -511,6 +511,7 @@ fn test_upd_works_with_unordered_publisher_set() -> Result<(), Box<dyn std::erro
         &instruction_data,
     )?;
 
+
     let price_data = load_checked::<PriceAccount>(&price_account, PC_VERSION).unwrap();
 
     // The result will be the following only if all the
diff --git a/program/rust/src/tests/test_upd_price_with_validator.rs b/program/rust/src/tests/test_upd_price_with_validator.rs
@@ -86,6 +86,7 @@ fn test_upd_price_with_validator() {
     )
     .is_ok());
 
+
     {
         let price_data = load_checked::<PriceAccount>(&price_account, PC_VERSION).unwrap();
         assert_eq!(price_data.comp_[0].latest_.price_, 42);
@@ -336,6 +337,7 @@ fn test_upd_price_with_validator() {
         assert_eq!(price_data.comp_[0].latest_.status_, PC_STATUS_TRADING);
     }
 
+
     assert!(process_instruction(
         &program_id,
         &[
@@ -405,6 +407,7 @@ fn test_upd_price_with_validator() {
     .unwrap();
     update_clock_slot(&mut clock_account, 8);
 
+
     assert!(process_instruction(
         &program_id,
         &[
@@ -439,6 +442,7 @@ fn test_upd_price_with_validator() {
     .unwrap();
     update_clock_slot(&mut clock_account, 9);
 
+
     assert!(process_instruction(
         &program_id,
         &[
diff --git a/program/rust/src/validator.rs b/program/rust/src/validator.rs
@@ -45,6 +45,7 @@ fn check_price_account_header(price_account_info: &[u8]) -> Result<(), ProgramEr
             && account_header.account_type == PriceAccount::ACCOUNT_TYPE,
         OracleError::InvalidAccountHeader.into(),
     )?;
+
     Ok(())
 }
 

Original file line number	Diff line number	Diff line change
`@@ -68,6 +68,7 @@ pub fn del_product(`
`68`	`68`	`cmd_args,`
`69`	`69`	`)?;`
`70`	`70`
	`71`	`+`
`71`	`72`	`{`
`72`	`73`	`let mut mapping_data = load_checked::<MappingAccount>(mapping_account, cmd_args.version)?;`
`73`	`74`	`let product_data = load_checked::<ProductAccount>(product_account, cmd_args.version)?;`