From a1efe92b667137c8e22832c1f47369bb7bf83890 Mon Sep 17 00:00:00 2001 From: majabbour <59592028+majabbour@users.noreply.github.com> Date: Mon, 25 Jul 2022 13:02:23 -0500 Subject: [PATCH 1/3] Make c robust to price account size (#201) * make pythd robust * made the oracle accept new size * left space for extra publishers --- pc/request.cpp | 6 +++--- program/c/src/oracle/oracle.c | 3 +++ program/c/src/oracle/oracle.h | 5 +++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/pc/request.cpp b/pc/request.cpp index 6f70b2c75..b95a175c0 100644 --- a/pc/request.cpp +++ b/pc/request.cpp @@ -354,7 +354,7 @@ void product::update( T *res ) return; } pc_prod_t *prod; - size_t plen = std::max( sizeof(pc_price_t), (size_t)PC_PROD_ACC_SIZE ); + size_t plen = std::max( PRICE_ACCOUNT_SIZE, (size_t)PC_PROD_ACC_SIZE ); if ( sizeof( pc_prod_t ) > res->get_data_ref( prod, plen ) || prod->magic_ != PC_MAGIC || !init_from_account( prod ) ) { @@ -464,7 +464,7 @@ price::price( const pub_key& acc, product *prod ) preq_->set_account( &apub_ ); areq_->set_sub( this ); preq_->set_sub( this ); - size_t tlen = ZSTD_compressBound( sizeof(pc_price_t) ); + size_t tlen = ZSTD_compressBound( PRICE_ACCOUNT_SIZE ); pptr_ = (pc_price_t*)new char[tlen]; __builtin_memset( pptr_, 0, tlen ); } @@ -952,7 +952,7 @@ void price::update( T *res ) } // get account data - size_t tlen = ZSTD_compressBound( sizeof(pc_price_t) ); + size_t tlen = ZSTD_compressBound( PRICE_ACCOUNT_SIZE ); res->get_data_val( pptr_, tlen ); if ( PC_UNLIKELY( pptr_->magic_ != PC_MAGIC ) ) { on_error_sub( "bad price account header", this ); diff --git a/program/c/src/oracle/oracle.c b/program/c/src/oracle/oracle.c index 17cbfc653..b7ab6e55e 100644 --- a/program/c/src/oracle/oracle.c +++ b/program/c/src/oracle/oracle.c @@ -5,6 +5,7 @@ #include "oracle.h" #include "upd_aggregate.h" + // Returns the minimum number of lamports required to make an account // with dlen bytes of data rent exempt. These values were calculated // using the getMinimumBalanceForRentExemption RPC call, and are @@ -19,6 +20,8 @@ static uint64_t rent_exempt_amount( uint64_t dlen ) return 4454400; case sizeof( pc_price_t ): return 23942400; + case PRICE_ACCOUNT_SIZE: + return 36915840; default: return UINT64_MAX; } diff --git a/program/c/src/oracle/oracle.h b/program/c/src/oracle/oracle.h index 6ca196855..ea4fa8bba 100644 --- a/program/c/src/oracle/oracle.h +++ b/program/c/src/oracle/oracle.h @@ -17,6 +17,9 @@ const uint64_t SUCCESSFULLY_UPDATED_AGGREGATE = 1000ULL; // Rust portion of the codebase. const uint64_t TIME_MACHINE_STRUCT_SIZE = 1864ULL; +const uint64_t EXTRA_PUBLISHER_SPACE = 1000ULL; + + // magic number at head of account #define PC_MAGIC 0xa1b2c3d4 @@ -190,6 +193,8 @@ typedef struct pc_price static_assert( sizeof( pc_price_t ) == 3312, "" ); +const uint64_t PRICE_ACCOUNT_SIZE = TIME_MACHINE_STRUCT_SIZE + EXTRA_PUBLISHER_SPACE + sizeof( pc_price_t ); + // command enumeration typedef enum { From 9934c60f5e304c6cc49332c0991611fef3b27a64 Mon Sep 17 00:00:00 2001 From: majabbour <59592028+majabbour@users.noreply.github.com> Date: Tue, 26 Jul 2022 09:14:34 -0500 Subject: [PATCH 2/3] Make c robust to price account size (#207) * make pythd robust * made the oracle accept new size * left space for extra publishers From b4390533edf36120d512d9aa3ac744cb4f688b42 Mon Sep 17 00:00:00 2001 From: majabbour <59592028+majabbour@users.noreply.github.com> Date: Tue, 26 Jul 2022 15:22:02 -0500 Subject: [PATCH 3/3] 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 --- program/c/src/oracle/oracle.h | 6 ++++ program/rust/build.rs | 2 -- program/rust/build_utils.rs | 1 + program/rust/src/error.rs | 10 +++++- program/rust/src/lib.rs | 36 ++++++++++++++++----- program/rust/src/log.rs | 9 +++--- program/rust/src/processor.rs | 44 ++++++++++++++++++++++++++ program/rust/src/rust_oracle.rs | 27 ++++++++++++++++ program/rust/src/time_machine_types.rs | 11 ++++--- 9 files changed, 126 insertions(+), 20 deletions(-) create mode 100644 program/rust/src/processor.rs create mode 100644 program/rust/src/rust_oracle.rs diff --git a/program/c/src/oracle/oracle.h b/program/c/src/oracle/oracle.h index ea4fa8bba..fc9ea6dac 100644 --- a/program/c/src/oracle/oracle.h +++ b/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 diff --git a/program/rust/build.rs b/program/rust/build.rs index e41e4b53b..ae5b99c18 100644 --- a/program/rust/build.rs +++ b/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") diff --git a/program/rust/build_utils.rs b/program/rust/build_utils.rs index 7848d73a3..a081373c1 100644 --- a/program/rust/build_utils.rs +++ b/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 diff --git a/program/rust/src/error.rs b/program/rust/src/error.rs index 8ea7c7fb8..ba859b7dd 100644 --- a/program/rust/src/error.rs +++ b/program/rust/src/error.rs @@ -1,7 +1,9 @@ //! 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; /// Errors that may be returned by the oracle program #[derive(Clone, Debug, Eq, Error, PartialEq)] @@ -9,6 +11,12 @@ 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 for ProgramError { diff --git a/program/rust/src/lib.rs b/program/rust/src/lib.rs index 2ddf766b0..1f664eeb2 100644 --- a/program/rust/src/lib.rs +++ b/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,23 +42,37 @@ 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 { @@ -60,5 +80,5 @@ pub extern "C" fn entrypoint(input: *mut u8) -> u64 { } } -solana_program::custom_heap_default!(); -solana_program::custom_panic_default!(); +custom_heap_default!(); +custom_panic_default!(); diff --git a/program/rust/src/log.rs b/program/rust/src/log.rs index 9176e3722..7e0274352 100644 --- a/program/rust/src/log.rs +++ b/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_, diff --git a/program/rust/src/processor.rs b/program/rust/src/processor.rs new file mode 100644 index 000000000..d13e3f82d --- /dev/null +++ b/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, + instruction_data: &[u8], + input: *mut u8, +) -> OracleResult { + let cmd_hdr_size = size_of::(); + 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), + } +} diff --git a/program/rust/src/rust_oracle.rs b/program/rust/src/rust_oracle.rs new file mode 100644 index 000000000..3ba916332 --- /dev/null +++ b/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, + 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, + instruction_data: &[u8], +) -> OracleResult { + panic!("Need to merge fix to pythd in order to implement this"); + Ok(0) //SUCCESS +} diff --git a/program/rust/src/time_machine_types.rs b/program/rust/src/time_machine_types.rs index 590fbedab..935ab2de0 100644 --- a/program/rust/src/time_machine_types.rs +++ b/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::(), - 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::(), + 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::() ); }