Pubkey (#277)

* Pubkey

* Use fault for zero pubkey

* Restore tests

* Add sanity check test for Pubkey::default

* Get current onchain bytes can get interpreted as Pubkey properly

* Update test

Author: guibescos

program/rust/Cargo.toml

@@ -19,6 +19,7 @@ num-traits = "0.2"
 solana-program-test = "=1.10.29"
 solana-sdk = "=1.10.29"
 tokio = "1.14.1"
+hex = "0.3.1"
 
 [features]
 debug = []

program/rust/src/c_oracle_header.rs

@@ -82,9 +82,9 @@ pub struct PriceAccount {
     pub unused_2_:          i16,
     pub unused_3_:          i32,
     /// Corresponding product account
-    pub product_account:    CPubkey,
+    pub product_account:    Pubkey,
     /// Next price account in the list
-    pub next_price_account: CPubkey,
+    pub next_price_account: Pubkey,
     /// Second to last slot where aggregation was succesful (i.e. status : TRADING)
     pub prev_slot_:         u64,
     /// Aggregate price at prev_slot_
@@ -102,7 +102,7 @@ pub struct PriceAccount {
 #[repr(C)]
 #[derive(Copy, Clone, Pod, Zeroable)]
 pub struct PriceComponent {
-    pub pub_:    CPubkey,
+    pub pub_:    Pubkey,
     pub agg_:    PriceInfo,
     pub latest_: PriceInfo,
 }
@@ -138,7 +138,7 @@ pub struct AccountHeader {
 #[derive(Copy, Clone, Pod, Zeroable)]
 pub struct ProductAccount {
     pub header:              AccountHeader,
-    pub first_price_account: CPubkey,
+    pub first_price_account: Pubkey,
 }
 
 #[repr(C)]
@@ -147,35 +147,12 @@ pub struct MappingAccount {
     pub header:               AccountHeader,
     pub number_of_products:   u32,
     pub unused_:              u32,
-    pub next_mapping_account: CPubkey,
-    pub products_list:        [CPubkey; PC_MAP_TABLE_SIZE as usize],
+    pub next_mapping_account: Pubkey,
+    pub products_list:        [Pubkey; PC_MAP_TABLE_SIZE as usize],
 }
 
-// Unsafe impl because CPubkey is a union
-unsafe impl Pod for CPubkey {
-}
-unsafe impl Zeroable for CPubkey {
-}
-
-
 // Unsafe impl because product_list is of size 640 and there's no derived trait for this size
 unsafe impl Pod for MappingAccount {
 }
 unsafe impl Zeroable for MappingAccount {
 }
-
-#[repr(C)]
-#[derive(Copy, Clone)]
-pub union CPubkey {
-    pub k1_: [u8; 32usize],
-    pub k8_: [u64; 4usize],
-}
-
-impl CPubkey {
-    pub fn new_unique() -> CPubkey {
-        let solana_unique = Pubkey::new_unique();
-        CPubkey {
-            k1_: solana_unique.to_bytes(),
-        }
-    }
-}

program/rust/src/instruction.rs

@@ -1,7 +1,4 @@
-use crate::c_oracle_header::{
-    CPubkey,
-    PC_VERSION,
-};
+use crate::c_oracle_header::PC_VERSION;
 use crate::deserialize::load;
 use crate::error::OracleError;
 use bytemuck::{
@@ -13,6 +10,7 @@ use num_derive::{
     ToPrimitive,
 };
 use num_traits::FromPrimitive;
+use solana_program::pubkey::Pubkey;
 
 /// WARNING : NEW COMMANDS SHOULD BE ADDED AT THE END OF THE LIST
 #[repr(i32)]
@@ -122,7 +120,7 @@ pub type InitPriceArgs = AddPriceArgs;
 #[derive(Zeroable, Pod, Copy, Clone)]
 pub struct AddPublisherArgs {
     pub header:    CommandHeader,
-    pub publisher: CPubkey,
+    pub publisher: Pubkey,
 }
 
 pub type DelPublisherArgs = AddPublisherArgs;

program/rust/src/rust_oracle.rs

@@ -4,7 +4,6 @@ use std::mem::{
 };
 
 use crate::c_oracle_header::{
-    CPubkey,
     MappingAccount,
     PriceAccount,
     PriceComponent,
@@ -40,19 +39,12 @@ use crate::utils::{
     check_valid_signable_account,
     check_valid_writable_account,
     is_component_update,
-    pubkey_assign,
-    pubkey_clear,
-    pubkey_equal,
-    pubkey_is_zero,
     pyth_assert,
     read_pc_str_t,
     try_convert,
 };
 use crate::OracleError;
-use bytemuck::{
-    bytes_of,
-    bytes_of_mut,
-};
+use bytemuck::bytes_of_mut;
 use solana_program::account_info::AccountInfo;
 use solana_program::clock::Clock;
 use solana_program::entrypoint::ProgramResult;
@@ -197,15 +189,12 @@ pub fn add_mapping(
     let mut cur_mapping = load_checked::<MappingAccount>(cur_mapping, hdr.version)?;
     pyth_assert(
         cur_mapping.number_of_products == PC_MAP_TABLE_SIZE
-            && pubkey_is_zero(&cur_mapping.next_mapping_account),
+            && cur_mapping.next_mapping_account == Pubkey::default(),
         ProgramError::InvalidArgument,
     )?;
 
     initialize_pyth_account_checked::<MappingAccount>(next_mapping, hdr.version)?;
-    pubkey_assign(
-        &mut cur_mapping.next_mapping_account,
-        &next_mapping.key.to_bytes(),
-    );
+    cur_mapping.next_mapping_account = *next_mapping.key;
 
     Ok(())
 }
@@ -240,10 +229,7 @@ pub fn upd_price(
 
         // Verify that publisher is authorized
         while publisher_index < price_data.num_ as usize {
-            if pubkey_equal(
-                &price_data.comp_[publisher_index].pub_,
-                &funding_account.key.to_bytes(),
-            ) {
+            if price_data.comp_[publisher_index].pub_ == *funding_account.key {
                 break;
             }
             publisher_index += 1;
@@ -357,18 +343,9 @@ pub fn add_price(
         initialize_pyth_account_checked::<PriceAccount>(price_account, cmd_args.header.version)?;
     price_data.exponent = cmd_args.exponent;
     price_data.price_type = cmd_args.price_type;
-    pubkey_assign(
-        &mut price_data.product_account,
-        &product_account.key.to_bytes(),
-    );
-    pubkey_assign(
-        &mut price_data.next_price_account,
-        bytes_of(&product_data.first_price_account),
-    );
-    pubkey_assign(
-        &mut product_data.first_price_account,
-        &price_account.key.to_bytes(),
-    );
+    price_data.product_account = *product_account.key;
+    price_data.next_price_account = product_data.first_price_account;
+    product_data.first_price_account = *price_account.key;
 
     Ok(())
 }
@@ -399,22 +376,16 @@ pub fn del_price(
         let mut product_data = load_checked::<ProductAccount>(product_account, cmd_args.version)?;
         let price_data = load_checked::<PriceAccount>(price_account, cmd_args.version)?;
         pyth_assert(
-            pubkey_equal(
-                &product_data.first_price_account,
-                &price_account.key.to_bytes(),
-            ),
+            product_data.first_price_account == *price_account.key,
             ProgramError::InvalidArgument,
         )?;
 
         pyth_assert(
-            pubkey_equal(&price_data.product_account, &product_account.key.to_bytes()),
+            price_data.product_account == *product_account.key,
             ProgramError::InvalidArgument,
         )?;
 
-        pubkey_assign(
-            &mut product_data.first_price_account,
-            bytes_of(&price_data.next_price_account),
-        );
+        product_data.first_price_account = price_data.next_price_account;
     }
 
     // Zero out the balance of the price account to delete it.
@@ -502,7 +473,7 @@ pub fn add_publisher(
 
     pyth_assert(
         instruction_data.len() == size_of::<AddPublisherArgs>()
-            && !pubkey_is_zero(&cmd_args.publisher),
+            && cmd_args.publisher != Pubkey::default(),
         ProgramError::InvalidArgument,
     )?;
 
@@ -521,7 +492,7 @@ pub fn add_publisher(
     }
 
     for i in 0..(price_data.num_ as usize) {
-        if pubkey_equal(&cmd_args.publisher, bytes_of(&price_data.comp_[i].pub_)) {
+        if cmd_args.publisher == price_data.comp_[i].pub_ {
             return Err(ProgramError::InvalidArgument);
         }
     }
@@ -532,10 +503,7 @@ pub fn add_publisher(
         0,
         size_of::<PriceComponent>(),
     );
-    pubkey_assign(
-        &mut price_data.comp_[current_index].pub_,
-        bytes_of(&cmd_args.publisher),
-    );
+    price_data.comp_[current_index].pub_ = cmd_args.publisher;
     price_data.num_ += 1;
     price_data.header.size =
         try_convert::<_, u32>(size_of::<PriceAccount>() - size_of_val(&price_data.comp_))?
@@ -555,7 +523,7 @@ pub fn del_publisher(
 
     pyth_assert(
         instruction_data.len() == size_of::<DelPublisherArgs>()
-            && !pubkey_is_zero(&cmd_args.publisher),
+            && cmd_args.publisher != Pubkey::default(),
         ProgramError::InvalidArgument,
     )?;
 
@@ -570,7 +538,7 @@ pub fn del_publisher(
     let mut price_data = load_checked::<PriceAccount>(price_account, cmd_args.header.version)?;
 
     for i in 0..(price_data.num_ as usize) {
-        if pubkey_equal(&cmd_args.publisher, bytes_of(&price_data.comp_[i].pub_)) {
+        if cmd_args.publisher == price_data.comp_[i].pub_ {
             for j in i + 1..(price_data.num_ as usize) {
                 price_data.comp_[j - 1] = price_data.comp_[j];
             }
@@ -615,15 +583,12 @@ pub fn add_product(
     initialize_pyth_account_checked::<ProductAccount>(new_product_account, hdr.version)?;
 
     let current_index: usize = try_convert(mapping_data.number_of_products)?;
-    pubkey_assign(
-        &mut mapping_data.products_list[current_index],
-        bytes_of(&new_product_account.key.to_bytes()),
-    );
+    mapping_data.products_list[current_index] = *new_product_account.key;
     mapping_data.number_of_products += 1;
     mapping_data.header.size = try_convert::<_, u32>(
         size_of::<MappingAccount>() - size_of_val(&mapping_data.products_list),
     )? + mapping_data.number_of_products
-        * try_convert::<_, u32>(size_of::<CPubkey>())?;
+        * try_convert::<_, u32>(size_of::<Pubkey>())?;
 
     Ok(())
 }
@@ -746,15 +711,15 @@ pub fn del_product(
             ProgramError::InvalidArgument,
         )?;
         pyth_assert(
-            pubkey_is_zero(&product_data.first_price_account),
+            product_data.first_price_account == Pubkey::default(),
             ProgramError::InvalidArgument,
         )?;
 
-        let product_key = product_account.key.to_bytes();
+        let product_key = product_account.key;
         let product_index = mapping_data
             .products_list
             .iter()
-            .position(|x| pubkey_equal(x, &product_key))
+            .position(|x| *x == *product_key)
             .ok_or(ProgramError::InvalidArgument)?;
 
         let num_after_removal: usize = try_convert(
@@ -765,16 +730,13 @@ pub fn del_product(
         )?;
 
         let last_key_bytes = mapping_data.products_list[num_after_removal];
-        pubkey_assign(
-            &mut mapping_data.products_list[product_index],
-            bytes_of(&last_key_bytes),
-        );
-        pubkey_clear(&mut mapping_data.products_list[num_after_removal]);
+        mapping_data.products_list[product_index] = last_key_bytes;
+        mapping_data.products_list[num_after_removal] = Pubkey::default();
         mapping_data.number_of_products = try_convert::<_, u32>(num_after_removal)?;
         mapping_data.header.size = try_convert::<_, u32>(
             size_of::<MappingAccount>() - size_of_val(&mapping_data.products_list),
         )? + mapping_data.number_of_products
-            * try_convert::<_, u32>(size_of::<CPubkey>())?;
+            * try_convert::<_, u32>(size_of::<Pubkey>())?;
     }
 
     // Zero out the balance of the price account to delete it.

program/rust/src/tests/test_add_mapping.rs

@@ -15,12 +15,7 @@ use crate::instruction::{
 };
 use crate::processor::process_instruction;
 use crate::tests::test_utils::AccountSetup;
-use crate::utils::{
-    clear_account,
-    pubkey_assign,
-    pubkey_equal,
-    pubkey_is_zero,
-};
+use crate::utils::clear_account;
 use bytemuck::bytes_of;
 use solana_program::program_error::ProgramError;
 use solana_program::pubkey::Pubkey;
@@ -64,15 +59,9 @@ fn test_add_mapping() {
         let mut cur_mapping_data =
             load_checked::<MappingAccount>(&cur_mapping, PC_VERSION).unwrap();
 
-        assert!(pubkey_equal(
-            &cur_mapping_data.next_mapping_account,
-            &next_mapping.key.to_bytes()
-        ));
-        assert!(pubkey_is_zero(&next_mapping_data.next_mapping_account));
-        pubkey_assign(
-            &mut cur_mapping_data.next_mapping_account,
-            &Pubkey::default().to_bytes(),
-        );
+        assert!(cur_mapping_data.next_mapping_account == *next_mapping.key);
+        assert!(next_mapping_data.next_mapping_account == Pubkey::default());
+        cur_mapping_data.next_mapping_account = Pubkey::default();
         cur_mapping_data.number_of_products = 0;
     }
 
@@ -94,7 +83,7 @@ fn test_add_mapping() {
     {
         let mut cur_mapping_data =
             load_checked::<MappingAccount>(&cur_mapping, PC_VERSION).unwrap();
-        assert!(pubkey_is_zero(&cur_mapping_data.next_mapping_account));
+        assert!(cur_mapping_data.next_mapping_account == Pubkey::default());
         cur_mapping_data.number_of_products = PC_MAP_TABLE_SIZE;
         cur_mapping_data.header.magic_number = 0;
     }