Rust instructions (#264)

* OracleCommand struct

* Cleanup entrypoint:

* Format

* Cleanup

* Additional comment

* Delete accidental files

* Explicit instruction order

* Fix rust oracle

Author: guibescos

program/rust/Cargo.toml

@@ -12,6 +12,8 @@ bindgen = "0.60.1"
 solana-program = "=1.10.29"
 bytemuck = "1.11.0"
 thiserror = "1.0"
+num-derive = "0.3"
+num-traits = "0.2"
 
 [dev-dependencies]
 solana-program-test = "=1.10.29"

program/rust/src/deserialize.rs

@@ -11,6 +11,7 @@ use crate::c_oracle_header::{
     PythAccount,
     PC_MAGIC,
 };
+use crate::error::OracleError;
 use crate::utils::{
     clear_account,
     pyth_assert,
@@ -23,22 +24,25 @@ use std::cell::{
 };
 
 /// Interpret the bytes in `data` as a value of type `T`
-pub fn load<T: Pod>(data: &[u8]) -> Result<&T, ProgramError> {
+/// This will fail if :
+/// - `data` is too short
+/// - `data` is not aligned for T
+pub fn load<T: Pod>(data: &[u8]) -> Result<&T, OracleError> {
     try_from_bytes(
         data.get(0..size_of::<T>())
-            .ok_or(ProgramError::InvalidArgument)?,
+            .ok_or(OracleError::InstructionDataTooShort)?,
     )
-    .map_err(|_| ProgramError::InvalidArgument)
+    .map_err(|_| OracleError::InstructionDataSliceMisaligned)
 }
 
 /// Interpret the bytes in `data` as a mutable value of type `T`
 #[allow(unused)]
-pub fn load_mut<T: Pod>(data: &mut [u8]) -> Result<&mut T, ProgramError> {
+pub fn load_mut<T: Pod>(data: &mut [u8]) -> Result<&mut T, OracleError> {
     try_from_bytes_mut(
         data.get_mut(0..size_of::<T>())
-            .ok_or(ProgramError::InvalidArgument)?,
+            .ok_or(OracleError::InstructionDataTooShort)?,
     )
-    .map_err(|_| ProgramError::InvalidArgument)
+    .map_err(|_| OracleError::InstructionDataSliceMisaligned)
 }
 
 /// Get the data stored in `account` as a value of type `T`

program/rust/src/error.rs

@@ -7,25 +7,31 @@ use thiserror::Error;
 pub enum OracleError {
     /// Generic catch all error
     #[error("Generic")]
-    Generic                 = 600,
+    Generic                        = 600,
     /// integer casting error
     #[error("IntegerCastingError")]
-    IntegerCastingError     = 601,
+    IntegerCastingError            = 601,
     /// c_entrypoint returned an unexpected value
     #[error("UnknownCError")]
-    UnknownCError           = 602,
+    UnknownCError                  = 602,
     #[error("UnrecognizedInstruction")]
-    UnrecognizedInstruction = 603,
+    UnrecognizedInstruction        = 603,
     #[error("InvalidFundingAccount")]
-    InvalidFundingAccount   = 604,
+    InvalidFundingAccount          = 604,
     #[error("InvalidSignableAccount")]
-    InvalidSignableAccount  = 605,
+    InvalidSignableAccount         = 605,
     #[error("InvalidSystemAccount")]
-    InvalidSystemAccount    = 606,
+    InvalidSystemAccount           = 606,
     #[error("InvalidWritableAccount")]
-    InvalidWritableAccount  = 607,
+    InvalidWritableAccount         = 607,
     #[error("InvalidFreshAccount")]
-    InvalidFreshAccount     = 608,
+    InvalidFreshAccount            = 608,
+    #[error("InvalidInstructionVersion")]
+    InvalidInstructionVersion      = 609,
+    #[error("InstructionDataTooShort")]
+    InstructionDataTooShort        = 610,
+    #[error("InstructionDataSliceMisaligned")]
+    InstructionDataSliceMisaligned = 611,
 }
 
 impl From<OracleError> for ProgramError {

program/rust/src/instruction.rs

@@ -0,0 +1,107 @@
+use crate::c_oracle_header::PC_VERSION;
+use crate::deserialize::load;
+use crate::error::OracleError;
+use bytemuck::{
+    Pod,
+    Zeroable,
+};
+use num_derive::{
+    FromPrimitive,
+    ToPrimitive,
+};
+use num_traits::FromPrimitive;
+
+/// WARNING : NEW COMMANDS SHOULD BE ADDED AT THE END OF THE LIST
+#[repr(i32)]
+#[derive(FromPrimitive, ToPrimitive)]
+pub enum OracleCommand {
+    // initialize first mapping list account
+    // account[0] funding account       [signer writable]
+    // account[1] mapping account       [signer writable]
+    InitMapping           = 0,
+    // initialize and add new mapping account
+    // account[0] funding account       [signer writable]
+    // account[1] tail mapping account  [signer writable]
+    // account[2] new mapping account   [signer writable]
+    AddMapping            = 1,
+    // initialize and add new product reference data account
+    // account[0] funding account       [signer writable]
+    // account[1] mapping account       [signer writable]
+    // account[2] new product account   [signer writable]
+    AddProduct            = 2,
+    // update product account
+    // account[0] funding account       [signer writable]
+    // account[1] product account       [signer writable]
+    UpdProduct            = 3,
+    // add new price account to a product account
+    // account[0] funding account       [signer writable]
+    // account[1] product account       [signer writable]
+    // account[2] new price account     [signer writable]
+    AddPrice              = 4,
+    // add publisher to symbol account
+    // account[0] funding account       [signer writable]
+    // account[1] price account         [signer writable]
+    AddPublisher          = 5,
+    // delete publisher from symbol account
+    // account[0] funding account       [signer writable]
+    // account[1] price account         [signer writable]
+    DelPublisher          = 6,
+    // publish component price
+    // account[0] funding account       [signer writable]
+    // account[1] price account         [writable]
+    // account[2] sysvar_clock account  []
+    UpdPrice              = 7,
+    // compute aggregate price
+    // account[0] funding account       [signer writable]
+    // account[1] price account         [writable]
+    // account[2] sysvar_clock account  []
+    AggPrice              = 8,
+    // (re)initialize price account
+    // account[0] funding account       [signer writable]
+    // account[1] new price account     [signer writable]
+    InitPrice             = 9,
+    // deprecated
+    InitTest              = 10,
+    // deprecated
+    UpdTest               = 11,
+    // set min publishers
+    // account[0] funding account       [signer writable]
+    // account[1] price account         [signer writable]
+    SetMinPub             = 12,
+    // publish component price, never returning an error even if the update failed
+    // account[0] funding account       [signer writable]
+    // account[1] price account         [writable]
+    // account[2] sysvar_clock account  []
+    UpdPriceNoFailOnError = 13,
+    // resizes a price account so that it fits the Time Machine
+    // account[0] funding account       [signer writable]
+    // account[1] price account         [signer writable]
+    // account[2] system program        []
+    ResizePriceAccount    = 14,
+    // deletes a price account
+    // account[0] funding account       [signer writable]
+    // account[1] product account       [signer writable]
+    // account[2] price account         [signer writable]
+    DelPrice              = 15,
+    // deletes a product account
+    // key[0] funding account       [signer writable]
+    // key[1] mapping account       [signer writable]
+    // key[2] product account       [signer writable]
+    DelProduct            = 16,
+}
+
+#[repr(C)]
+#[derive(Zeroable, Pod, Copy, Clone)]
+pub struct CommandHeader {
+    pub version: u32,
+    pub command: i32,
+}
+
+pub fn load_command_header_checked(data: &[u8]) -> Result<OracleCommand, OracleError> {
+    let command_header = load::<CommandHeader>(data)?;
+
+    if command_header.version != PC_VERSION {
+        return Err(OracleError::InvalidInstructionVersion);
+    }
+    OracleCommand::from_i32(command_header.command).ok_or(OracleError::UnrecognizedInstruction)
+}

program/rust/src/lib.rs

@@ -7,6 +7,7 @@
 mod c_oracle_header;
 mod deserialize;
 mod error;
+mod instruction;
 mod processor;
 mod rust_oracle;
 mod time_machine_types;

program/rust/src/processor.rs

@@ -1,26 +1,9 @@
-use crate::c_oracle_header::{
-    cmd_hdr,
-    command_t_e_cmd_add_mapping,
-    command_t_e_cmd_add_price,
-    command_t_e_cmd_add_product,
-    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,
-    command_t_e_cmd_resize_price_account,
-    command_t_e_cmd_set_min_pub,
-    command_t_e_cmd_upd_price,
-    command_t_e_cmd_upd_price_no_fail_on_error,
-    command_t_e_cmd_upd_product,
-    PC_VERSION,
-};
-use crate::deserialize::load;
 use crate::error::OracleError;
+use crate::instruction::{
+    load_command_header_checked,
+    OracleCommand,
+};
 use solana_program::entrypoint::ProgramResult;
-use solana_program::program_error::ProgramError;
 use solana_program::pubkey::Pubkey;
 use solana_program::sysvar::slot_history::AccountInfo;
 
@@ -47,37 +30,27 @@ pub fn process_instruction(
     accounts: &[AccountInfo],
     instruction_data: &[u8],
 ) -> ProgramResult {
-    let cmd_data = load::<cmd_hdr>(instruction_data)?;
-
-    if cmd_data.ver_ != PC_VERSION {
-        return Err(ProgramError::InvalidArgument);
-    }
-
-    match cmd_data
-        .cmd_
-        .try_into()
-        .map_err(|_| OracleError::IntegerCastingError)?
-    {
-        command_t_e_cmd_upd_price | command_t_e_cmd_agg_price => {
-            upd_price(program_id, accounts, instruction_data)
-        }
-        command_t_e_cmd_upd_price_no_fail_on_error => {
+    match load_command_header_checked(instruction_data)? {
+        OracleCommand::InitMapping => init_mapping(program_id, accounts, instruction_data),
+        OracleCommand::AddMapping => add_mapping(program_id, accounts, instruction_data),
+        OracleCommand::AddProduct => add_product(program_id, accounts, instruction_data),
+        OracleCommand::UpdProduct => upd_product(program_id, accounts, instruction_data),
+        OracleCommand::AddPrice => add_price(program_id, accounts, instruction_data),
+        OracleCommand::AddPublisher => add_publisher(program_id, accounts, instruction_data),
+        OracleCommand::DelPublisher => del_publisher(program_id, accounts, instruction_data),
+        OracleCommand::UpdPrice => upd_price(program_id, accounts, instruction_data),
+        OracleCommand::AggPrice => upd_price(program_id, accounts, instruction_data),
+        OracleCommand::InitPrice => init_price(program_id, accounts, instruction_data),
+        OracleCommand::InitTest => Err(OracleError::UnrecognizedInstruction.into()),
+        OracleCommand::UpdTest => Err(OracleError::UnrecognizedInstruction.into()),
+        OracleCommand::SetMinPub => set_min_pub(program_id, accounts, instruction_data),
+        OracleCommand::UpdPriceNoFailOnError => {
             upd_price_no_fail_on_error(program_id, accounts, instruction_data)
         }
-        command_t_e_cmd_resize_price_account => {
+        OracleCommand::ResizePriceAccount => {
             resize_price_account(program_id, accounts, instruction_data)
         }
-        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_init_price => init_price(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),
-        command_t_e_cmd_del_publisher => del_publisher(program_id, accounts, instruction_data),
-        command_t_e_cmd_add_product => add_product(program_id, accounts, instruction_data),
-        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()),
+        OracleCommand::DelPrice => del_price(program_id, accounts, instruction_data),
+        OracleCommand::DelProduct => del_product(program_id, accounts, instruction_data),
     }
 }

program/rust/src/rust_oracle.rs

@@ -47,12 +47,12 @@ use crate::c_oracle_header::{
     PC_VERSION,
 };
 use crate::deserialize::{
-    initialize_pyth_account_checked, /* TODO: This has a confusingly similar name to a Solana
-                                      * sdk function */
+    initialize_pyth_account_checked,
     load,
     load_account_as_mut,
     load_checked,
 };
+use crate::instruction::CommandHeader;
 use crate::time_machine_types::PriceAccountWrapper;
 use crate::utils::{
     check_exponent_range,
@@ -729,9 +729,9 @@ pub fn del_product(
     check_valid_signable_account(program_id, product_account, PC_PROD_ACC_SIZE as usize)?;
 
     {
-        let cmd_args = load::<cmd_hdr_t>(instruction_data)?;
-        let mut mapping_data = load_checked::<pc_map_table_t>(mapping_account, cmd_args.ver_)?;
-        let product_data = load_checked::<pc_prod_t>(product_account, cmd_args.ver_)?;
+        let cmd_args = load::<CommandHeader>(instruction_data)?;
+        let mut mapping_data = load_checked::<pc_map_table_t>(mapping_account, cmd_args.version)?;
+        let product_data = load_checked::<pc_prod_t>(product_account, cmd_args.version)?;
 
         // This assertion is just to make the subtractions below simpler
         pyth_assert(mapping_data.num_ >= 1, ProgramError::InvalidArgument)?;

program/rust/src/tests/test_utils.rs

@@ -1,4 +1,12 @@
-use crate::c_oracle_header::PythAccount;
+use crate::c_oracle_header::{
+    PythAccount,
+    PC_VERSION,
+};
+use crate::instruction::{
+    CommandHeader,
+    OracleCommand,
+};
+use num_traits::ToPrimitive;
 use solana_program::account_info::AccountInfo;
 use solana_program::clock::Epoch;
 use solana_program::native_token::LAMPORTS_PER_SOL;
@@ -13,7 +21,6 @@ use solana_program::{
     system_program,
     sysvar,
 };
-
 const UPPER_BOUND_OF_ALL_ACCOUNT_SIZES: usize = 20536;
 
 /// The goal of this struct is to easily instantiate fresh solana accounts
@@ -97,3 +104,12 @@ pub fn update_clock_slot(clock_account: &mut AccountInfo, slot: u64) {
     clock_data.slot = slot;
     clock_data.to_account_info(clock_account);
 }
+
+impl Into<CommandHeader> for OracleCommand {
+    fn into(self) -> CommandHeader {
+        return CommandHeader {
+            version: PC_VERSION,
+            command: self.to_i32().unwrap(), // This can never fail and is only used in tests
+        };
+    }
+}