Intercepting price updates (#204)

* changed c return value of update price to indicate aggregation (#190)

* changed c return value of update price to indicate aggregation

* forgot to add bindings

* removed the need for casting

* fixed update_no_fail

* removed unnecessary docker packages

* timeless coments

* a convinient way to derive traits

* changed importing style

* added register_traits method

* fixed formatting

* add a new insctruction and interecepted update price calls

* fixed formatting

* more helpful error message for incorrect c account size

* fixed merging errors

* addressed some warning

* updated comment

* merging formatting issues

* memory borrowing issues after rebasing

* updated import

* more import style changes

* removed panics

* bug introduced by merge

* moved dispatching to processor

* better formatting

* cleaner error handling

* error handling wrapper for c_entrypoint

* removed unwrap

* more comments

* removed unused imports + changed import style

Author: majabbour

program/c/src/oracle/oracle.h

@@ -269,6 +269,12 @@ typedef enum {
   // key[1] price account         [writable]
   // key[2] sysvar_clock account  [readable]
   e_cmd_upd_price_no_fail_on_error,
+
+  // performs migation logic on the upgraded account. (resizes price accounts)
+  // key[0] funding account       [signer writable]
+  // key[1] upgraded account      [writable]
+  // key[2] system program        [readable]
+  e_cmd_upd_account_version,
 } command_t;
 
 typedef struct cmd_hdr

program/rust/build.rs

@@ -13,8 +13,6 @@ fn main() {
     parser.register_traits("pc_price_info", borsh_derives.to_vec());
     parser.register_traits("cmd_upd_price", borsh_derives.to_vec());
 
-
-
     //generate and write bindings
     let bindings = Builder::default()
         .header("./src/bindings.h")

program/rust/build_utils.rs

@@ -1,6 +1,7 @@
 use bindgen::callbacks::ParseCallbacks;
 use std::collections::HashMap;
 use std::panic::UnwindSafe;
+
 ///This type stores a hashmap from structnames
 ///to vectors of trait names, and ensures
 ///that the traits of each struct are added to its

program/rust/src/error.rs

@@ -1,14 +1,22 @@
 //! Error types
-
 use solana_program::program_error::ProgramError;
+use std::result::Result;
 use thiserror::Error;
+// similar to ProgramResult but allows for multiple success values
+pub type OracleResult = Result<u64, ProgramError>;
 
 /// Errors that may be returned by the oracle program
 #[derive(Clone, Debug, Eq, Error, PartialEq)]
 pub enum OracleError {
     /// Generic catch all error
     #[error("Generic")]
     Generic = 600,
+    /// integer casting error
+    #[error("IntegerCastingError")]
+    IntegerCastingError = 601,
+    /// c_entrypoint returned an unexpected value
+    #[error("UnknownCError")]
+    UnknownCError = 602,
 }
 
 impl From<OracleError> for ProgramError {

program/rust/src/lib.rs

@@ -1,10 +1,16 @@
 mod c_oracle_header;
-mod time_machine_types;
 mod error;
 mod log;
+mod processor;
+mod rust_oracle;
+mod time_machine_types;
 
+use crate::c_oracle_header::SUCCESSFULLY_UPDATED_AGGREGATE;
+use crate::error::{OracleError, OracleResult};
 use crate::log::{post_log, pre_log};
-use solana_program::entrypoint::deserialize;
+use processor::process_instruction;
+use solana_program::program_error::ProgramError;
+use solana_program::{custom_heap_default, custom_panic_default, entrypoint::deserialize};
 
 //Below is a high lever description of the rust/c setup.
 
@@ -36,29 +42,43 @@ pub extern "C" fn c_entrypoint(input: *mut u8) -> u64 {
     0 //SUCCESS value
 }
 
+pub fn c_entrypoint_wrapper(input: *mut u8) -> OracleResult {
+    //Throwing an exception from C into Rust is undefined behavior
+    //This seems to be the best we can do
+    match unsafe { c_entrypoint(input) } {
+        0 => Ok(0), // Success
+        SUCCESSFULLY_UPDATED_AGGREGATE => Ok(SUCCESSFULLY_UPDATED_AGGREGATE),
+        2 => Err(ProgramError::InvalidArgument), //2 is ERROR_INVALID_ARGUMENT in solana_sdk.h
+        _ => Err(OracleError::UnknownCError.into()),
+    }
+}
+
 #[no_mangle]
 pub extern "C" fn entrypoint(input: *mut u8) -> u64 {
-    let (_program_id, accounts, instruction_data) = unsafe { deserialize(input) };
+    let (program_id, accounts, instruction_data) = unsafe { deserialize(input) };
 
-    match pre_log(&accounts,instruction_data) {
+    match pre_log(&accounts, instruction_data) {
         Err(error) => return error.into(),
         _ => {}
     }
 
-    let c_ret_val = unsafe { c_entrypoint(input) };
+    let c_ret_val = match process_instruction(program_id, &accounts, instruction_data, input) {
+        Err(error) => error.into(),
+        Ok(success_status) => success_status,
+    };
 
     match post_log(c_ret_val, &accounts) {
         Err(error) => return error.into(),
         _ => {}
     }
 
-    if c_ret_val == c_oracle_header::SUCCESSFULLY_UPDATED_AGGREGATE {
+    if c_ret_val == SUCCESSFULLY_UPDATED_AGGREGATE {
         //0 is the SUCCESS value for solana
         return 0;
     } else {
         return c_ret_val;
     }
 }
 
-solana_program::custom_heap_default!();
-solana_program::custom_panic_default!();
+custom_heap_default!();
+custom_panic_default!();

program/rust/src/log.rs

@@ -6,7 +6,7 @@ use solana_program::entrypoint::ProgramResult;
 use solana_program::msg;
 use std::mem::size_of;
 
-pub fn pre_log(accounts : &[AccountInfo], instruction_data: &[u8]) -> ProgramResult {
+pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResult {
     msg!("Pyth oracle contract");
 
     let instruction_header: cmd_hdr = cmd_hdr::try_from_slice(&instruction_data[..8])?;
@@ -15,7 +15,7 @@ pub fn pre_log(accounts : &[AccountInfo], instruction_data: &[u8]) -> ProgramRes
         .try_into()
         .map_err(|_| OracleError::Generic)?;
     match instruction_id {
-        command_t_e_cmd_upd_price | command_t_e_cmd_agg_price=> {
+        command_t_e_cmd_upd_price | command_t_e_cmd_agg_price => {
             let instruction: cmd_upd_price = cmd_upd_price::try_from_slice(instruction_data)?;
             msg!(
                 "UpdatePrice: publisher={:}, price_account={:}, price={:}, conf={:}, status={:}, slot={:}",
@@ -29,7 +29,7 @@ pub fn pre_log(accounts : &[AccountInfo], instruction_data: &[u8]) -> ProgramRes
                 instruction.pub_slot_
             );
         }
-        command_t_e_cmd_upd_price_no_fail_on_error=> {
+        command_t_e_cmd_upd_price_no_fail_on_error => {
             let instruction: cmd_upd_price = cmd_upd_price::try_from_slice(instruction_data)?;
             msg!(
                 "UpdatePriceNoFailOnError: publisher={:}, price_account={:}, price={:}, conf={:}, status={:}, slot={:}",
@@ -93,8 +93,7 @@ pub fn post_log(c_ret_val: u64, accounts: &[AccountInfo]) -> ProgramResult {
         )?;
         msg!(
             "UpdateAggregate : price_account={:}, price={:}, conf={:}, status={:}, slot={:}",
-            accounts.get(1)
-            .ok_or(OracleError::Generic)?.key,
+            accounts.get(1).ok_or(OracleError::Generic)?.key,
             aggregate_price_info.price_,
             aggregate_price_info.conf_,
             aggregate_price_info.status_,

program/rust/src/processor.rs

@@ -0,0 +1,44 @@
+use crate::c_entrypoint_wrapper;
+use crate::c_oracle_header::{
+    cmd_hdr, command_t_e_cmd_agg_price, command_t_e_cmd_upd_account_version,
+    command_t_e_cmd_upd_price, command_t_e_cmd_upd_price_no_fail_on_error, PC_VERSION,
+};
+use crate::error::{OracleError, OracleResult};
+use crate::rust_oracle::{update_price, update_version};
+use ::std::mem::size_of;
+use borsh::BorshDeserialize;
+use solana_program::pubkey::Pubkey;
+use solana_program::sysvar::slot_history::AccountInfo;
+///dispatch to the right instruction in the oracle
+pub fn process_instruction(
+    program_id: &Pubkey,
+    accounts: &Vec<AccountInfo>,
+    instruction_data: &[u8],
+    input: *mut u8,
+) -> OracleResult {
+    let cmd_hdr_size = size_of::<cmd_hdr>();
+    let cmd_data = cmd_hdr::try_from_slice(&instruction_data[..cmd_hdr_size])?;
+
+    if cmd_data.ver_ != PC_VERSION {
+        //FIXME: I am not sure what's best to do here (this is copied from C)
+        // it seems to me like we should not break when version numbers change
+        //instead we should log a message that asks users to call update_version
+        panic!("incorrect version numbers");
+    }
+
+    match cmd_data
+        .cmd_
+        .try_into()
+        .map_err(|_| OracleError::IntegerCastingError)?
+    {
+        command_t_e_cmd_upd_price
+        | command_t_e_cmd_upd_price_no_fail_on_error
+        | command_t_e_cmd_agg_price => {
+            update_price(program_id, &accounts, &instruction_data, input)
+        }
+        command_t_e_cmd_upd_account_version => {
+            update_version(program_id, &accounts, &instruction_data)
+        }
+        _ => c_entrypoint_wrapper(input),
+    }
+}

program/rust/src/rust_oracle.rs

@@ -0,0 +1,27 @@
+use super::c_entrypoint_wrapper;
+use crate::error::OracleResult;
+use solana_program::pubkey::Pubkey;
+use solana_program::sysvar::slot_history::AccountInfo;
+
+///Calls the c oracle update_price, and updates the Time Machine if needed
+pub fn update_price(
+    program_id: &Pubkey,
+    accounts: &Vec<AccountInfo>,
+    instruction_data: &[u8],
+    input: *mut u8,
+) -> OracleResult {
+    //For now, we did not change the behavior of this. this is just to show the proposed structure of the
+    //program
+    c_entrypoint_wrapper(input)
+}
+/// has version number/ account type dependant logic to make sure the given account is compatible
+/// with the current version
+/// updates the version number for all accounts, and resizes price accounts
+pub fn update_version(
+    program_id: &Pubkey,
+    accounts: &Vec<AccountInfo>,
+    instruction_data: &[u8],
+) -> OracleResult {
+    panic!("Need to merge fix to pythd in order to implement this");
+    Ok(0) //SUCCESS
+}

program/rust/src/time_machine_types.rs

@@ -1,4 +1,5 @@
-use super::c_oracle_header;
+use super::c_oracle_header::TIME_MACHINE_STRUCT_SIZE;
+use ::std::mem::size_of;
 #[derive(Debug, Clone)]
 #[repr(C)]
 /// this wraps multiple SMA and tick trackers, and includes all the state
@@ -13,8 +14,10 @@ pub struct TimeMachineWrapper {
 ///defined in Rust
 fn c_time_machine_size_is_correct() {
     assert_eq!(
-        ::std::mem::size_of::<TimeMachineWrapper>(),
-        c_oracle_header::TIME_MACHINE_STRUCT_SIZE.try_into().unwrap(),
-        "expected TIME_MACHINE_STRUCT_SIZE in oracle.h to the same as the size of TimeMachineWrapper"
+        size_of::<TimeMachineWrapper>(),
+        TIME_MACHINE_STRUCT_SIZE.try_into().unwrap(),
+        "expected TIME_MACHINE_STRUCT_SIZE ({}) in oracle.h to the same as the size of TimeMachineWrapper ({})",
+        TIME_MACHINE_STRUCT_SIZE,
+        size_of::<TimeMachineWrapper>()
     );
 }