diff --git a/.github/workflows/check-fomatting.yml b/.github/workflows/check-fomatting.yml index 2a2cdeaa..f0c55509 100644 --- a/.github/workflows/check-fomatting.yml +++ b/.github/workflows/check-fomatting.yml @@ -16,4 +16,4 @@ jobs: profile: minimal toolchain: nightly-2023-03-01 components: rustfmt, clippy - - uses: pre-commit/action@v2.0.3 + - uses: pre-commit/action@v3.0.0 diff --git a/.github/workflows/docker.yaml b/.github/workflows/docker.yaml index d05dac42..3fda859b 100644 --- a/.github/workflows/docker.yaml +++ b/.github/workflows/docker.yaml @@ -61,8 +61,9 @@ jobs: run : | docker create -ti --name container "${DOCKER_IMAGE}" bash docker cp container:/home/pyth/pyth-client/target/pyth/pythnet/pyth_oracle_pythnet.so . + docker cp container:/home/pyth/pyth-client/target/pyth/pythnet/pyth_oracle_pythnet_no_accumulator_v2.so . docker rm -f container - + - name : Publish Pythnet binary if : env.IS_ORACLE_RELEASE == 'true' uses: svenstaro/upload-release-action@133984371c30d34e38222a64855679a414cb7575 @@ -72,6 +73,15 @@ jobs: asset_name: pyth_oracle_pythnet.so tag: ${{ github.ref }} + - name : Publish Pythnet No Default Accumulator v2 binary + if : env.IS_ORACLE_RELEASE == 'true' + uses: svenstaro/upload-release-action@133984371c30d34e38222a64855679a414cb7575 + with: + repo_token: ${{ secrets.GITHUB_TOKEN }} + file: ./pyth_oracle_pythnet_no_accumulator_v2.so + asset_name: pyth_oracle_pythnet_no_accumulator_v2.so + tag: ${{ github.ref }} + pinning: runs-on: ubuntu-latest steps: diff --git a/Cargo.lock b/Cargo.lock index f5e30104..4c0ab946 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2578,7 +2578,7 @@ dependencies = [ [[package]] name = "pyth-oracle" -version = "2.34.0" +version = "2.35.0" dependencies = [ "bincode", "bindgen", diff --git a/program/c/src/oracle/oracle.h b/program/c/src/oracle/oracle.h index e7240df9..8b641d84 100644 --- a/program/c/src/oracle/oracle.h +++ b/program/c/src/oracle/oracle.h @@ -193,8 +193,9 @@ typedef struct pc_price uint8_t min_pub_; // min publishers for valid price int8_t message_sent_; // flag to indicate if the current aggregate price has been sent as a message to the message buffer, 0 if not sent, 1 if sent uint8_t max_latency_; // configurable max latency in slots between send and receive - int8_t drv3_; // space for future derived values - int32_t drv4_; // space for future derived values + uint8_t flags; // Various bit flags. See PriceAccountFlags rust struct for more details. + // 0: ACCUMULATOR_V2, 1: MESSAGE_BUFFER_CLEARED 2: ALLOW_ZERO_CI + uint32_t feed_index; // Globally unique feed index for this price feed pc_pub_key_t prod_; // product id/ref-account pc_pub_key_t next_; // next price account in list uint64_t prev_slot_; // valid slot of previous aggregate with TRADING status diff --git a/program/c/src/oracle/upd_aggregate.h b/program/c/src/oracle/upd_aggregate.h index 21ae8e45..07e7c85d 100644 --- a/program/c/src/oracle/upd_aggregate.h +++ b/program/c/src/oracle/upd_aggregate.h @@ -155,6 +155,8 @@ static inline bool upd_aggregate( pc_price_t *ptr, uint64_t slot, int64_t timest uint32_t numv = 0; uint32_t nprcs = (uint32_t)0; int64_t prcs[ PC_NUM_COMP * 3 ]; // ~0.75KiB for current PC_NUM_COMP (FIXME: DOUBLE CHECK THIS FITS INTO STACK FRAME LIMIT) + bool allow_zero_ci = (ptr->flags & 0x4) != 0; + for ( uint32_t i = 0; i != ptr->num_; ++i ) { pc_price_comp_t *iptr = &ptr->comp_[i]; // copy contributing price to aggregate snapshot @@ -165,9 +167,10 @@ static inline bool upd_aggregate( pc_price_t *ptr, uint64_t slot, int64_t timest int64_t conf = ( int64_t )( iptr->agg_.conf_ ); int64_t max_latency = ptr->max_latency_ ? ptr->max_latency_ : PC_MAX_SEND_LATENCY; if ( iptr->agg_.status_ == PC_STATUS_TRADING && - // No overflow for INT64_MIN+conf or INT64_MAX-conf as 0 < conf < INT64_MAX - // These checks ensure that price - conf and price + conf do not overflow. - (int64_t)0 < conf && (INT64_MIN + conf) <= price && price <= (INT64_MAX-conf) && + // Only accept confidence of zero if the flag is set + (allow_zero_ci || conf > 0) && + // these checks ensure that price - conf and price + conf do not overflow. + (INT64_MIN + conf) <= price && price <= (INT64_MAX-conf) && // slot_diff is implicitly >= 0 due to the check in Rust code ensuring publishing_slot is always less than or equal to the current slot. slot_diff <= max_latency ) { numv += 1; @@ -201,10 +204,9 @@ static inline bool upd_aggregate( pc_price_t *ptr, uint64_t slot, int64_t timest // use the larger of the left and right confidences agg_conf = agg_conf_right > agg_conf_left ? agg_conf_right : agg_conf_left; - // if the confidences end up at zero, we abort - // this is paranoia as it is currently not possible when nprcs>2 and - // positive confidences given the current pricing model - if( agg_conf <= (int64_t)0 ) { + // when zero CI is not allowed, the confidence should not be zero. + // and this check is not necessary, but we do it anyway to be safe. + if( (!allow_zero_ci) && agg_conf <= (int64_t)0 ) { ptr->agg_.status_ = PC_STATUS_UNKNOWN; return false; } diff --git a/program/rust/Cargo.toml b/program/rust/Cargo.toml index bde90559..f93ccee8 100644 --- a/program/rust/Cargo.toml +++ b/program/rust/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pyth-oracle" -version = "2.34.0" +version = "2.35.0" edition = "2021" license = "Apache 2.0" publish = false @@ -45,6 +45,7 @@ time = "=0.3.7" check = [] # Skips make build in build.rs, use with cargo-clippy and cargo-check debug = [] library = ["solana-sdk"] +no-default-accumulator-v2 = [] [lib] crate-type = ["cdylib", "lib"] diff --git a/program/rust/src/accounts/price.rs b/program/rust/src/accounts/price.rs index fc207539..e8745998 100644 --- a/program/rust/src/accounts/price.rs +++ b/program/rust/src/accounts/price.rs @@ -108,6 +108,8 @@ mod price_pythnet { /// If unset, the program will remove old messages from its message buffer account /// and set this flag. const MESSAGE_BUFFER_CLEARED = 0b10; + /// If set, the program allows publishing of zero confidence interval updates. + const ALLOW_ZERO_CI = 0b100; } } diff --git a/program/rust/src/processor.rs b/program/rust/src/processor.rs index 80ee1740..325df8e3 100644 --- a/program/rust/src/processor.rs +++ b/program/rust/src/processor.rs @@ -41,8 +41,10 @@ mod upd_product; #[cfg(test)] pub use add_publisher::{ + ALLOW_ZERO_CI, DISABLE_ACCUMULATOR_V2, ENABLE_ACCUMULATOR_V2, + FORBID_ZERO_CI, }; use solana_program::{ program_error::ProgramError, diff --git a/program/rust/src/processor/add_price.rs b/program/rust/src/processor/add_price.rs index 5d8d14db..9e8bd99b 100644 --- a/program/rust/src/processor/add_price.rs +++ b/program/rust/src/processor/add_price.rs @@ -84,9 +84,12 @@ pub fn add_price( price_data.next_price_account = product_data.first_price_account; price_data.min_pub_ = PRICE_ACCOUNT_DEFAULT_MIN_PUB; price_data.feed_index = reserve_new_price_feed_index(permissions_account)?; - price_data - .flags - .insert(PriceAccountFlags::ACCUMULATOR_V2 | PriceAccountFlags::MESSAGE_BUFFER_CLEARED); + + if !cfg!(feature = "no-default-accumulator-v2") { + price_data + .flags + .insert(PriceAccountFlags::ACCUMULATOR_V2 | PriceAccountFlags::MESSAGE_BUFFER_CLEARED); + } product_data.first_price_account = *price_account.key; diff --git a/program/rust/src/processor/add_publisher.rs b/program/rust/src/processor/add_publisher.rs index 99601525..95b26b1f 100644 --- a/program/rust/src/processor/add_publisher.rs +++ b/program/rust/src/processor/add_publisher.rs @@ -40,6 +40,12 @@ pub const ENABLE_ACCUMULATOR_V2: [u8; 32] = [ pub const DISABLE_ACCUMULATOR_V2: [u8; 32] = [ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, ]; +pub const ALLOW_ZERO_CI: [u8; 32] = [ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, +]; +pub const FORBID_ZERO_CI: [u8; 32] = [ + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, +]; /// Add publisher to symbol account // account[0] funding account [signer writable] @@ -73,11 +79,10 @@ pub fn add_publisher( let mut price_data = load_checked::(price_account, cmd_args.header.version)?; + // Hack: we use add_publisher instruction to configure the price feeds for some operations. + // This is mostly because we are constrained on contract size and can't add separate + // instructions for these operations. if cmd_args.publisher == Pubkey::from(ENABLE_ACCUMULATOR_V2) { - // Hack: we use add_publisher instruction to configure the `ACCUMULATOR_V2` flag. Using a new - // instruction would be cleaner but it would require more work in the tooling. - // These special cases can be removed along with the v1 aggregation code once the transition - // is complete. price_data.flags.insert(PriceAccountFlags::ACCUMULATOR_V2); return Ok(()); } else if cmd_args.publisher == Pubkey::from(DISABLE_ACCUMULATOR_V2) { @@ -85,6 +90,12 @@ pub fn add_publisher( .flags .remove(PriceAccountFlags::ACCUMULATOR_V2 | PriceAccountFlags::MESSAGE_BUFFER_CLEARED); return Ok(()); + } else if cmd_args.publisher == Pubkey::from(ALLOW_ZERO_CI) { + price_data.flags.insert(PriceAccountFlags::ALLOW_ZERO_CI); + return Ok(()); + } else if cmd_args.publisher == Pubkey::from(FORBID_ZERO_CI) { + price_data.flags.remove(PriceAccountFlags::ALLOW_ZERO_CI); + return Ok(()); } if price_data.num_ >= PC_NUM_COMP { diff --git a/program/rust/src/tests/mod.rs b/program/rust/src/tests/mod.rs index 789d7843..3095ba68 100644 --- a/program/rust/src/tests/mod.rs +++ b/program/rust/src/tests/mod.rs @@ -4,6 +4,7 @@ mod test_add_product; mod test_add_publisher; mod test_aggregate_v2; mod test_aggregation; +mod test_aggregation_zero_conf; mod test_c_code; mod test_check_valid_signable_account_or_permissioned_funding_account; mod test_del_price; diff --git a/program/rust/src/tests/test_aggregation_zero_conf.rs b/program/rust/src/tests/test_aggregation_zero_conf.rs new file mode 100644 index 00000000..7c4e21b8 --- /dev/null +++ b/program/rust/src/tests/test_aggregation_zero_conf.rs @@ -0,0 +1,206 @@ +use { + crate::{ + accounts::{ + PermissionAccount, + PriceAccount, + PriceAccountFlags, + PythAccount, + }, + c_oracle_header::{ + PC_STATUS_TRADING, + PC_STATUS_UNKNOWN, + PC_VERSION, + }, + deserialize::{ + load_checked, + load_mut, + }, + instruction::{ + AddPublisherArgs, + OracleCommand, + UpdPriceArgs, + }, + processor::{ + process_instruction, + ALLOW_ZERO_CI, + FORBID_ZERO_CI, + }, + tests::test_utils::{ + update_clock_slot, + AccountSetup, + }, + }, + bytemuck::bytes_of, + solana_program::pubkey::Pubkey, + std::mem::size_of, +}; + +struct Accounts { + program_id: Pubkey, + publisher_account: AccountSetup, + funding_account: AccountSetup, + price_account: AccountSetup, + permissions_account: AccountSetup, + clock_account: AccountSetup, +} + +impl Accounts { + fn new() -> Self { + let program_id = Pubkey::new_unique(); + let publisher_account = AccountSetup::new_funding(); + let clock_account = AccountSetup::new_clock(); + let mut funding_account = AccountSetup::new_funding(); + let mut permissions_account = AccountSetup::new_permission(&program_id); + let mut price_account = AccountSetup::new::(&program_id); + + PriceAccount::initialize(&price_account.as_account_info(), PC_VERSION).unwrap(); + + { + let permissions_account_info = permissions_account.as_account_info(); + let mut permissions_account_data = + PermissionAccount::initialize(&permissions_account_info, PC_VERSION).unwrap(); + permissions_account_data.master_authority = *funding_account.as_account_info().key; + permissions_account_data.data_curation_authority = + *funding_account.as_account_info().key; + permissions_account_data.security_authority = *funding_account.as_account_info().key; + } + + Self { + program_id, + publisher_account, + funding_account, + price_account, + permissions_account, + clock_account, + } + } +} + +fn add_publisher(accounts: &mut Accounts, publisher: Option) { + let args = AddPublisherArgs { + header: OracleCommand::AddPublisher.into(), + publisher: publisher.unwrap_or(*accounts.publisher_account.as_account_info().key), + }; + + assert!(process_instruction( + &accounts.program_id, + &[ + accounts.funding_account.as_account_info(), + accounts.price_account.as_account_info(), + accounts.permissions_account.as_account_info(), + ], + bytes_of::(&args) + ) + .is_ok()); +} + +fn update_price(accounts: &mut Accounts, price: i64, conf: u64, slot: u64) { + let instruction_data = &mut [0u8; size_of::()]; + let mut cmd = load_mut::(instruction_data).unwrap(); + cmd.header = OracleCommand::UpdPrice.into(); + cmd.status = PC_STATUS_TRADING; + cmd.price = price; + cmd.confidence = conf; + cmd.publishing_slot = slot; + cmd.unused_ = 0; + + let mut clock = accounts.clock_account.as_account_info(); + clock.is_signer = false; + clock.is_writable = false; + + process_instruction( + &accounts.program_id, + &[ + accounts.publisher_account.as_account_info(), + accounts.price_account.as_account_info(), + clock, + ], + instruction_data, + ) + .unwrap(); +} + +#[test] +fn test_aggregate_v2_toggle() { + let accounts = &mut Accounts::new(); + + // Add an initial Publisher to test with. + add_publisher(accounts, None); + + // Update the price, no aggregation will happen on the first slot. + { + update_clock_slot(&mut accounts.clock_account.as_account_info(), 1); + update_price(accounts, 42, 2, 1); + let info = accounts.price_account.as_account_info(); + let price_data = load_checked::(&info, PC_VERSION).unwrap(); + assert_eq!(price_data.last_slot_, 0); + assert!(!price_data.flags.contains(PriceAccountFlags::ACCUMULATOR_V2)); + } + + // Update again, component is now TRADING so aggregation should trigger. + { + update_clock_slot(&mut accounts.clock_account.as_account_info(), 2); + update_price(accounts, 43, 3, 2); + let info = accounts.price_account.as_account_info(); + let price_data = load_checked::(&info, PC_VERSION).unwrap(); + assert_eq!(price_data.agg_.status_, PC_STATUS_TRADING); + assert_eq!(price_data.last_slot_, 2); + assert_eq!(price_data.agg_.price_, 42); + assert_eq!(price_data.agg_.conf_, 2); + } + + // Update again, but with confidence set to 0, it should not aggregate *in the next slot*. + { + update_clock_slot(&mut accounts.clock_account.as_account_info(), 3); + update_price(accounts, 44, 0, 3); + let info = accounts.price_account.as_account_info(); + let price_data = load_checked::(&info, PC_VERSION).unwrap(); + assert_eq!(price_data.agg_.status_, PC_STATUS_TRADING); + assert_eq!(price_data.last_slot_, 3); + assert_eq!(price_data.agg_.price_, 43); + assert_eq!(price_data.agg_.conf_, 3); + } + + // Update again, to trigger aggregation. We should see status go to unknown + { + update_clock_slot(&mut accounts.clock_account.as_account_info(), 4); + update_price(accounts, 45, 0, 4); + let info = accounts.price_account.as_account_info(); + let price_data = load_checked::(&info, PC_VERSION).unwrap(); + println!("Price Data: {:?}", price_data.agg_); + assert_eq!(price_data.agg_.status_, PC_STATUS_UNKNOWN); + assert_eq!(price_data.last_slot_, 3); + } + + // Enable allow zero confidence bit + add_publisher(accounts, Some(ALLOW_ZERO_CI.into())); + + // Update again, with allow zero confidence bit set, aggregation should support + // zero confidence values. Note that we don't need to do this twice, because the + // price with ci zero was already stored in the previous slot. + { + update_clock_slot(&mut accounts.clock_account.as_account_info(), 5); + update_price(accounts, 46, 0, 5); + let info = accounts.price_account.as_account_info(); + let price_data = load_checked::(&info, PC_VERSION).unwrap(); + assert!(price_data.flags.contains(PriceAccountFlags::ALLOW_ZERO_CI)); + assert_eq!(price_data.agg_.status_, PC_STATUS_TRADING); + assert_eq!(price_data.last_slot_, 5); + assert_eq!(price_data.agg_.price_, 45); + assert_eq!(price_data.agg_.conf_, 0); + } + + // Disable allow zero confidence bit + add_publisher(accounts, Some(FORBID_ZERO_CI.into())); + + // Update again, with forbid zero confidence bit set, aggregation should have status + // of unknown + { + update_clock_slot(&mut accounts.clock_account.as_account_info(), 6); + update_price(accounts, 47, 0, 6); + let info = accounts.price_account.as_account_info(); + let price_data = load_checked::(&info, PC_VERSION).unwrap(); + assert!(!price_data.flags.contains(PriceAccountFlags::ALLOW_ZERO_CI)); + assert_eq!(price_data.agg_.status_, PC_STATUS_UNKNOWN); + } +} diff --git a/program/rust/src/tests/test_full_publisher_set.rs b/program/rust/src/tests/test_full_publisher_set.rs index 5f294bd4..cf211688 100644 --- a/program/rust/src/tests/test_full_publisher_set.rs +++ b/program/rust/src/tests/test_full_publisher_set.rs @@ -79,8 +79,13 @@ async fn test_full_publisher_set() -> Result<(), Box> { .await .unwrap(); - assert_eq!(price_data.agg_.price_, 0); - assert_eq!(price_data.agg_.conf_, 0); + if cfg!(feature = "no-default-accumulator-v2") { + assert_eq!(price_data.agg_.price_, 110); + assert_eq!(price_data.agg_.conf_, 20); + } else { + assert_eq!(price_data.agg_.price_, 0); + assert_eq!(price_data.agg_.conf_, 0); + } } Ok(()) diff --git a/program/rust/src/tests/test_publish.rs b/program/rust/src/tests/test_publish.rs index 2b6b2b87..0b38ac13 100644 --- a/program/rust/src/tests/test_publish.rs +++ b/program/rust/src/tests/test_publish.rs @@ -102,8 +102,14 @@ async fn test_publish() { assert_eq!(price_data.comp_[0].latest_.conf_, 0); assert_eq!(price_data.comp_[0].latest_.status_, PC_STATUS_UNKNOWN); - assert_eq!(price_data.comp_[0].agg_.price_, 0); - assert_eq!(price_data.comp_[0].agg_.conf_, 0); - assert_eq!(price_data.comp_[0].agg_.status_, PC_STATUS_UNKNOWN); + if cfg!(feature = "no-default-accumulator-v2") { + assert_eq!(price_data.comp_[0].agg_.price_, 150); + assert_eq!(price_data.comp_[0].agg_.conf_, 7); + assert_eq!(price_data.comp_[0].agg_.status_, PC_STATUS_TRADING); + } else { + assert_eq!(price_data.comp_[0].agg_.price_, 0); + assert_eq!(price_data.comp_[0].agg_.conf_, 0); + assert_eq!(price_data.comp_[0].agg_.status_, PC_STATUS_UNKNOWN); + } } } diff --git a/program/rust/src/tests/test_publish_batch.rs b/program/rust/src/tests/test_publish_batch.rs index 245d4e0b..d284c7d4 100644 --- a/program/rust/src/tests/test_publish_batch.rs +++ b/program/rust/src/tests/test_publish_batch.rs @@ -111,7 +111,7 @@ async fn test_publish_batch() { .get_account_data_as::(*price) .await .unwrap(); - let _quote = quotes.get(key).unwrap(); + let quote = quotes.get(key).unwrap(); let new_quote = new_quotes.get(key).unwrap(); assert_eq!(price_data.comp_[0].latest_.price_, new_quote.price); @@ -125,8 +125,18 @@ async fn test_publish_batch() { ) .unwrap() ); - assert_eq!(price_data.comp_[0].agg_.price_, 0); - assert_eq!(price_data.comp_[0].agg_.conf_, 0); - assert_eq!(price_data.comp_[0].agg_.status_, PC_STATUS_UNKNOWN); + if cfg!(feature = "no-default-accumulator-v2") { + assert_eq!(price_data.comp_[0].agg_.price_, quote.price); + assert_eq!(price_data.comp_[0].agg_.conf_, quote.confidence); + assert_eq!( + price_data.comp_[0].agg_.status_, + get_status_for_conf_price_ratio(quote.price, quote.confidence, quote.status) + .unwrap() + ); + } else { + assert_eq!(price_data.comp_[0].agg_.price_, 0); + assert_eq!(price_data.comp_[0].agg_.conf_, 0); + assert_eq!(price_data.comp_[0].agg_.status_, PC_STATUS_UNKNOWN); + } } } diff --git a/scripts/build-bpf.sh b/scripts/build-bpf.sh index fd85e4fa..9c1bc35b 100755 --- a/scripts/build-bpf.sh +++ b/scripts/build-bpf.sh @@ -19,12 +19,19 @@ set -x # Build both parts of the program (both C and rust) via Cargo cd "${PYTH_DIR}" -# Re-run tests affected by features cargo-test-bpf - cargo-build-bpf -- --locked -Z build-std=std,panic_abort -Z build-std-features=panic_immediate_abort sha256sum ./target/**/*.so echo "Checking size of pyth_oracle.so for pythnet" ./scripts/check-size.sh 88429 mkdir -p target/pyth/pythnet/ mv target/deploy/pyth_oracle.so target/pyth/pythnet/pyth_oracle_pythnet.so + +# Re-run tests affected by features +cargo-build-bpf -- --locked -Z build-std=std,panic_abort -Z build-std-features=panic_immediate_abort --features no-default-accumulator-v2 +cargo test --locked --features no-default-accumulator-v2 +sha256sum ./target/**/*.so +echo "Checking size of pyth_oracle.so for pythnet with no accumulator" +./scripts/check-size.sh 88429 +mkdir -p target/pyth/pythnet/ +mv target/deploy/pyth_oracle.so target/pyth/pythnet/pyth_oracle_pythnet_no_accumulator_v2.so