From 636c431a8109da535b50cafea4246cf3bcb1590f Mon Sep 17 00:00:00 2001 From: Jayant Krishnamurthy Date: Tue, 28 Dec 2021 10:19:32 -0800 Subject: [PATCH 1/4] instruction counts --- src/instruction.rs | 41 +++++++++++++++++++++++++ src/price_conf.rs | 4 +-- src/processor.rs | 44 ++++++++++++++++++++------- tests/integration.rs | 71 ++++++++++++++++++++++++++++++-------------- 4 files changed, 126 insertions(+), 34 deletions(-) diff --git a/src/instruction.rs b/src/instruction.rs index d04df30..8502c81 100644 --- a/src/instruction.rs +++ b/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 { diff --git a/src/price_conf.rs b/src/price_conf.rs index 87f75ad..a578395 100644 --- a/src/price_conf.rs +++ b/src/price_conf.rs @@ -188,7 +188,7 @@ impl PriceConf { if delta >= 0 { let mut p = self.price; let mut c = self.conf; - while delta > 0 { + while delta > 0 && (p != 0 || c != 0) { p /= 10; c /= 10; delta -= 1; @@ -203,7 +203,7 @@ impl PriceConf { let mut p = Some(self.price); let mut c = Some(self.conf); - while delta < 0 { + while delta < 0 && p.is_some() && c.is_some() { p = p?.checked_mul(10); c = c?.checked_mul(10); delta += 1; diff --git a/src/processor.rs b/src/processor.rs index ad74d38..ea90216 100644 --- a/src/processor.rs +++ b/src/processor.rs @@ -21,24 +21,48 @@ 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(); + // msg!("Calculating numerator.div(denominator)"); + // sol_log_compute_units(); let result = numerator.div(&denominator); - sol_log_compute_units(); - msg!("result: {:?}", result); + // sol_log_compute_units(); + // msg!("result: {:?}", result); Ok(()) } PythClientInstruction::Multiply { x, y } => { - msg!("Calculating numerator.mul(denominator)"); - sol_log_compute_units(); + // msg!("Calculating numerator.mul(denominator)"); + // sol_log_compute_units(); let result = x.mul(&y); - sol_log_compute_units(); - msg!("result: {:?}", result); + // sol_log_compute_units(); + // msg!("result: {:?}", result); + Ok(()) + } + PythClientInstruction::Add { x, y } => { + // msg!("Calculating numerator.mul(denominator)"); + // sol_log_compute_units(); + let result = x.add(&y); + // sol_log_compute_units(); + // msg!("result: {:?}", result); + Ok(()) + } + PythClientInstruction::Normalize { x } => { + // msg!("Calculating numerator.mul(denominator)"); + // sol_log_compute_units(); + let result = x.normalize(); + // sol_log_compute_units(); + // msg!("result: {:?}", result); + Ok(()) + } + PythClientInstruction::ScaleToExponent { x, expo } => { + // msg!("Calculating numerator.mul(denominator)"); + // sol_log_compute_units(); + let result = x.scale_to_exponent(expo); + // sol_log_compute_units(); + // msg!("result: {:?}", result); Ok(()) } PythClientInstruction::Noop => { - msg!("Do nothing"); - msg!("{}", 0_u64); + // msg!("Do nothing"); + // msg!("{}", 0_u64); Ok(()) } } diff --git a/tests/integration.rs b/tests/integration.rs index d5ec2e3..ed2cb3c 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -27,39 +27,66 @@ async fn test_instr(instr: Instruction) { 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_div() { +async fn test_scale_to_exponent_down_worst_case() { + test_instr(instruction::scale_to_exponent(pc(1, u64::MAX, -1000), 1000)).await +} + +#[tokio::test] +async fn test_scale_to_exponent_up_worst_case() { + 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_conf_worst_case() { + test_instr(instruction::normalize(pc(1, u64::MAX, 0))).await +} + +#[tokio::test] +async fn test_normalize_price_worst_case() { + test_instr(instruction::normalize(pc(i64::MAX, 1, 0))).await +} + +#[tokio::test] +async fn test_normalize_price_worst_case2() { + 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_worst_case() { test_instr(instruction::divide( - PriceConf { - price: i64::MAX, - conf: 1, - expo: 0 - }, - PriceConf { - price: 1, - conf: 1, - expo: 0 - } + pc(i64::MAX, 1, 0), + pc(1, 1, 0) )).await; } #[tokio::test] -async fn test_mul() { +async fn test_mul_worst_case() { test_instr(instruction::multiply( - PriceConf { - price: 100, - conf: 1, - expo: 2 - }, - PriceConf { - price: 123, - conf: 1, - expo: -2 - } + pc(i64::MAX, 1, 2), + pc(123, 1, 2), )).await; } From 5791ef6814c35fc169fc56ced7203d09bed615b4 Mon Sep 17 00:00:00 2001 From: Jayant Krishnamurthy Date: Tue, 28 Dec 2021 10:29:47 -0800 Subject: [PATCH 2/4] reduce normalize opcount --- src/price_conf.rs | 16 ++++++++-------- tests/{integration.rs => instruction_count.rs} | 0 2 files changed, 8 insertions(+), 8 deletions(-) rename tests/{integration.rs => instruction_count.rs} (100%) diff --git a/src/price_conf.rs b/src/price_conf.rs index a578395..437c1b9 100644 --- a/src/price_conf.rs +++ b/src/price_conf.rs @@ -155,18 +155,19 @@ impl PriceConf { * have been normalized to be between `MIN_PD_V_I64` and `MAX_PD_V_I64`. */ pub fn normalize(&self) -> Option { - 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, }) @@ -226,11 +227,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) diff --git a/tests/integration.rs b/tests/instruction_count.rs similarity index 100% rename from tests/integration.rs rename to tests/instruction_count.rs From 38ac730680ad95eda786b016961aed9dda6ab114 Mon Sep 17 00:00:00 2001 From: Jayant Krishnamurthy Date: Tue, 28 Dec 2021 10:46:14 -0800 Subject: [PATCH 3/4] instruction counts --- README.md | 8 +++++++- src/price_conf.rs | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 272606f..50b3594 100644 --- a/README.md +++ b/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. diff --git a/src/price_conf.rs b/src/price_conf.rs index 437c1b9..4261ba2 100644 --- a/src/price_conf.rs +++ b/src/price_conf.rs @@ -81,6 +81,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, From 93ad10f563a7698c66ea593b0245df6d5bf9ef2c Mon Sep 17 00:00:00 2001 From: Jayant Krishnamurthy Date: Tue, 28 Dec 2021 10:53:29 -0800 Subject: [PATCH 4/4] tests --- src/price_conf.rs | 9 ++++++--- src/processor.rs | 34 +++++----------------------------- tests/instruction_count.rs | 34 ++++++++++++++++++++++++---------- 3 files changed, 35 insertions(+), 42 deletions(-) diff --git a/src/price_conf.rs b/src/price_conf.rs index 4261ba2..05f09ca 100644 --- a/src/price_conf.rs +++ b/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. @@ -190,6 +188,7 @@ impl PriceConf { if delta >= 0 { let mut p = self.price; let mut c = self.conf; + // 2nd term is a short-circuit to bound op consumption while delta > 0 && (p != 0 || c != 0) { p /= 10; c /= 10; @@ -205,6 +204,7 @@ impl PriceConf { let mut p = Some(self.price); let mut c = Some(self.conf); + // 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); @@ -241,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 { diff --git a/src/processor.rs b/src/processor.rs index ea90216..a135233 100644 --- a/src/processor.rs +++ b/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,48 +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 } => { - // msg!("Calculating numerator.mul(denominator)"); - // sol_log_compute_units(); - let result = x.add(&y); - // sol_log_compute_units(); - // msg!("result: {:?}", result); + x.add(&y); Ok(()) } PythClientInstruction::Normalize { x } => { - // msg!("Calculating numerator.mul(denominator)"); - // sol_log_compute_units(); - let result = x.normalize(); - // sol_log_compute_units(); - // msg!("result: {:?}", result); + x.normalize(); Ok(()) } PythClientInstruction::ScaleToExponent { x, expo } => { - // msg!("Calculating numerator.mul(denominator)"); - // sol_log_compute_units(); - let result = x.scale_to_exponent(expo); - // sol_log_compute_units(); - // msg!("result: {:?}", result); + x.scale_to_exponent(expo); Ok(()) } PythClientInstruction::Noop => { - // msg!("Do nothing"); - // msg!("{}", 0_u64); Ok(()) } } diff --git a/tests/instruction_count.rs b/tests/instruction_count.rs index ed2cb3c..f730264 100644 --- a/tests/instruction_count.rs +++ b/tests/instruction_count.rs @@ -1,14 +1,12 @@ use { - borsh::BorshDeserialize, pyth_client::{id, instruction, PriceConf}, pyth_client::processor::process_instruction, solana_program::{ - instruction::{AccountMeta, Instruction}, + instruction::Instruction, pubkey::Pubkey, }, solana_program_test::*, solana_sdk::{signature::Signer, transaction::Transaction}, - std::str::FromStr, }; async fn test_instr(instr: Instruction) { @@ -41,12 +39,12 @@ async fn test_noop() { } #[tokio::test] -async fn test_scale_to_exponent_down_worst_case() { +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_worst_case() { +async fn test_scale_to_exponent_up() { test_instr(instruction::scale_to_exponent(pc(1, u64::MAX, 1000), -1000)).await } @@ -56,17 +54,17 @@ async fn test_scale_to_exponent_best_case() { } #[tokio::test] -async fn test_normalize_conf_worst_case() { +async fn test_normalize_max_conf() { test_instr(instruction::normalize(pc(1, u64::MAX, 0))).await } #[tokio::test] -async fn test_normalize_price_worst_case() { +async fn test_normalize_max_price() { test_instr(instruction::normalize(pc(i64::MAX, 1, 0))).await } #[tokio::test] -async fn test_normalize_price_worst_case2() { +async fn test_normalize_min_price() { test_instr(instruction::normalize(pc(i64::MIN, 1, 0))).await } @@ -76,7 +74,7 @@ async fn test_normalize_best_case() { } #[tokio::test] -async fn test_div_worst_case() { +async fn test_div_max_price() { test_instr(instruction::divide( pc(i64::MAX, 1, 0), pc(1, 1, 0) @@ -84,9 +82,25 @@ async fn test_div_worst_case() { } #[tokio::test] -async fn test_mul_worst_case() { +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; +}