Instruction counts and optimizations (#9)

* instruction counts

* reduce normalize opcount

* instruction counts

* tests

Co-authored-by: Jayant Krishnamurthy <[email protected]>

Author: jayantk

README.md

@@ -2,7 +2,6 @@
 
 A rust API for describing on-chain pyth account structures.  A primer on pyth accounts can be found at https://github.com/pyth-network/pyth-client/blob/main/doc/aggregate_price.md
 
-
 Contains a library for use in on-chain program development and an off-chain example program for loading and printing product reference data and aggregate prices from all devnet pyth accounts.
 
 ### Running the Example
@@ -38,3 +37,10 @@ product_account .. 6MEwdxe4g1NeAF9u6KDG14anJpFsVEa2cvr5H6iriFZ8
     twap ......... 7426390900
     twac ......... 2259870
 ```
+
+
+### Development
+
+Run `cargo test-bpf` to build in BPF and run the unit tests.
+This command will also build an instruction count program that logs the resource consumption
+of various functions.

src/instruction.rs

@@ -19,6 +19,17 @@ pub enum PythClientInstruction {
     x: PriceConf,
     y: PriceConf,
   },
+  Add {
+    x: PriceConf,
+    y: PriceConf,
+  },
+  ScaleToExponent {
+    x: PriceConf,
+    expo: i32,
+  },
+  Normalize {
+    x: PriceConf,
+  },
   /// Don't do anything for comparison
   ///
   /// No accounts required for this instruction
@@ -45,6 +56,36 @@ pub fn multiply(x: PriceConf, y: PriceConf) -> Instruction {
   }
 }
 
+pub fn add(x: PriceConf, y: PriceConf) -> Instruction {
+  Instruction {
+    program_id: id(),
+    accounts: vec![],
+    data: PythClientInstruction::Add { x, y }
+      .try_to_vec()
+      .unwrap(),
+  }
+}
+
+pub fn scale_to_exponent(x: PriceConf, expo: i32) -> Instruction {
+  Instruction {
+    program_id: id(),
+    accounts: vec![],
+    data: PythClientInstruction::ScaleToExponent { x, expo }
+      .try_to_vec()
+      .unwrap(),
+  }
+}
+
+pub fn normalize(x: PriceConf) -> Instruction {
+  Instruction {
+    program_id: id(),
+    accounts: vec![],
+    data: PythClientInstruction::Normalize { x }
+      .try_to_vec()
+      .unwrap(),
+  }
+}
+
 /// Noop instruction for comparison purposes
 pub fn noop() -> Instruction {
   Instruction {

src/price_conf.rs

@@ -6,8 +6,6 @@ use {
 const PD_EXPO: i32 = -9;
 const PD_SCALE: u64 = 1_000_000_000;
 const MAX_PD_V_U64: u64 = (1 << 28) - 1;
-const MAX_PD_V_I64: i64 = MAX_PD_V_U64 as i64;
-const MIN_PD_V_I64: i64 = -MAX_PD_V_I64;
 
 /**
  * A price with a degree of uncertainty, represented as a price +- a confidence interval.
@@ -81,6 +79,7 @@ impl PriceConf {
     let other_confidence_pct: u64 = (other.conf * PD_SCALE) / other_price;
 
     // first term is 57 bits, second term is 57 + 58 - 29 = 86 bits. Same exponent as the midprice.
+    // Note: the computation of the 2nd term consumes about 3k ops. We may want to optimize this.
     let conf = (((base.conf * PD_SCALE) / other_price) as u128) + ((other_confidence_pct as u128) * (midprice as u128)) / (PD_SCALE as u128);
 
     // Note that this check only fails if an argument's confidence interval was >> its price,
@@ -155,18 +154,19 @@ impl PriceConf {
    * have been normalized to be between `MIN_PD_V_I64` and `MAX_PD_V_I64`.
    */
   pub fn normalize(&self) -> Option<PriceConf> {
-    let mut p = self.price;
+    // signed division is very expensive in op count
+    let (mut p, s) = PriceConf::to_unsigned(self.price);
     let mut c = self.conf;
     let mut e = self.expo;
 
-    while p > MAX_PD_V_I64 || p < MIN_PD_V_I64 || c > MAX_PD_V_U64 {
+    while p > MAX_PD_V_U64 || c > MAX_PD_V_U64 {
       p = p / 10;
       c = c / 10;
       e = e.checked_add(1)?;
     }
 
     Some(PriceConf {
-      price: p,
+      price: (p as i64) * s,
       conf: c,
       expo: e,
     })
@@ -188,7 +188,8 @@ impl PriceConf {
     if delta >= 0 {
       let mut p = self.price;
       let mut c = self.conf;
-      while delta > 0 {
+      // 2nd term is a short-circuit to bound op consumption
+      while delta > 0 && (p != 0 || c != 0) {
         p /= 10;
         c /= 10;
         delta -= 1;
@@ -203,7 +204,8 @@ impl PriceConf {
       let mut p = Some(self.price);
       let mut c = Some(self.conf);
 
-      while delta < 0 {
+      // 2nd & 3rd terms are a short-circuit to bound op consumption
+      while delta < 0 && p.is_some() && c.is_some() {
         p = p?.checked_mul(10);
         c = c?.checked_mul(10);
         delta += 1;
@@ -226,11 +228,10 @@ impl PriceConf {
    * some of the computations above.
    */
   fn to_unsigned(x: i64) -> (u64, i64) {
-    // this check is stricter than necessary. it technically only needs to guard against
-    // i64::MIN, which can't be negated. However, this method should only be used in the context
-    // of normalized numbers.
-    assert!(x <= MAX_PD_V_I64 && x >= MIN_PD_V_I64);
-    if x < 0 {
+    if x == i64::MIN {
+      // special case because i64::MIN == -i64::MIN
+      (i64::MAX as u64 + 1, -1)
+    } else if x < 0 {
       (-x as u64, -1)
     } else {
       (x as u64, 1)
@@ -240,7 +241,10 @@ impl PriceConf {
 
 #[cfg(test)]
 mod test {
-  use crate::price_conf::{MAX_PD_V_U64, MAX_PD_V_I64, MIN_PD_V_I64, PD_EXPO, PD_SCALE, PriceConf};
+  use crate::price_conf::{MAX_PD_V_U64, PD_EXPO, PD_SCALE, PriceConf};
+
+  const MAX_PD_V_I64: i64 = MAX_PD_V_U64 as i64;
+  const MIN_PD_V_I64: i64 = -MAX_PD_V_I64;
 
   fn pc(price: i64, conf: u64, expo: i32) -> PriceConf {
     PriceConf {

src/processor.rs

@@ -4,8 +4,6 @@ use borsh::BorshDeserialize;
 use solana_program::{
   account_info::AccountInfo,
   entrypoint::ProgramResult,
-  log::sol_log_compute_units,
-  msg,
   pubkey::Pubkey,
 };
 
@@ -21,24 +19,26 @@ pub fn process_instruction(
   let instruction = PythClientInstruction::try_from_slice(input).unwrap();
   match instruction {
     PythClientInstruction::Divide { numerator, denominator } => {
-      msg!("Calculating numerator.div(denominator)");
-      sol_log_compute_units();
-      let result = numerator.div(&denominator);
-      sol_log_compute_units();
-      msg!("result: {:?}", result);
+      numerator.div(&denominator);
       Ok(())
     }
     PythClientInstruction::Multiply { x, y } => {
-      msg!("Calculating numerator.mul(denominator)");
-      sol_log_compute_units();
-      let result = x.mul(&y);
-      sol_log_compute_units();
-      msg!("result: {:?}", result);
+      x.mul(&y);
+      Ok(())
+    }
+    PythClientInstruction::Add { x, y } => {
+      x.add(&y);
+      Ok(())
+    }
+    PythClientInstruction::Normalize { x } => {
+      x.normalize();
+      Ok(())
+    }
+    PythClientInstruction::ScaleToExponent { x, expo } => {
+      x.scale_to_exponent(expo);
       Ok(())
     }
     PythClientInstruction::Noop => {
-      msg!("Do nothing");
-      msg!("{}", 0_u64);
       Ok(())
     }
   }

tests/instruction_count.rs

@@ -0,0 +1,106 @@
+use {
+    pyth_client::{id, instruction, PriceConf},
+    pyth_client::processor::process_instruction,
+    solana_program::{
+        instruction::Instruction,
+        pubkey::Pubkey,
+    },
+    solana_program_test::*,
+    solana_sdk::{signature::Signer, transaction::Transaction},
+};
+
+async fn test_instr(instr: Instruction) {
+    let (mut banks_client, payer, recent_blockhash) = ProgramTest::new(
+        "pyth_client",
+        id(),
+        processor!(process_instruction),
+    )
+        .start()
+        .await;
+    let mut transaction = Transaction::new_with_payer(
+        &[instr],
+        Some(&payer.pubkey()),
+    );
+    transaction.sign(&[&payer], recent_blockhash);
+    banks_client.process_transaction(transaction).await.unwrap();
+}
+
+fn pc(price: i64, conf: u64, expo: i32) -> PriceConf {
+    PriceConf {
+        price: price,
+        conf: conf,
+        expo: expo,
+    }
+}
+
+#[tokio::test]
+async fn test_noop() {
+    test_instr(instruction::noop()).await;
+}
+
+#[tokio::test]
+async fn test_scale_to_exponent_down() {
+    test_instr(instruction::scale_to_exponent(pc(1, u64::MAX, -1000), 1000)).await
+}
+
+#[tokio::test]
+async fn test_scale_to_exponent_up() {
+    test_instr(instruction::scale_to_exponent(pc(1, u64::MAX, 1000), -1000)).await
+}
+
+#[tokio::test]
+async fn test_scale_to_exponent_best_case() {
+    test_instr(instruction::scale_to_exponent(pc(1, u64::MAX, 10), 10)).await
+}
+
+#[tokio::test]
+async fn test_normalize_max_conf() {
+    test_instr(instruction::normalize(pc(1, u64::MAX, 0))).await
+}
+
+#[tokio::test]
+async fn test_normalize_max_price() {
+    test_instr(instruction::normalize(pc(i64::MAX, 1, 0))).await
+}
+
+#[tokio::test]
+async fn test_normalize_min_price() {
+    test_instr(instruction::normalize(pc(i64::MIN, 1, 0))).await
+}
+
+#[tokio::test]
+async fn test_normalize_best_case() {
+    test_instr(instruction::normalize(pc(1, 1, 0))).await
+}
+
+#[tokio::test]
+async fn test_div_max_price() {
+    test_instr(instruction::divide(
+        pc(i64::MAX, 1, 0),
+        pc(1, 1, 0)
+    )).await;
+}
+
+#[tokio::test]
+async fn test_div_max_price_2() {
+    test_instr(instruction::divide(
+        pc(i64::MAX, 1, 0),
+        pc(i64::MAX, 1, 0)
+    )).await;
+}
+
+#[tokio::test]
+async fn test_mul_max_price() {
+    test_instr(instruction::multiply(
+        pc(i64::MAX, 1, 2),
+        pc(123, 1, 2),
+    )).await;
+}
+
+#[tokio::test]
+async fn test_mul_max_price_2() {
+    test_instr(instruction::multiply(
+        pc(i64::MAX, 1, 2),
+        pc(i64::MAX, 1, 2),
+    )).await;
+}