From 966f672e20756327aa9fb2d46ffdf06d1fd35110 Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Thu, 28 Jul 2022 20:02:42 +0000 Subject: [PATCH 01/20] Format --- .github/workflows/check-fomatting.yml | 19 +++++++++++++++++++ .pre-commit-config.yaml | 14 ++++++++++++++ Cargo.toml | 4 ++++ program/rust/.gitignore | 1 + program/rust/build_utils.rs | 3 ++- program/rust/src/c_oracle_header.rs | 5 ++++- program/rust/src/error.rs | 4 ++-- program/rust/src/lib.rs | 16 +++++++++++++--- program/rust/src/processor.rs | 18 ++++++++++++++---- program/rust/src/rust_oracle.rs | 4 ++-- 10 files changed, 75 insertions(+), 13 deletions(-) create mode 100644 .github/workflows/check-fomatting.yml create mode 100644 .pre-commit-config.yaml create mode 100644 Cargo.toml create mode 100644 program/rust/.gitignore diff --git a/.github/workflows/check-fomatting.yml b/.github/workflows/check-fomatting.yml new file mode 100644 index 000000000..7ae5af5aa --- /dev/null +++ b/.github/workflows/check-fomatting.yml @@ -0,0 +1,19 @@ +name: Check Formatting + +on: + pull_request: + push: + branches: [main] + +jobs: + pre-commit: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-python@v2 + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: nightly + components: rustfmt + - uses: pre-commit/action@v2.0.3 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 000000000..c419fd838 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,14 @@ +repos: +- repo: https://github.com/pre-commit/pre-commit-hooks + rev: v3.2.0 + hooks: + - id: trailing-whitespace + - id: end-of-file-fixer + - id: check-added-large-files +- repo: local + hooks: + - id: cargo-fmt-nightly + name: Cargo Fmt Nightly + language: "rust" + entry: cargo +nightly fmt + pass_filenames: false diff --git a/Cargo.toml b/Cargo.toml new file mode 100644 index 000000000..6bb097555 --- /dev/null +++ b/Cargo.toml @@ -0,0 +1,4 @@ +[workspace] +members = [ + "program/rust" +] diff --git a/program/rust/.gitignore b/program/rust/.gitignore new file mode 100644 index 000000000..55300bdb3 --- /dev/null +++ b/program/rust/.gitignore @@ -0,0 +1 @@ +bindings.rs diff --git a/program/rust/build_utils.rs b/program/rust/build_utils.rs index a081373c1..6c3cfcd85 100644 --- a/program/rust/build_utils.rs +++ b/program/rust/build_utils.rs @@ -24,7 +24,8 @@ impl<'a> DeriveAdderParserCallback<'a> { } //this is required to implement the callback trait -impl UnwindSafe for DeriveAdderParserCallback<'_> {} +impl UnwindSafe for DeriveAdderParserCallback<'_> { +} impl ParseCallbacks for DeriveAdderParserCallback<'_> { fn add_derives(&self, _name: &str) -> Vec { diff --git a/program/rust/src/c_oracle_header.rs b/program/rust/src/c_oracle_header.rs index 5432e3e7b..5f2f8399f 100644 --- a/program/rust/src/c_oracle_header.rs +++ b/program/rust/src/c_oracle_header.rs @@ -5,7 +5,10 @@ #![allow(unused_variables)] #![allow(dead_code)] //All the custom trait imports should go here -use borsh::{BorshDeserialize, BorshSerialize}; +use borsh::{ + BorshDeserialize, + BorshSerialize, +}; //bindings.rs is generated by build.rs to include //things defined in bindings.h include!("../bindings.rs"); diff --git a/program/rust/src/error.rs b/program/rust/src/error.rs index ba859b7dd..514fd8676 100644 --- a/program/rust/src/error.rs +++ b/program/rust/src/error.rs @@ -10,13 +10,13 @@ pub type OracleResult = Result; pub enum OracleError { /// Generic catch all error #[error("Generic")] - Generic = 600, + Generic = 600, /// integer casting error #[error("IntegerCastingError")] IntegerCastingError = 601, /// c_entrypoint returned an unexpected value #[error("UnknownCError")] - UnknownCError = 602, + UnknownCError = 602, } impl From for ProgramError { diff --git a/program/rust/src/lib.rs b/program/rust/src/lib.rs index 1f664eeb2..ad9aca120 100644 --- a/program/rust/src/lib.rs +++ b/program/rust/src/lib.rs @@ -6,11 +6,21 @@ 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 crate::error::{ + OracleError, + OracleResult, +}; +use crate::log::{ + post_log, + pre_log, +}; use processor::process_instruction; +use solana_program::entrypoint::deserialize; use solana_program::program_error::ProgramError; -use solana_program::{custom_heap_default, custom_panic_default, entrypoint::deserialize}; +use solana_program::{ + custom_heap_default, + custom_panic_default, +}; //Below is a high lever description of the rust/c setup. diff --git a/program/rust/src/processor.rs b/program/rust/src/processor.rs index d13e3f82d..6397674c3 100644 --- a/program/rust/src/processor.rs +++ b/program/rust/src/processor.rs @@ -1,10 +1,20 @@ 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, + 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 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; diff --git a/program/rust/src/rust_oracle.rs b/program/rust/src/rust_oracle.rs index 3ba916332..a17970e9f 100644 --- a/program/rust/src/rust_oracle.rs +++ b/program/rust/src/rust_oracle.rs @@ -10,8 +10,8 @@ pub fn update_price( 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 + //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 From bf9c96836cbbe0506f08ddb33758a7af64a68644 Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Thu, 28 Jul 2022 20:18:04 +0000 Subject: [PATCH 02/20] CI --- .pre-commit-config.yaml | 2 ++ program/c/src/oracle/model/clean | 1 - program/c/src/oracle/model/price_model.c | 7 +++---- program/c/src/oracle/model/run_tests | 1 - program/c/src/oracle/model/test_price_model.c | 1 - program/c/src/oracle/oracle.c | 1 - program/c/src/oracle/oracle.h | 2 +- program/c/src/oracle/pd.h | 1 - program/c/src/oracle/sort/clean | 1 - program/c/src/oracle/sort/run_tests | 1 - program/c/src/oracle/sort/sort_stable_base_gen.c | 1 - program/c/src/oracle/sort/test_sort_stable.c | 7 +++---- program/c/src/oracle/sort/tmpl/sort_stable.c | 5 ++--- program/c/src/oracle/upd_aggregate.h | 1 - program/c/src/oracle/util/avg.h | 7 +++---- program/c/src/oracle/util/clean | 1 - program/c/src/oracle/util/hash.h | 1 - program/c/src/oracle/util/prng.h | 10 +++++----- program/c/src/oracle/util/run_tests | 1 - program/c/src/oracle/util/sar.h | 1 - program/c/src/oracle/util/test_align.c | 3 +-- program/c/src/oracle/util/test_avg.c | 7 +++---- program/c/src/oracle/util/test_hash.c | 1 - program/c/src/oracle/util/test_prng.c | 1 - program/c/src/oracle/util/test_prng_battery.c | 1 - program/c/src/oracle/util/test_round.c | 1 - program/c/src/oracle/util/test_sar.c | 3 +-- program/c/src/oracle/util/util.h | 1 - program/rust/src/price_t_offsets.h | 2 +- 29 files changed, 25 insertions(+), 48 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c419fd838..88f24ce08 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -3,7 +3,9 @@ repos: rev: v3.2.0 hooks: - id: trailing-whitespace + files : program/ - id: end-of-file-fixer + files : program/ - id: check-added-large-files - repo: local hooks: diff --git a/program/c/src/oracle/model/clean b/program/c/src/oracle/model/clean index 860ef1641..2ca7bfcfd 100755 --- a/program/c/src/oracle/model/clean +++ b/program/c/src/oracle/model/clean @@ -1,3 +1,2 @@ #!/bin/sh rm -rfv bin - diff --git a/program/c/src/oracle/model/price_model.c b/program/c/src/oracle/model/price_model.c index 303d52e0a..846ee898d 100644 --- a/program/c/src/oracle/model/price_model.c +++ b/program/c/src/oracle/model/price_model.c @@ -17,14 +17,14 @@ price_model_core( uint64_t cnt, optimized mergesort (merge with an unrolled insertion sorting network small n base cases). The best case is ~0.5 n lg n compares and the average and worst cases are ~n lg n compares. - + While not completely data oblivious, this has quite low variance in operation count practically and this is _better_ than quicksort's average case and quicksort's worst case is a computational denial-of-service and timing attack vulnerable O(n^2). Unlike quicksort, this is also stable (but this stability does not currently matter ... it might be a factor in future models). - + A data oblivious sorting network approach might be viable here with and would have a completely deterministic operations count. It currently isn't used as the best known practical approaches for @@ -101,7 +101,7 @@ price_model_core( uint64_t cnt, *_p50 = avg_2_int64( vl, vr ); } - + /* Extract the p75 (this is the mirror image of the p25 case) */ uint64_t p75_idx = cnt - ((uint64_t)1) - p25_idx; @@ -110,4 +110,3 @@ price_model_core( uint64_t cnt, return sort_quote; } - diff --git a/program/c/src/oracle/model/run_tests b/program/c/src/oracle/model/run_tests index 11c8f8731..ab5c8a1ce 100755 --- a/program/c/src/oracle/model/run_tests +++ b/program/c/src/oracle/model/run_tests @@ -15,4 +15,3 @@ $CC test_price_model.c price_model.c -o bin/test_price_model || exit 1 bin/test_price_model || exit 1 echo all tests passed - diff --git a/program/c/src/oracle/model/test_price_model.c b/program/c/src/oracle/model/test_price_model.c index 321ff1746..06038d8e6 100644 --- a/program/c/src/oracle/model/test_price_model.c +++ b/program/c/src/oracle/model/test_price_model.c @@ -66,4 +66,3 @@ main( int argc, printf( "pass\n" ); return 0; } - diff --git a/program/c/src/oracle/oracle.c b/program/c/src/oracle/oracle.c index b7ab6e55e..e555d7d62 100644 --- a/program/c/src/oracle/oracle.c +++ b/program/c/src/oracle/oracle.c @@ -563,4 +563,3 @@ extern uint64_t c_entrypoint(const uint8_t *input) } return dispatch( prm, ka ); } - diff --git a/program/c/src/oracle/oracle.h b/program/c/src/oracle/oracle.h index fc9ea6dac..a85e0632c 100644 --- a/program/c/src/oracle/oracle.h +++ b/program/c/src/oracle/oracle.h @@ -13,7 +13,7 @@ extern "C" { // defines to u32 (even with ULL suffix) const uint64_t SUCCESSFULLY_UPDATED_AGGREGATE = 1000ULL; -// The size of the "time machine" account defined in the +// The size of the "time machine" account defined in the // Rust portion of the codebase. const uint64_t TIME_MACHINE_STRUCT_SIZE = 1864ULL; diff --git a/program/c/src/oracle/pd.h b/program/c/src/oracle/pd.h index 413e4ccdb..d8b282fae 100644 --- a/program/c/src/oracle/pd.h +++ b/program/c/src/oracle/pd.h @@ -188,4 +188,3 @@ static inline void pd_sqrt( pd_t *r, pd_t *val, const int64_t *f ) #ifdef __cplusplus } #endif - diff --git a/program/c/src/oracle/sort/clean b/program/c/src/oracle/sort/clean index 860ef1641..2ca7bfcfd 100755 --- a/program/c/src/oracle/sort/clean +++ b/program/c/src/oracle/sort/clean @@ -1,3 +1,2 @@ #!/bin/sh rm -rfv bin - diff --git a/program/c/src/oracle/sort/run_tests b/program/c/src/oracle/sort/run_tests index 0eb277605..310a2e4ae 100755 --- a/program/c/src/oracle/sort/run_tests +++ b/program/c/src/oracle/sort/run_tests @@ -15,4 +15,3 @@ $CC test_sort_stable.c -o bin/test_sort_stable || exit 1 bin/test_sort_stable || exit 1 echo all tests passed - diff --git a/program/c/src/oracle/sort/sort_stable_base_gen.c b/program/c/src/oracle/sort/sort_stable_base_gen.c index 598f85a4d..046abf8b8 100644 --- a/program/c/src/oracle/sort/sort_stable_base_gen.c +++ b/program/c/src/oracle/sort/sort_stable_base_gen.c @@ -92,4 +92,3 @@ main( int argc, sort_gen( n ); return 0; } - diff --git a/program/c/src/oracle/sort/test_sort_stable.c b/program/c/src/oracle/sort/test_sort_stable.c index 86d72b59f..dc620a0d8 100644 --- a/program/c/src/oracle/sort/test_sort_stable.c +++ b/program/c/src/oracle/sort/test_sort_stable.c @@ -49,11 +49,11 @@ main( int argc, prng_t _prng[1]; prng_t * prng = prng_join( prng_new( _prng, (uint32_t)0, (uint64_t)0 ) ); - int ctr = 0; + int ctr = 0; for( int iter=0; iter<10000000; iter++ ) { if( !ctr ) { printf( "Randomized: Completed %i iterations\n", iter ); ctr = 100000; } ctr--; - + int n = (int)(prng_uint32( prng ) % (uint32_t)(N+1)); /* In [0,N], approx uniform IID */ for( int i=0; i64-bit integer hash functions (i.e. full avalanche) with the property hash_uint64(0)==0 and hash_uint64(i) for i in [0,2^64) yields a permutation of [0,2^64) @@ -88,7 +88,7 @@ prng_private_contract( uint64_t seq ) { state of a _prng and returns ownership of the underlying memory region to the caller. There should be no joins in the system on the prng. Returns a pointer to the underlying memory region. - + FIXME: CONSIDER FLATTENING ALIGN? */ static inline uint64_t prng_footprint( void ) { return (uint64_t)sizeof ( prng_t ); } @@ -143,7 +143,7 @@ prng_idx_set( prng_t * prng, possible values of of a signed int uniform IID can be obtained by casting the output of the unsigned generator of the same, assuming typical twos complement arithmetic platform.) - + The theory for this that hash_uint64(i) for i in [0,2^64) specifies a random looking permutation of the integers in [0,2^64). Returning the low order bits of this random permutation then yields a high @@ -207,7 +207,7 @@ static inline int64_t prng_int64( prng_t * prng ) { return (int64_t)( prng_uint6 goes from [i/N,(i+1)/N) where i is in [0,N). For single (double) precision, "float" ("double"), the largest N for which the range of each interval is _exactly_ representable is N = 2^24 (2^53). - + Given then a uniform IID uint32_t random input, the prng_uint32_to_float_c0 converter output is as though an continuous IID uniform random in [0,1) was generated and then rounded down to @@ -246,7 +246,7 @@ static inline int64_t prng_int64( prng_t * prng ) { return (int64_t)( prng_uint6 Note that it is possible to make converters that will handle exactly rounding toward all possible floating point representations in [0,1] but this are significantly more expensive. - + Assumes IEEE-754 style float and doubles. FIXME: ADD UNIT TEST COVERAGE */ diff --git a/program/c/src/oracle/util/run_tests b/program/c/src/oracle/util/run_tests index 38b754b23..00ae9dbba 100755 --- a/program/c/src/oracle/util/run_tests +++ b/program/c/src/oracle/util/run_tests @@ -52,4 +52,3 @@ bin/test_avg || exit 1 #bin/test_prng_battery 4 0 0 || exit 1 # BigCrush: Takes >~3 hours (seq 0, idx 0) echo all tests passed - diff --git a/program/c/src/oracle/util/sar.h b/program/c/src/oracle/util/sar.h index 9b0722560..34322e423 100644 --- a/program/c/src/oracle/util/sar.h +++ b/program/c/src/oracle/util/sar.h @@ -71,4 +71,3 @@ sar_int64( int64_t x, #endif #endif /* _pyth_oracle_util_sar_h_ */ - diff --git a/program/c/src/oracle/util/test_align.c b/program/c/src/oracle/util/test_align.c index eaa55767b..f958c89f2 100644 --- a/program/c/src/oracle/util/test_align.c +++ b/program/c/src/oracle/util/test_align.c @@ -17,7 +17,7 @@ main( int argc, prng_t _prng[1]; prng_t * prng = prng_join( prng_new( _prng, (uint32_t)0, (uint64_t)0 ) ); - int ctr = 0; + int ctr = 0; for( int i=0; i<1000000000; i++ ) { if( !ctr ) { printf( "Completed %i iterations\n", i ); ctr = 10000000; } ctr--; @@ -74,4 +74,3 @@ main( int argc, return 0; } - diff --git a/program/c/src/oracle/util/test_avg.c b/program/c/src/oracle/util/test_avg.c index 21f042449..98f4f2502 100644 --- a/program/c/src/oracle/util/test_avg.c +++ b/program/c/src/oracle/util/test_avg.c @@ -10,12 +10,12 @@ main( int argc, prng_t * prng = prng_join( prng_new( _prng, (uint32_t)0, (uint64_t)0 ) ); int ctr; - - ctr = 0; + + ctr = 0; for( int i=0; i<1000000000; i++ ) { if( !ctr ) { printf( "reg: Completed %i iterations\n", i ); ctr = 10000000; } ctr--; - + # define TEST(w) do { \ uint##w##_t x = prng_uint##w( prng ); \ uint##w##_t y = prng_uint##w( prng ); \ @@ -140,4 +140,3 @@ main( int argc, return 0; } - diff --git a/program/c/src/oracle/util/test_hash.c b/program/c/src/oracle/util/test_hash.c index e4dcc6cf0..ce39b1e80 100644 --- a/program/c/src/oracle/util/test_hash.c +++ b/program/c/src/oracle/util/test_hash.c @@ -56,4 +56,3 @@ main( int argc, printf( "pass\n" ); return 0; } - diff --git a/program/c/src/oracle/util/test_prng.c b/program/c/src/oracle/util/test_prng.c index 20f65e360..d5a9ccafa 100644 --- a/program/c/src/oracle/util/test_prng.c +++ b/program/c/src/oracle/util/test_prng.c @@ -174,4 +174,3 @@ main( int argc, printf( "pass\n" ); return 0; } - diff --git a/program/c/src/oracle/util/test_prng_battery.c b/program/c/src/oracle/util/test_prng_battery.c index 41ffb9ee2..8a198ca8d 100644 --- a/program/c/src/oracle/util/test_prng_battery.c +++ b/program/c/src/oracle/util/test_prng_battery.c @@ -89,4 +89,3 @@ main( int argc, return 0; } - diff --git a/program/c/src/oracle/util/test_round.c b/program/c/src/oracle/util/test_round.c index aff69822f..c9326d832 100644 --- a/program/c/src/oracle/util/test_round.c +++ b/program/c/src/oracle/util/test_round.c @@ -28,4 +28,3 @@ main( int argc, return 0; } - diff --git a/program/c/src/oracle/util/test_sar.c b/program/c/src/oracle/util/test_sar.c index f0b1ac5ef..88366fb2c 100644 --- a/program/c/src/oracle/util/test_sar.c +++ b/program/c/src/oracle/util/test_sar.c @@ -12,7 +12,7 @@ main( int argc, /* FIXME: EXPLICT COVERAGE OF EDGE CASES (PROBABLY STATICALLY FULLY SAMPLED ALREADY THOUGH FOR 8 AND 16 BIT TYPES) */ - int ctr = 0; + int ctr = 0; for( int i=0; i<1000000000; i++ ) { if( !ctr ) { printf( "Completed %i iterations\n", i ); ctr = 10000000; } ctr--; @@ -46,4 +46,3 @@ main( int argc, return 0; } - diff --git a/program/c/src/oracle/util/util.h b/program/c/src/oracle/util/util.h index c6a179cc6..c9aaa36e8 100644 --- a/program/c/src/oracle/util/util.h +++ b/program/c/src/oracle/util/util.h @@ -8,4 +8,3 @@ #include "prng.h" /* includes stdalign.h and hash.h */ #endif /* _pyth_oracle_util_util_h_ */ - diff --git a/program/rust/src/price_t_offsets.h b/program/rust/src/price_t_offsets.h index 64df8d2f6..072be70cf 100644 --- a/program/rust/src/price_t_offsets.h +++ b/program/rust/src/price_t_offsets.h @@ -11,4 +11,4 @@ const size_t PRICE_T_AGGREGATE_PRICE_OFFSET = offsetof(struct pc_price, agg_) + const size_t PRICE_T_AGGREGATE_STATUS_OFFSET = offsetof(struct pc_price, agg_) + offsetof(struct pc_price_info, status_); 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_); \ No newline at end of file +const size_t PRICE_T_PREV_AGGREGATE_OFFSET = offsetof(struct pc_price, prev_price_); From 594948cef3805d5ee8c8a16ce07174a8287c3c54 Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Thu, 28 Jul 2022 20:23:54 +0000 Subject: [PATCH 03/20] Fixed --- program/README.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 program/README.md diff --git a/program/README.md b/program/README.md new file mode 100644 index 000000000..7ea34d71a --- /dev/null +++ b/program/README.md @@ -0,0 +1,5 @@ +### pre-commit hooks +pre-commit is a tool that checks and fixes simple issues (formatting, ...) before each commit. You can install it by following [their website](https://pre-commit.com/). In order to enable checks for this repo run `pre-commit install` from command-line in the root of this repo. + +The checks are also performed in the CI to ensure the code follows consistent formatting. +You might also need to install the nightly toolchain to run the formatting by running `rustup toolchain install nightly`. From f185b631b9806cb9d999538790b6fdac6f91c71c Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Thu, 28 Jul 2022 20:25:16 +0000 Subject: [PATCH 04/20] Add comment to README --- README.md | 6 ++++++ program/README.md | 5 ----- 2 files changed, 6 insertions(+), 5 deletions(-) delete mode 100644 program/README.md diff --git a/README.md b/README.md index 2e5d28897..0295ec09e 100644 --- a/README.md +++ b/README.md @@ -107,3 +107,9 @@ root@pyth-dev# usermod -u 1002 -g 1004 -s /bin/bash pyth Finally, in docker extension inside VS Code click right and choose "Attach VS Code". If you're using a remote host in VS Code make sure to let this connection be open. To get best experience from C++ IntelliSense, open entire `/home/pyth` in VS Code to include `solana` directory in home for lookup directories. + +### pre-commit hooks +pre-commit is a tool that checks and fixes simple issues (formatting, ...) before each commit. You can install it by following [their website](https://pre-commit.com/). In order to enable checks for this repo run `pre-commit install` from command-line in the root of this repo. + +The checks are also performed in the CI to ensure the code follows consistent formatting. Formatting is only currently enforced in the `program/` directory. +You might also need to install the nightly toolchain to run the formatting by running `rustup toolchain install nightly`. diff --git a/program/README.md b/program/README.md deleted file mode 100644 index 7ea34d71a..000000000 --- a/program/README.md +++ /dev/null @@ -1,5 +0,0 @@ -### pre-commit hooks -pre-commit is a tool that checks and fixes simple issues (formatting, ...) before each commit. You can install it by following [their website](https://pre-commit.com/). In order to enable checks for this repo run `pre-commit install` from command-line in the root of this repo. - -The checks are also performed in the CI to ensure the code follows consistent formatting. -You might also need to install the nightly toolchain to run the formatting by running `rustup toolchain install nightly`. From 6967eb2ebbbe8106c089ddf99cd65188a39ab7eb Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Thu, 28 Jul 2022 20:26:23 +0000 Subject: [PATCH 05/20] Format config --- rustfmt.toml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 rustfmt.toml diff --git a/rustfmt.toml b/rustfmt.toml new file mode 100644 index 000000000..89ad8b9a5 --- /dev/null +++ b/rustfmt.toml @@ -0,0 +1,22 @@ +# Merge similar crates together to avoid multiple use statements. +imports_granularity = "Module" + +# Consistency in formatting makes tool based searching/editing better. +empty_item_single_line = false + +# Easier editing when arbitrary mixed use statements do not collapse. +imports_layout = "Vertical" + +# Default rustfmt formatting of match arms with branches is awful. +match_arm_leading_pipes = "Preserve" + +# Align Fields +enum_discrim_align_threshold = 80 +struct_field_align_threshold = 80 + +# Allow up to two blank lines for grouping. +blank_lines_upper_bound = 2 + +# Wrap comments +comment_width = 120 +wrap_comments = true \ No newline at end of file From 3a46596b531ce2ca81f518e9fbe88fe0f7208ceb Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Thu, 28 Jul 2022 20:52:38 +0000 Subject: [PATCH 06/20] Fix build --- program/rust/build.rs | 2 +- scripts/build-bpf.sh | 10 +--------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/program/rust/build.rs b/program/rust/build.rs index ae5b99c18..4b8c39def 100644 --- a/program/rust/build.rs +++ b/program/rust/build.rs @@ -2,7 +2,7 @@ mod build_utils; use bindgen::Builder; fn main() { - println!("cargo:rustc-link-search=../c/target"); + println!("cargo:rustc-link-search=./program/c/target"); let borsh_derives = ["BorshSerialize".to_string(), "BorshDeserialize".to_string()]; diff --git a/scripts/build-bpf.sh b/scripts/build-bpf.sh index 8880ce140..2ed8be054 100755 --- a/scripts/build-bpf.sh +++ b/scripts/build-bpf.sh @@ -15,11 +15,6 @@ PYTH_DIR=$( cd "${1:-.}" && pwd) C_DIR="$( find $PYTH_DIR | grep makefile)" C_DIR=$(dirname $C_DIR) -#finds Cargo.toml in pyth-client -#ASSUMES THAT there is only one Cargo.toml there -RUST_DIR="$( find $PYTH_DIR | grep Cargo.toml )" -RUST_DIR=$(dirname $RUST_DIR) - if ! which cargo 2> /dev/null then # shellcheck disable=SC1090 @@ -39,16 +34,13 @@ rm ./target/*-keypair.json #build Rust and link it with C -cd "${RUST_DIR}" +cd "${PYTH_DIR}" cargo clean cargo test cargo clean cargo build-bpf sha256sum ./target/**/*.so -rm ./target/**/*-keypair.json -rm -r $PYTH_DIR/target || true -mv ./target $PYTH_DIR/target From ac2718d9dd06323c00eeaf9cf35c367d6fa7081c Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Thu, 28 Jul 2022 00:07:30 +0000 Subject: [PATCH 07/20] Log time and ema --- program/rust/build.rs | 1 + program/rust/src/log.rs | 34 ++++++++++++++++++++++++------ program/rust/src/price_t_offsets.h | 1 + 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/program/rust/build.rs b/program/rust/build.rs index 4b8c39def..b8aee2acb 100644 --- a/program/rust/build.rs +++ b/program/rust/build.rs @@ -12,6 +12,7 @@ fn main() { 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()); //generate and write bindings let bindings = Builder::default() diff --git a/program/rust/src/log.rs b/program/rust/src/log.rs index 7e0274352..43b1876f8 100644 --- a/program/rust/src/log.rs +++ b/program/rust/src/log.rs @@ -2,8 +2,10 @@ use crate::c_oracle_header::*; use crate::error::OracleError; use borsh::BorshDeserialize; use solana_program::account_info::AccountInfo; +use solana_program::clock::Clock; use solana_program::entrypoint::ProgramResult; use solana_program::msg; +use solana_program::sysvar::Sysvar; use std::mem::size_of; pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResult { @@ -14,11 +16,14 @@ pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResu .cmd_ .try_into() .map_err(|_| OracleError::Generic)?; + + let clock = Clock::get()?; + 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)?; msg!( - "UpdatePrice: publisher={:}, price_account={:}, price={:}, conf={:}, status={:}, slot={:}", + "UpdatePrice: publisher={:}, price_account={:}, price={:}, conf={:}, status={:}, slot={:}, solana_time={:}", accounts.get(0) .ok_or(OracleError::Generic)?.key, accounts.get(1) @@ -26,13 +31,14 @@ pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResu instruction.price_, instruction.conf_, instruction.status_, - instruction.pub_slot_ + instruction.pub_slot_, + clock.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)?; msg!( - "UpdatePriceNoFailOnError: publisher={:}, price_account={:}, price={:}, conf={:}, status={:}, slot={:}", + "UpdatePriceNoFailOnError: publisher={:}, price_account={:}, price={:}, conf={:}, status={:}, slot={:}, solana_time={:}", accounts.get(0) .ok_or(OracleError::Generic)?.key, accounts.get(1) @@ -40,7 +46,8 @@ pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResu instruction.price_, instruction.conf_, instruction.status_, - instruction.pub_slot_ + instruction.pub_slot_, + clock.unix_timestamp ); } command_t_e_cmd_add_mapping => { @@ -91,13 +98,26 @@ pub fn post_log(c_ret_val: u64, accounts: &[AccountInfo]) -> ProgramResult { .ok_or(OracleError::Generic)? .try_borrow_data()?[start..(start + size_of::())], )?; + + let ema_info: pc_ema = pc_ema::try_from_slice( + &accounts + .get(1) + .ok_or(OracleError::Generic)? + .try_borrow_data()?[start..(start + size_of::())], + )?; + + let clock = Clock::get()?; + msg!( - "UpdateAggregate : price_account={:}, price={:}, conf={:}, status={:}, slot={:}", - accounts.get(1).ok_or(OracleError::Generic)?.key, + "UpdateAggregate : price_account={:}, price={:}, conf={:}, status={:}, slot={:}, solana_time={:}, ema={:}", + accounts.get(1) + .ok_or(OracleError::Generic)?.key, aggregate_price_info.price_, aggregate_price_info.conf_, aggregate_price_info.status_, - aggregate_price_info.pub_slot_ + aggregate_price_info.pub_slot_, + clock.unix_timestamp, + ema_info.val_ ); } Ok(()) diff --git a/program/rust/src/price_t_offsets.h b/program/rust/src/price_t_offsets.h index 072be70cf..d515398b3 100644 --- a/program/rust/src/price_t_offsets.h +++ b/program/rust/src/price_t_offsets.h @@ -9,6 +9,7 @@ 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_); From 715a55b02f47d42aba02d1bfb5900c6653daa12f Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Thu, 28 Jul 2022 00:14:12 +0000 Subject: [PATCH 08/20] Fix ema deserialize --- program/rust/src/log.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/program/rust/src/log.rs b/program/rust/src/log.rs index 43b1876f8..ced0b7e33 100644 --- a/program/rust/src/log.rs +++ b/program/rust/src/log.rs @@ -88,7 +88,7 @@ pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResu pub fn post_log(c_ret_val: u64, accounts: &[AccountInfo]) -> ProgramResult { if c_ret_val == SUCCESSFULLY_UPDATED_AGGREGATE { - let start: usize = PRICE_T_AGGREGATE_OFFSET + let aggregate_price_start: usize = PRICE_T_AGGREGATE_OFFSET .try_into() .map_err(|_| OracleError::Generic)?; // We trust that the C oracle has properly checked this account @@ -96,14 +96,18 @@ pub fn post_log(c_ret_val: u64, accounts: &[AccountInfo]) -> ProgramResult { &accounts .get(1) .ok_or(OracleError::Generic)? - .try_borrow_data()?[start..(start + size_of::())], + .try_borrow_data()?[aggregate_price_start..(aggregate_price_start + size_of::())], )?; + let ema_start: usize = PRICE_T_EMA_OFFSET + .try_into() + .map_err(|_| OracleError::Generic)?; + let ema_info: pc_ema = pc_ema::try_from_slice( &accounts .get(1) .ok_or(OracleError::Generic)? - .try_borrow_data()?[start..(start + size_of::())], + .try_borrow_data()?[ema_start..(ema_start + size_of::())], )?; let clock = Clock::get()?; From 8410e80f8347c6d85addabd6d92117f6b9eab3cf Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Thu, 28 Jul 2022 00:15:51 +0000 Subject: [PATCH 09/20] Revert format --- program/rust/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/program/rust/src/lib.rs b/program/rust/src/lib.rs index ad9aca120..2ed760075 100644 --- a/program/rust/src/lib.rs +++ b/program/rust/src/lib.rs @@ -1,4 +1,5 @@ mod c_oracle_header; +mod time_machine_types; mod error; mod log; mod processor; @@ -67,7 +68,7 @@ pub fn c_entrypoint_wrapper(input: *mut u8) -> OracleResult { pub extern "C" fn entrypoint(input: *mut u8) -> u64 { 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(), _ => {} } From 905172a5ac6bd11bb89a9182f9192dfe00adc34c Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Thu, 28 Jul 2022 02:42:35 +0000 Subject: [PATCH 10/20] Add expo --- program/rust/src/log.rs | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/program/rust/src/log.rs b/program/rust/src/log.rs index ced0b7e33..b08c980f6 100644 --- a/program/rust/src/log.rs +++ b/program/rust/src/log.rs @@ -19,17 +19,28 @@ pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResu let clock = Clock::get()?; + let expo_start : usize = PRICE_T_EXPO_OFFSET.try_into() + .map_err(|_| OracleError::Generic)?; + + let expo: i32 = i32::try_from_slice( + &accounts + .get(1) + .ok_or(OracleError::Generic)? + .try_borrow_data()?[expo_start..(expo_start + size_of::())], + )?; + 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)?; msg!( - "UpdatePrice: publisher={:}, price_account={:}, price={:}, conf={:}, status={:}, slot={:}, solana_time={:}", + "UpdatePrice: publisher={:}, price_account={:}, price={:}, conf={:}, expo={:}, status={:}, slot={:}, solana_time={:}", accounts.get(0) .ok_or(OracleError::Generic)?.key, accounts.get(1) .ok_or(OracleError::Generic)?.key, instruction.price_, instruction.conf_, + expo, instruction.status_, instruction.pub_slot_, clock.unix_timestamp @@ -38,13 +49,14 @@ 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 = cmd_upd_price::try_from_slice(instruction_data)?; msg!( - "UpdatePriceNoFailOnError: publisher={:}, price_account={:}, price={:}, conf={:}, status={:}, slot={:}, solana_time={:}", + "UpdatePriceNoFailOnError: publisher={:}, price_account={:}, price={:}, conf={:}, expo={:}, status={:}, slot={:}, solana_time={:}", accounts.get(0) .ok_or(OracleError::Generic)?.key, accounts.get(1) .ok_or(OracleError::Generic)?.key, instruction.price_, instruction.conf_, + expo, instruction.status_, instruction.pub_slot_, clock.unix_timestamp @@ -110,14 +122,25 @@ pub fn post_log(c_ret_val: u64, accounts: &[AccountInfo]) -> ProgramResult { .try_borrow_data()?[ema_start..(ema_start + size_of::())], )?; + let expo_start : usize = PRICE_T_EXPO_OFFSET.try_into() + .map_err(|_| OracleError::Generic)?; + + let expo: i32 = i32::try_from_slice( + &accounts + .get(1) + .ok_or(OracleError::Generic)? + .try_borrow_data()?[expo_start..(expo_start + size_of::())], + )?; + let clock = Clock::get()?; msg!( - "UpdateAggregate : price_account={:}, price={:}, conf={:}, status={:}, slot={:}, solana_time={:}, ema={:}", + "UpdateAggregate : price_account={:}, price={:}, conf={:}, expo={:}, status={:}, slot={:}, solana_time={:}, ema={:}", accounts.get(1) .ok_or(OracleError::Generic)?.key, aggregate_price_info.price_, aggregate_price_info.conf_, + expo, aggregate_price_info.status_, aggregate_price_info.pub_slot_, clock.unix_timestamp, From 8a690b8132dcac8d0abaf6b52060e94bc177f2bb Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Thu, 28 Jul 2022 21:46:00 +0000 Subject: [PATCH 11/20] Fix comments --- program/rust/src/deserialize.rs | 36 +++++++++++++ program/rust/src/error.rs | 8 +-- program/rust/src/lib.rs | 5 +- program/rust/src/log.rs | 94 ++++++++++++++------------------- 4 files changed, 83 insertions(+), 60 deletions(-) create mode 100644 program/rust/src/deserialize.rs diff --git a/program/rust/src/deserialize.rs b/program/rust/src/deserialize.rs new file mode 100644 index 000000000..12565e6f3 --- /dev/null +++ b/program/rust/src/deserialize.rs @@ -0,0 +1,36 @@ +use crate::c_oracle_header::size_t; +use crate::error::OracleError; +use borsh::BorshDeserialize; +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) +} + +/// Deserialize field in `i` rank of `accounts` with offset `offset` +pub fn deserialize_single_field_from_account( + accounts: &[AccountInfo], + i: usize, + offset: Option, +) -> Result { + Ok(deserialize_single_field_from_buffer::( + &accounts + .get(i) + .ok_or(ProgramError::NotEnoughAccountKeys)? + .try_borrow_data()?, + offset, + )?) +} diff --git a/program/rust/src/error.rs b/program/rust/src/error.rs index 514fd8676..0bdc2b1ac 100644 --- a/program/rust/src/error.rs +++ b/program/rust/src/error.rs @@ -10,13 +10,15 @@ pub type OracleResult = Result; 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, } impl From for ProgramError { diff --git a/program/rust/src/lib.rs b/program/rust/src/lib.rs index 2ed760075..bd0373ca9 100644 --- a/program/rust/src/lib.rs +++ b/program/rust/src/lib.rs @@ -1,5 +1,5 @@ mod c_oracle_header; -mod time_machine_types; +mod deserialize; mod error; mod log; mod processor; @@ -7,6 +7,7 @@ mod rust_oracle; mod time_machine_types; use crate::c_oracle_header::SUCCESSFULLY_UPDATED_AGGREGATE; +use crate::deserialize::deserialize_single_field_from_buffer; use crate::error::{ OracleError, OracleResult, @@ -68,7 +69,7 @@ pub fn c_entrypoint_wrapper(input: *mut u8) -> OracleResult { pub extern "C" fn entrypoint(input: *mut u8) -> u64 { 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(), _ => {} } diff --git a/program/rust/src/log.rs b/program/rust/src/log.rs index b08c980f6..a9485b9b1 100644 --- a/program/rust/src/log.rs +++ b/program/rust/src/log.rs @@ -1,65 +1,71 @@ use crate::c_oracle_header::*; +use crate::deserialize::{ + deserialize_single_field_from_account, + deserialize_single_field_from_buffer, +}; use crate::error::OracleError; use borsh::BorshDeserialize; use solana_program::account_info::AccountInfo; use solana_program::clock::Clock; use solana_program::entrypoint::ProgramResult; use solana_program::msg; +use solana_program::program_error::ProgramError; use solana_program::sysvar::Sysvar; -use std::mem::size_of; 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])?; + let instruction_header: cmd_hdr = + deserialize_single_field_from_buffer::(&instruction_data, None)?; let instruction_id: u32 = instruction_header .cmd_ .try_into() - .map_err(|_| OracleError::Generic)?; + .map_err(|_| OracleError::IntegerCastingError)?; - let clock = Clock::get()?; - - let expo_start : usize = PRICE_T_EXPO_OFFSET.try_into() - .map_err(|_| OracleError::Generic)?; - - let expo: i32 = i32::try_from_slice( - &accounts - .get(1) - .ok_or(OracleError::Generic)? - .try_borrow_data()?[expo_start..(expo_start + size_of::())], - )?; 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)?; + // Account 1 is price_info in this instruction + let expo: i32 = deserialize_single_field_from_account::( + accounts, + 1, + Some(PRICE_T_EXPO_OFFSET), + )?; msg!( "UpdatePrice: publisher={:}, price_account={:}, price={:}, conf={:}, expo={:}, status={:}, slot={:}, solana_time={:}", accounts.get(0) - .ok_or(OracleError::Generic)?.key, + .ok_or(ProgramError::NotEnoughAccountKeys)?.key, accounts.get(1) - .ok_or(OracleError::Generic)?.key, + .ok_or(ProgramError::NotEnoughAccountKeys)?.key, instruction.price_, instruction.conf_, expo, instruction.status_, instruction.pub_slot_, - clock.unix_timestamp + 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)?; + // Account 1 is price_info in this instruction + let expo: i32 = deserialize_single_field_from_account::( + accounts, + 1, + Some(PRICE_T_EXPO_OFFSET), + )?; msg!( "UpdatePriceNoFailOnError: publisher={:}, price_account={:}, price={:}, conf={:}, expo={:}, status={:}, slot={:}, solana_time={:}", accounts.get(0) - .ok_or(OracleError::Generic)?.key, + .ok_or(ProgramError::NotEnoughAccountKeys)?.key, accounts.get(1) - .ok_or(OracleError::Generic)?.key, + .ok_or(ProgramError::NotEnoughAccountKeys)?.key, instruction.price_, instruction.conf_, expo, instruction.status_, instruction.pub_slot_, - clock.unix_timestamp + Clock::get()?.unix_timestamp ); } command_t_e_cmd_add_mapping => { @@ -92,7 +98,7 @@ pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResu } _ => { msg!("UnrecognizedInstruction"); - return Err(OracleError::Generic.into()); + return Err(OracleError::UnrecognizedInstruction.into()); } } Ok(()) @@ -100,50 +106,28 @@ pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResu pub fn post_log(c_ret_val: u64, accounts: &[AccountInfo]) -> ProgramResult { if c_ret_val == SUCCESSFULLY_UPDATED_AGGREGATE { - let aggregate_price_start: usize = PRICE_T_AGGREGATE_OFFSET - .try_into() - .map_err(|_| OracleError::Generic)?; - // We trust that the C oracle has properly checked this account - let aggregate_price_info: pc_price_info = pc_price_info::try_from_slice( - &accounts - .get(1) - .ok_or(OracleError::Generic)? - .try_borrow_data()?[aggregate_price_start..(aggregate_price_start + size_of::())], - )?; - - let ema_start: usize = PRICE_T_EMA_OFFSET - .try_into() - .map_err(|_| OracleError::Generic)?; - - let ema_info: pc_ema = pc_ema::try_from_slice( - &accounts - .get(1) - .ok_or(OracleError::Generic)? - .try_borrow_data()?[ema_start..(ema_start + size_of::())], + // 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 expo_start : usize = PRICE_T_EXPO_OFFSET.try_into() - .map_err(|_| OracleError::Generic)?; - - let expo: i32 = i32::try_from_slice( - &accounts - .get(1) - .ok_or(OracleError::Generic)? - .try_borrow_data()?[expo_start..(expo_start + size_of::())], - )?; - - let clock = Clock::get()?; + 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))?; msg!( "UpdateAggregate : price_account={:}, price={:}, conf={:}, expo={:}, status={:}, slot={:}, solana_time={:}, ema={:}", accounts.get(1) - .ok_or(OracleError::Generic)?.key, + .ok_or(ProgramError::NotEnoughAccountKeys)?.key, aggregate_price_info.price_, aggregate_price_info.conf_, expo, aggregate_price_info.status_, aggregate_price_info.pub_slot_, - clock.unix_timestamp, + Clock::get()?.unix_timestamp, ema_info.val_ ); } From a5509f04279d00be8283ce9fa29220ddde4532b1 Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Thu, 28 Jul 2022 21:52:27 +0000 Subject: [PATCH 12/20] Unused import --- program/rust/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/program/rust/src/lib.rs b/program/rust/src/lib.rs index bd0373ca9..05938cc95 100644 --- a/program/rust/src/lib.rs +++ b/program/rust/src/lib.rs @@ -7,7 +7,6 @@ mod rust_oracle; mod time_machine_types; use crate::c_oracle_header::SUCCESSFULLY_UPDATED_AGGREGATE; -use crate::deserialize::deserialize_single_field_from_buffer; use crate::error::{ OracleError, OracleResult, From 74ce22358841d5b7656c9d43ddf4ce16c67249bd Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Fri, 29 Jul 2022 00:15:24 +0000 Subject: [PATCH 13/20] Clippy --- program/rust/src/c_oracle_header.rs | 1 - program/rust/src/lib.rs | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/program/rust/src/c_oracle_header.rs b/program/rust/src/c_oracle_header.rs index 5f2f8399f..1b55a4cb2 100644 --- a/program/rust/src/c_oracle_header.rs +++ b/program/rust/src/c_oracle_header.rs @@ -1,4 +1,3 @@ -#![allow(non_upper_case_globals)] #![allow(non_camel_case_types)] #![allow(non_snake_case)] //we do not use all the variables in oracle.h, so this helps with the warnings diff --git a/program/rust/src/lib.rs b/program/rust/src/lib.rs index 05938cc95..03a1b72d2 100644 --- a/program/rust/src/lib.rs +++ b/program/rust/src/lib.rs @@ -1,3 +1,5 @@ +#![allow(non_upper_case_globals)] + mod c_oracle_header; mod deserialize; mod error; @@ -66,6 +68,7 @@ pub fn c_entrypoint_wrapper(input: *mut u8) -> OracleResult { #[no_mangle] pub extern "C" fn entrypoint(input: *mut u8) -> u64 { + #[allow(clippy::not_unsafe_ptr_arg_deref)] let (program_id, accounts, instruction_data) = unsafe { deserialize(input) }; match pre_log(&accounts, instruction_data) { From 4b5371e717577d08dec9b4ff209623f90afc620d Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Fri, 29 Jul 2022 18:18:11 +0000 Subject: [PATCH 14/20] Clippy 2 --- program/rust/build_utils.rs | 2 +- program/rust/src/deserialize.rs | 4 ++-- program/rust/src/lib.rs | 18 ++++++++---------- program/rust/src/log.rs | 2 +- program/rust/src/processor.rs | 8 +++----- program/rust/src/rust_oracle.rs | 13 ++++++------- program/rust/src/time_machine_types.rs | 2 -- 7 files changed, 21 insertions(+), 28 deletions(-) diff --git a/program/rust/build_utils.rs b/program/rust/build_utils.rs index 6c3cfcd85..0be568b66 100644 --- a/program/rust/build_utils.rs +++ b/program/rust/build_utils.rs @@ -19,7 +19,7 @@ impl<'a> DeriveAdderParserCallback<'a> { } //add pairs of types and their desired traits pub fn register_traits(&mut self, type_name: &'a str, traits: Vec) { - self.types_to_traits.insert(&type_name, traits); + self.types_to_traits.insert(type_name, traits); } } diff --git a/program/rust/src/deserialize.rs b/program/rust/src/deserialize.rs index 12565e6f3..04fae794e 100644 --- a/program/rust/src/deserialize.rs +++ b/program/rust/src/deserialize.rs @@ -26,11 +26,11 @@ pub fn deserialize_single_field_from_account( i: usize, offset: Option, ) -> Result { - Ok(deserialize_single_field_from_buffer::( + deserialize_single_field_from_buffer::( &accounts .get(i) .ok_or(ProgramError::NotEnoughAccountKeys)? .try_borrow_data()?, offset, - )?) + ) } diff --git a/program/rust/src/lib.rs b/program/rust/src/lib.rs index 03a1b72d2..89b1f0ea9 100644 --- a/program/rust/src/lib.rs +++ b/program/rust/src/lib.rs @@ -51,14 +51,14 @@ extern "C" { //make the C entrypoint a no-op when running cargo test #[cfg(not(target_arch = "bpf"))] -pub extern "C" fn c_entrypoint(input: *mut u8) -> u64 { +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) } { + match 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 @@ -71,9 +71,8 @@ pub extern "C" fn entrypoint(input: *mut u8) -> u64 { #[allow(clippy::not_unsafe_ptr_arg_deref)] let (program_id, accounts, instruction_data) = unsafe { deserialize(input) }; - match pre_log(&accounts, instruction_data) { - Err(error) => return error.into(), - _ => {} + if let Err(error) = pre_log(&accounts, instruction_data) { + return error.into(); } let c_ret_val = match process_instruction(program_id, &accounts, instruction_data, input) { @@ -81,16 +80,15 @@ pub extern "C" fn entrypoint(input: *mut u8) -> u64 { Ok(success_status) => success_status, }; - match post_log(c_ret_val, &accounts) { - Err(error) => return error.into(), - _ => {} + if let Err(error) = post_log(c_ret_val, &accounts) { + return error.into(); } if c_ret_val == SUCCESSFULLY_UPDATED_AGGREGATE { //0 is the SUCCESS value for solana - return 0; + 0 } else { - return c_ret_val; + c_ret_val } } diff --git a/program/rust/src/log.rs b/program/rust/src/log.rs index a9485b9b1..bc7e3dac5 100644 --- a/program/rust/src/log.rs +++ b/program/rust/src/log.rs @@ -16,7 +16,7 @@ pub fn pre_log(accounts: &[AccountInfo], instruction_data: &[u8]) -> ProgramResu msg!("Pyth oracle contract"); let instruction_header: cmd_hdr = - deserialize_single_field_from_buffer::(&instruction_data, None)?; + deserialize_single_field_from_buffer::(instruction_data, None)?; let instruction_id: u32 = instruction_header .cmd_ .try_into() diff --git a/program/rust/src/processor.rs b/program/rust/src/processor.rs index 6397674c3..79e831024 100644 --- a/program/rust/src/processor.rs +++ b/program/rust/src/processor.rs @@ -22,7 +22,7 @@ use solana_program::sysvar::slot_history::AccountInfo; ///dispatch to the right instruction in the oracle pub fn process_instruction( program_id: &Pubkey, - accounts: &Vec, + accounts: &[AccountInfo], instruction_data: &[u8], input: *mut u8, ) -> OracleResult { @@ -43,11 +43,9 @@ pub fn process_instruction( { 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_agg_price => update_price(program_id, accounts, instruction_data, input), command_t_e_cmd_upd_account_version => { - update_version(program_id, &accounts, &instruction_data) + 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 index a17970e9f..141814409 100644 --- a/program/rust/src/rust_oracle.rs +++ b/program/rust/src/rust_oracle.rs @@ -5,9 +5,9 @@ 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], + _program_id: &Pubkey, + _accounts: &[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 @@ -18,10 +18,9 @@ pub fn update_price( /// 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], + _program_id: &Pubkey, + _accounts: &[AccountInfo], + _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 935ab2de0..d9f1af3ce 100644 --- a/program/rust/src/time_machine_types.rs +++ b/program/rust/src/time_machine_types.rs @@ -1,5 +1,3 @@ -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 From fb8239dacc66a4ac5ffacede9bf38932faeecc1c Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Tue, 2 Aug 2022 23:31:10 +0000 Subject: [PATCH 15/20] Clippy --- .github/workflows/check-fomatting.yml | 2 +- .pre-commit-config.yaml | 5 +++++ program/rust/Cargo.toml | 2 -- program/rust/src/lib.rs | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/check-fomatting.yml b/.github/workflows/check-fomatting.yml index 7ae5af5aa..edf734f05 100644 --- a/.github/workflows/check-fomatting.yml +++ b/.github/workflows/check-fomatting.yml @@ -15,5 +15,5 @@ jobs: with: profile: minimal toolchain: nightly - components: rustfmt + components: rustfmt, clippy - uses: pre-commit/action@v2.0.3 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 88f24ce08..466e630e5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -14,3 +14,8 @@ repos: language: "rust" entry: cargo +nightly fmt pass_filenames: false + - id: cargo-clippy + name: Cargo clippy + language: "rust" + entry : cargo +nightly clippy -- -D warnings + pass_filenames : false diff --git a/program/rust/Cargo.toml b/program/rust/Cargo.toml index c3e881ac5..f3e4492bd 100644 --- a/program/rust/Cargo.toml +++ b/program/rust/Cargo.toml @@ -1,5 +1,3 @@ -cargo-features = ["edition2021"] - [package] name = "pyth-oracle" version = "2.13.1" diff --git a/program/rust/src/lib.rs b/program/rust/src/lib.rs index 89b1f0ea9..ed1df9436 100644 --- a/program/rust/src/lib.rs +++ b/program/rust/src/lib.rs @@ -1,4 +1,5 @@ #![allow(non_upper_case_globals)] +#![allow(clippy::not_unsafe_ptr_arg_deref)] mod c_oracle_header; mod deserialize; @@ -18,6 +19,7 @@ use crate::log::{ pre_log, }; use processor::process_instruction; + use solana_program::entrypoint::deserialize; use solana_program::program_error::ProgramError; use solana_program::{ @@ -68,7 +70,6 @@ pub fn c_entrypoint_wrapper(input: *mut u8) -> OracleResult { #[no_mangle] pub extern "C" fn entrypoint(input: *mut u8) -> u64 { - #[allow(clippy::not_unsafe_ptr_arg_deref)] let (program_id, accounts, instruction_data) = unsafe { deserialize(input) }; if let Err(error) = pre_log(&accounts, instruction_data) { From 0360db5d3927208f65f3c39c4a73db96618c0764 Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Tue, 2 Aug 2022 23:38:03 +0000 Subject: [PATCH 16/20] Fix tests --- program/rust/src/time_machine_types.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/program/rust/src/time_machine_types.rs b/program/rust/src/time_machine_types.rs index d9f1af3ce..8298f6baa 100644 --- a/program/rust/src/time_machine_types.rs +++ b/program/rust/src/time_machine_types.rs @@ -7,15 +7,21 @@ pub struct TimeMachineWrapper { place_holder: [u8; 1864], } -#[test] -///test that the size defined in C matches that -///defined in Rust -fn c_time_machine_size_is_correct() { - assert_eq!( +#[cfg(test)] +pub mod tests { + use crate::c_oracle_header::TIME_MACHINE_STRUCT_SIZE; + use crate::time_machine_types::TimeMachineWrapper; + use std::mem::size_of; + #[test] + ///test that the size defined in C matches that + ///defined in Rust + fn c_time_machine_size_is_correct() { + assert_eq!( 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::() ); + } } From 990fdde6c6dc5e3ab086008ad1fc0157e98a0ad1 Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Tue, 2 Aug 2022 23:59:14 +0000 Subject: [PATCH 17/20] Deny warnings --- program/rust/src/c_oracle_header.rs | 3 --- program/rust/src/lib.rs | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/program/rust/src/c_oracle_header.rs b/program/rust/src/c_oracle_header.rs index 1b55a4cb2..0ea9f2d03 100644 --- a/program/rust/src/c_oracle_header.rs +++ b/program/rust/src/c_oracle_header.rs @@ -1,7 +1,4 @@ #![allow(non_camel_case_types)] -#![allow(non_snake_case)] -//we do not use all the variables in oracle.h, so this helps with the warnings -#![allow(unused_variables)] #![allow(dead_code)] //All the custom trait imports should go here use borsh::{ diff --git a/program/rust/src/lib.rs b/program/rust/src/lib.rs index ed1df9436..7aed2a5f4 100644 --- a/program/rust/src/lib.rs +++ b/program/rust/src/lib.rs @@ -1,3 +1,4 @@ +#![deny(warnings)] #![allow(non_upper_case_globals)] #![allow(clippy::not_unsafe_ptr_arg_deref)] From 51291e4ead7cad4321785b9ff033f3edd181fbd7 Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Tue, 2 Aug 2022 23:59:53 +0000 Subject: [PATCH 18/20] Restore allow --- program/rust/src/c_oracle_header.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/program/rust/src/c_oracle_header.rs b/program/rust/src/c_oracle_header.rs index 0ea9f2d03..55bd17be2 100644 --- a/program/rust/src/c_oracle_header.rs +++ b/program/rust/src/c_oracle_header.rs @@ -1,4 +1,6 @@ #![allow(non_camel_case_types)] +#![allow(non_snake_case)] +//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::{ From 931321d8c78fd211f2ce8cac29dd6e6f3db749c2 Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Wed, 3 Aug 2022 13:56:04 +0000 Subject: [PATCH 19/20] CI --- program/rust/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/program/rust/src/lib.rs b/program/rust/src/lib.rs index 7aed2a5f4..fe1786759 100644 --- a/program/rust/src/lib.rs +++ b/program/rust/src/lib.rs @@ -54,14 +54,15 @@ extern "C" { //make the C entrypoint a no-op when running cargo test #[cfg(not(target_arch = "bpf"))] -pub extern "C" fn c_entrypoint(_input: *mut u8) -> u64 { +#[allow(clippy::missing_safety_doc)] +pub unsafe 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 c_entrypoint(input) { + 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 From 4352746583437d9ca34c4afb1d988374d5db50bf Mon Sep 17 00:00:00 2001 From: Guillermo Bescos Date: Wed, 3 Aug 2022 14:41:14 +0000 Subject: [PATCH 20/20] Comments --- program/rust/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/program/rust/src/lib.rs b/program/rust/src/lib.rs index fe1786759..e397a0506 100644 --- a/program/rust/src/lib.rs +++ b/program/rust/src/lib.rs @@ -1,5 +1,7 @@ #![deny(warnings)] +// Allow non upper case globals from C #![allow(non_upper_case_globals)] +// Allow using the solana_program::entrypoint::deserialize function #![allow(clippy::not_unsafe_ptr_arg_deref)] mod c_oracle_header; @@ -53,6 +55,7 @@ extern "C" { } //make the C entrypoint a no-op when running cargo test +// Missing safety doc OK because this is just a no-op #[cfg(not(target_arch = "bpf"))] #[allow(clippy::missing_safety_doc)] pub unsafe extern "C" fn c_entrypoint(_input: *mut u8) -> u64 {