feat: support zero ci

.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

.github/workflows/docker.yaml

@@ -72,6 +72,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.so
+          asset_name: pyth_oracle_pythnet_no_default_accumulator_v2.so
+          tag: ${{ github.ref }}
+
   pinning:
     runs-on: ubuntu-latest
     steps:

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

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;
     }

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"]

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;
         }
     }
 

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,

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;
 

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,18 +79,23 @@ pub fn add_publisher(
 
     let mut price_data = load_checked::<PriceAccount>(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) {
         price_data
             .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 {

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;

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::<PriceAccount>(&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<Pubkey>) {
+    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::<AddPublisherArgs>(&args)
+    )
+    .is_ok());
+}
+
+fn update_price(accounts: &mut Accounts, price: i64, conf: u64, slot: u64) {
+    let instruction_data = &mut [0u8; size_of::<UpdPriceArgs>()];
+    let mut cmd = load_mut::<UpdPriceArgs>(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::<PriceAccount>(&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::<PriceAccount>(&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::<PriceAccount>(&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::<PriceAccount>(&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::<PriceAccount>(&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::<PriceAccount>(&info, PC_VERSION).unwrap();
+        assert!(!price_data.flags.contains(PriceAccountFlags::ALLOW_ZERO_CI));
+        assert_eq!(price_data.agg_.status_, PC_STATUS_UNKNOWN);
+    }
+}