From aa938ab52c2397d2c0512e5cbfda3909514d8503 Mon Sep 17 00:00:00 2001 From: Mark Jabbour Date: Fri, 5 Aug 2022 20:57:17 +0000 Subject: [PATCH 1/3] removed borsh calls --- program/rust/Cargo.toml | 1 - program/rust/build.rs | 9 +--- program/rust/build_utils.rs | 1 + program/rust/src/bindings.h | 1 - program/rust/src/c_oracle_header.rs | 28 +++++++++-- program/rust/src/deserialize.rs | 73 +++++++++++++++++------------ program/rust/src/lib.rs | 2 + program/rust/src/log.rs | 52 +++++++------------- program/rust/src/price_t_offsets.h | 15 ------ program/rust/src/processor.rs | 4 +- program/rust/src/rust_oracle.rs | 46 +++--------------- program/rust/src/test_oracle.rs | 2 +- 12 files changed, 97 insertions(+), 137 deletions(-) delete mode 100644 program/rust/src/price_t_offsets.h diff --git a/program/rust/Cargo.toml b/program/rust/Cargo.toml index 3e5a584b0..babf453bd 100644 --- a/program/rust/Cargo.toml +++ b/program/rust/Cargo.toml @@ -10,7 +10,6 @@ bindgen = "0.60.1" [dependencies] solana-program = "=1.10.29" -borsh = "0.9" bytemuck = "1.11.0" thiserror = "1.0" diff --git a/program/rust/build.rs b/program/rust/build.rs index b8aee2acb..f8fa5a649 100644 --- a/program/rust/build.rs +++ b/program/rust/build.rs @@ -4,15 +4,8 @@ use bindgen::Builder; fn main() { println!("cargo:rustc-link-search=./program/c/target"); - let borsh_derives = ["BorshSerialize".to_string(), "BorshDeserialize".to_string()]; - //make a parser and to it type, traits pairs - let mut parser = build_utils::DeriveAdderParserCallback::new(); - parser.register_traits("cmd_hdr", borsh_derives.to_vec()); - parser.register_traits("pc_acc", borsh_derives.to_vec()); - parser.register_traits("pc_price_info", borsh_derives.to_vec()); - parser.register_traits("cmd_upd_price", borsh_derives.to_vec()); - parser.register_traits("pc_ema", borsh_derives.to_vec()); + let parser = build_utils::DeriveAdderParserCallback::new(); //generate and write bindings let bindings = Builder::default() diff --git a/program/rust/build_utils.rs b/program/rust/build_utils.rs index 0be568b66..6767fdd38 100644 --- a/program/rust/build_utils.rs +++ b/program/rust/build_utils.rs @@ -18,6 +18,7 @@ impl<'a> DeriveAdderParserCallback<'a> { Default::default() } //add pairs of types and their desired traits + #[allow(dead_code)] pub fn register_traits(&mut self, type_name: &'a str, traits: Vec) { self.types_to_traits.insert(type_name, traits); } diff --git a/program/rust/src/bindings.h b/program/rust/src/bindings.h index e2a2be48b..ab8ce0656 100644 --- a/program/rust/src/bindings.h +++ b/program/rust/src/bindings.h @@ -12,4 +12,3 @@ typedef signed long int int64_t; typedef unsigned long int uint64_t; #include "../../c/src/oracle/oracle.h" -#include "price_t_offsets.h" diff --git a/program/rust/src/c_oracle_header.rs b/program/rust/src/c_oracle_header.rs index 045642115..a5dbc9a80 100644 --- a/program/rust/src/c_oracle_header.rs +++ b/program/rust/src/c_oracle_header.rs @@ -3,10 +3,6 @@ //we do not use all the variables in oracle.h, so this helps with the warnings #![allow(dead_code)] //All the custom trait imports should go here -use borsh::{ - BorshDeserialize, - BorshSerialize, -}; use bytemuck::{ Pod, Zeroable, @@ -55,3 +51,27 @@ unsafe impl Zeroable for cmd_hdr { #[cfg(target_endian = "little")] unsafe impl Pod for cmd_hdr { } + +#[cfg(target_endian = "little")] +unsafe impl Zeroable for pc_price_info { +} + +#[cfg(target_endian = "little")] +unsafe impl Pod for pc_price_info { +} + +#[cfg(target_endian = "little")] +unsafe impl Zeroable for cmd_upd_price { +} + +#[cfg(target_endian = "little")] +unsafe impl Pod for cmd_upd_price { +} + +#[cfg(target_endian = "little")] +unsafe impl Zeroable for pc_ema { +} + +#[cfg(target_endian = "little")] +unsafe impl Pod for pc_ema { +} diff --git a/program/rust/src/deserialize.rs b/program/rust/src/deserialize.rs index 04fae794e..0f61ccef7 100644 --- a/program/rust/src/deserialize.rs +++ b/program/rust/src/deserialize.rs @@ -1,36 +1,47 @@ -use crate::c_oracle_header::size_t; -use crate::error::OracleError; -use borsh::BorshDeserialize; +use std::mem::size_of; + +use bytemuck::{ + try_from_bytes, + try_from_bytes_mut, + Pod, +}; + +use std::cell::{ + Ref, + RefMut, +}; + use solana_program::account_info::AccountInfo; use solana_program::program_error::ProgramError; -use std::mem::size_of; -use std::result::Result; - -/// Deserialize field in `source` with offset `offset` -pub fn deserialize_single_field_from_buffer( - source: &[u8], - offset: Option, -) -> Result { - let start: usize = offset - .unwrap_or(0) - .try_into() - .map_err(|_| OracleError::IntegerCastingError)?; - - let res: T = T::try_from_slice(&source[start..(start + size_of::())])?; - Ok(res) + +/// Interpret the bytes in `data` as a value of type `T` +pub fn load(data: &[u8]) -> Result<&T, ProgramError> { + try_from_bytes(&data[0..size_of::()]).map_err(|_| ProgramError::InvalidArgument) } -/// Deserialize field in `i` rank of `accounts` with offset `offset` -pub fn deserialize_single_field_from_account( - accounts: &[AccountInfo], - i: usize, - offset: Option, -) -> Result { - deserialize_single_field_from_buffer::( - &accounts - .get(i) - .ok_or(ProgramError::NotEnoughAccountKeys)? - .try_borrow_data()?, - offset, - ) +/// Interpret the bytes in `data` as a mutable value of type `T` +#[allow(unused)] +pub fn load_mut(data: &mut [u8]) -> Result<&mut T, ProgramError> { + try_from_bytes_mut(&mut data[0..size_of::()]).map_err(|_| ProgramError::InvalidArgument) +} + +/// Get the data stored in `account` as a value of type `T` +pub fn load_account_as<'a, T: Pod>(account: &'a AccountInfo) -> Result, ProgramError> { + let data = account.try_borrow_data()?; + + Ok(Ref::map(data, |data| { + bytemuck::from_bytes(&data[0..size_of::()]) + })) +} + +/// Mutably borrow the data in `account` as a value of type `T`. +/// Any mutations to the returned value will be reflected in the account data. +pub fn load_account_as_mut<'a, T: Pod>( + account: &'a AccountInfo, +) -> Result, ProgramError> { + let data = account.try_borrow_mut_data()?; + + Ok(RefMut::map(data, |data| { + bytemuck::from_bytes_mut(&mut data[0..size_of::()]) + })) } diff --git a/program/rust/src/lib.rs b/program/rust/src/lib.rs index 139950f48..1d852e5cd 100644 --- a/program/rust/src/lib.rs +++ b/program/rust/src/lib.rs @@ -19,6 +19,7 @@ use crate::error::{ OracleError, OracleResult, }; + use crate::log::{ post_log, pre_log, @@ -92,6 +93,7 @@ pub extern "C" fn entrypoint(input: *mut u8) -> u64 { return error.into(); } + if c_ret_val == SUCCESSFULLY_UPDATED_AGGREGATE { //0 is the SUCCESS value for solana 0 diff --git a/program/rust/src/log.rs b/program/rust/src/log.rs index bc7e3dac5..d01a88c1a 100644 --- a/program/rust/src/log.rs +++ b/program/rust/src/log.rs @@ -1,10 +1,9 @@ use crate::c_oracle_header::*; use crate::deserialize::{ - deserialize_single_field_from_account, - deserialize_single_field_from_buffer, + load, + load_account_as, }; use crate::error::OracleError; -use borsh::BorshDeserialize; use solana_program::account_info::AccountInfo; use solana_program::clock::Clock; use solana_program::entrypoint::ProgramResult; @@ -15,8 +14,7 @@ use solana_program::sysvar::Sysvar; pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResult { msg!("Pyth oracle contract"); - let instruction_header: cmd_hdr = - deserialize_single_field_from_buffer::(instruction_data, None)?; + let instruction_header: cmd_hdr = *load::(instruction_data)?; let instruction_id: u32 = instruction_header .cmd_ .try_into() @@ -25,13 +23,9 @@ pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResu match instruction_id { 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)?; + let instruction: cmd_upd_price = *load::(instruction_data)?; // Account 1 is price_info in this instruction - let expo: i32 = deserialize_single_field_from_account::( - accounts, - 1, - Some(PRICE_T_EXPO_OFFSET), - )?; + let price_account = load_account_as::(&accounts[1])?; msg!( "UpdatePrice: publisher={:}, price_account={:}, price={:}, conf={:}, expo={:}, status={:}, slot={:}, solana_time={:}", accounts.get(0) @@ -40,20 +34,16 @@ pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResu .ok_or(ProgramError::NotEnoughAccountKeys)?.key, instruction.price_, instruction.conf_, - expo, + price_account.expo_, instruction.status_, instruction.pub_slot_, Clock::get()?.unix_timestamp ); } command_t_e_cmd_upd_price_no_fail_on_error => { - let instruction: cmd_upd_price = cmd_upd_price::try_from_slice(instruction_data)?; + let instruction: cmd_upd_price = *load::(instruction_data)?; // Account 1 is price_info in this instruction - let expo: i32 = deserialize_single_field_from_account::( - accounts, - 1, - Some(PRICE_T_EXPO_OFFSET), - )?; + let price_account = load_account_as::(&accounts[1])?; msg!( "UpdatePriceNoFailOnError: publisher={:}, price_account={:}, price={:}, conf={:}, expo={:}, status={:}, slot={:}, solana_time={:}", accounts.get(0) @@ -62,7 +52,7 @@ pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResu .ok_or(ProgramError::NotEnoughAccountKeys)?.key, instruction.price_, instruction.conf_, - expo, + price_account.expo_, instruction.status_, instruction.pub_slot_, Clock::get()?.unix_timestamp @@ -104,31 +94,23 @@ pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResu Ok(()) } + pub fn post_log(c_ret_val: u64, accounts: &[AccountInfo]) -> ProgramResult { if c_ret_val == SUCCESSFULLY_UPDATED_AGGREGATE { // We trust that the C oracle has properly checked account 1, we can only get here through // the update price instructions - let aggregate_price_info: pc_price_info = deserialize_single_field_from_account::< - pc_price_info, - >( - accounts, 1, Some(PRICE_T_AGGREGATE_OFFSET) - )?; - let ema_info: pc_ema = - deserialize_single_field_from_account::(accounts, 1, Some(PRICE_T_EMA_OFFSET))?; - let expo: i32 = - deserialize_single_field_from_account::(accounts, 1, Some(PRICE_T_EXPO_OFFSET))?; - + let price_account = load_account_as::(&accounts[1])?; msg!( "UpdateAggregate : price_account={:}, price={:}, conf={:}, expo={:}, status={:}, slot={:}, solana_time={:}, ema={:}", accounts.get(1) .ok_or(ProgramError::NotEnoughAccountKeys)?.key, - aggregate_price_info.price_, - aggregate_price_info.conf_, - expo, - aggregate_price_info.status_, - aggregate_price_info.pub_slot_, + price_account.agg_.price_, + price_account.agg_.conf_, + price_account.expo_, + price_account.agg_.status_, + price_account.agg_.pub_slot_, Clock::get()?.unix_timestamp, - ema_info.val_ + price_account.twap_.val_ ); } Ok(()) diff --git a/program/rust/src/price_t_offsets.h b/program/rust/src/price_t_offsets.h deleted file mode 100644 index d515398b3..000000000 --- a/program/rust/src/price_t_offsets.h +++ /dev/null @@ -1,15 +0,0 @@ -//The constants below are used by the rust oracle to efficiently access members in price_t -//price_t is a big struct, attemting to deserialize all of it directly in borsh causes stack -//issues on Solana. It is also more efficient to only access the members we need -#include "../../c/src/oracle/oracle.h" -#include -const size_t PRICE_T_EXPO_OFFSET = offsetof(struct pc_price, expo_); -const size_t PRICE_T_TIMESTAMP_OFFSET = offsetof(struct pc_price, timestamp_); -const size_t PRICE_T_AGGREGATE_OFFSET = offsetof(struct pc_price, agg_); -const size_t PRICE_T_AGGREGATE_CONF_OFFSET = offsetof(struct pc_price, agg_) + offsetof(struct pc_price_info, conf_); -const size_t PRICE_T_AGGREGATE_PRICE_OFFSET = offsetof(struct pc_price, agg_) + offsetof(struct pc_price_info, price_); -const size_t PRICE_T_AGGREGATE_STATUS_OFFSET = offsetof(struct pc_price, agg_) + offsetof(struct pc_price_info, status_); -const size_t PRICE_T_EMA_OFFSET = offsetof(struct pc_price, twap_); -const size_t PRICE_T_PREV_TIMESTAMP_OFFSET = offsetof(struct pc_price, prev_timestamp_); -const size_t PRICE_T_PREV_CONF_OFFSET = offsetof(struct pc_price, prev_conf_); -const size_t PRICE_T_PREV_AGGREGATE_OFFSET = offsetof(struct pc_price, prev_price_); diff --git a/program/rust/src/processor.rs b/program/rust/src/processor.rs index f008a5013..6e7dbb260 100644 --- a/program/rust/src/processor.rs +++ b/program/rust/src/processor.rs @@ -1,6 +1,5 @@ use std::mem::size_of; -use borsh::BorshDeserialize; use solana_program::program_error::ProgramError; use solana_program::pubkey::Pubkey; use solana_program::sysvar::slot_history::AccountInfo; @@ -27,6 +26,7 @@ use crate::rust_oracle::{ update_version, }; +use crate::deserialize::load; ///dispatch to the right instruction in the oracle pub fn process_instruction( program_id: &Pubkey, @@ -38,7 +38,7 @@ pub fn process_instruction( if instruction_data.len() < cmd_hdr_size { return Err(ProgramError::InvalidArgument); } - let cmd_data = cmd_hdr::try_from_slice(&instruction_data[..cmd_hdr_size])?; + let cmd_data = load::(&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) diff --git a/program/rust/src/rust_oracle.rs b/program/rust/src/rust_oracle.rs index b47be8aee..bb9e33c47 100644 --- a/program/rust/src/rust_oracle.rs +++ b/program/rust/src/rust_oracle.rs @@ -1,18 +1,17 @@ use std::borrow::BorrowMut; -use std::cell::{ - Ref, - RefMut, -}; +use std::cell::RefMut; use std::mem::{ size_of, size_of_val, }; -use bytemuck::{ - try_from_bytes, - try_from_bytes_mut, - Pod, +use crate::deserialize::{ + load, + load_account_as, + load_account_as_mut, }; + + use solana_program::account_info::AccountInfo; use solana_program::entrypoint::SUCCESS; use solana_program::program_error::ProgramError; @@ -152,37 +151,6 @@ pub fn clear_account(account: &AccountInfo) -> Result<(), ProgramError> { Ok(()) } -/// Interpret the bytes in `data` as a value of type `T` -fn load(data: &[u8]) -> Result<&T, ProgramError> { - try_from_bytes(&data[0..size_of::()]).map_err(|_| ProgramError::InvalidArgument) -} - -/// Interpret the bytes in `data` as a mutable value of type `T` -#[allow(unused)] -fn load_mut(data: &mut [u8]) -> Result<&mut T, ProgramError> { - try_from_bytes_mut(&mut data[0..size_of::()]).map_err(|_| ProgramError::InvalidArgument) -} - -/// Get the data stored in `account` as a value of type `T` -pub fn load_account_as<'a, T: Pod>(account: &'a AccountInfo) -> Result, ProgramError> { - let data = account.try_borrow_data()?; - - Ok(Ref::map(data, |data| { - bytemuck::from_bytes(&data[0..size_of::()]) - })) -} - -/// Mutably borrow the data in `account` as a value of type `T`. -/// Any mutations to the returned value will be reflected in the account data. -pub fn load_account_as_mut<'a, T: Pod>( - account: &'a AccountInfo, -) -> Result, ProgramError> { - let data = account.try_borrow_mut_data()?; - - Ok(RefMut::map(data, |data| { - bytemuck::from_bytes_mut(&mut data[0..size_of::()]) - })) -} /// Mutably borrow the data in `account` as a mapping account, validating that the account /// is properly formatted. Any mutations to the returned value will be reflected in the diff --git a/program/rust/src/test_oracle.rs b/program/rust/src/test_oracle.rs index 0e313cc49..1bba9e914 100644 --- a/program/rust/src/test_oracle.rs +++ b/program/rust/src/test_oracle.rs @@ -8,10 +8,10 @@ mod test { PC_MAGIC, PC_VERSION, }; + use crate::deserialize::load_account_as; use crate::rust_oracle::{ clear_account, init_mapping, - load_account_as, }; use bytemuck::bytes_of; use solana_program::account_info::AccountInfo; From ea0c113e1c33c8991d2ebd1eb96aba26c48801cb Mon Sep 17 00:00:00 2001 From: Mark Jabbour Date: Fri, 5 Aug 2022 21:08:05 +0000 Subject: [PATCH 2/3] fixed merging errors --- program/rust/src/c_oracle_header.rs | 2 +- program/rust/src/rust_oracle.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/program/rust/src/c_oracle_header.rs b/program/rust/src/c_oracle_header.rs index cbb40dee1..d9802bb2a 100644 --- a/program/rust/src/c_oracle_header.rs +++ b/program/rust/src/c_oracle_header.rs @@ -75,6 +75,7 @@ unsafe impl Zeroable for pc_ema { #[cfg(target_endian = "little")] unsafe impl Pod for pc_ema { +} unsafe impl Zeroable for cmd_add_price_t { } @@ -89,5 +90,4 @@ unsafe impl Zeroable for pc_pub_key_t { #[cfg(target_endian = "little")] unsafe impl Pod for pc_pub_key_t { - } diff --git a/program/rust/src/rust_oracle.rs b/program/rust/src/rust_oracle.rs index 45cddba7f..ed4427ecb 100644 --- a/program/rust/src/rust_oracle.rs +++ b/program/rust/src/rust_oracle.rs @@ -11,6 +11,7 @@ use crate::deserialize::{ load_account_as_mut, }; +use bytemuck::bytes_of; use solana_program::account_info::AccountInfo; use solana_program::entrypoint::SUCCESS; From 4cf2907204773e858e49b9f0d27d89a1ba20df3e Mon Sep 17 00:00:00 2001 From: Mark Jabbour Date: Fri, 5 Aug 2022 21:28:02 +0000 Subject: [PATCH 3/3] stopped derefrencing --- program/rust/src/log.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/program/rust/src/log.rs b/program/rust/src/log.rs index d01a88c1a..e91dad706 100644 --- a/program/rust/src/log.rs +++ b/program/rust/src/log.rs @@ -14,7 +14,7 @@ use solana_program::sysvar::Sysvar; pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResult { msg!("Pyth oracle contract"); - let instruction_header: cmd_hdr = *load::(instruction_data)?; + let instruction_header: &cmd_hdr = load::(instruction_data)?; let instruction_id: u32 = instruction_header .cmd_ .try_into() @@ -23,7 +23,7 @@ pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResu match instruction_id { command_t_e_cmd_upd_price | command_t_e_cmd_agg_price => { - let instruction: cmd_upd_price = *load::(instruction_data)?; + let instruction: &cmd_upd_price = load::(instruction_data)?; // Account 1 is price_info in this instruction let price_account = load_account_as::(&accounts[1])?; msg!( @@ -41,7 +41,7 @@ pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResu ); } command_t_e_cmd_upd_price_no_fail_on_error => { - let instruction: cmd_upd_price = *load::(instruction_data)?; + let instruction: &cmd_upd_price = load::(instruction_data)?; // Account 1 is price_info in this instruction let price_account = load_account_as::(&accounts[1])?; msg!(