From 38d68508613ade81252b396c743641e9efffb8bc Mon Sep 17 00:00:00 2001 From: Daniel Chew Date: Thu, 15 Feb 2024 15:11:19 +0900 Subject: [PATCH 01/10] add initial code to support max latency --- program/c/src/oracle/oracle.h | 5 +++-- program/c/src/oracle/upd_aggregate.h | 2 +- program/rust/src/accounts/price.rs | 17 +++++++++++++---- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/program/c/src/oracle/oracle.h b/program/c/src/oracle/oracle.h index 250ce9390..b4e95b534 100644 --- a/program/c/src/oracle/oracle.h +++ b/program/c/src/oracle/oracle.h @@ -195,8 +195,9 @@ typedef struct pc_price pc_ema_t twac_; // time-weighted average conf interval int64_t timestamp_; // unix timestamp of aggregate price uint8_t min_pub_; // min publishers for valid price - int8_t drv2_; // space for future derived values - int16_t drv3_; // space for future derived values + 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 pc_pub_key_t prod_; // product id/ref-account pc_pub_key_t next_; // next price account in list diff --git a/program/c/src/oracle/upd_aggregate.h b/program/c/src/oracle/upd_aggregate.h index 62f895cbb..114d35c90 100644 --- a/program/c/src/oracle/upd_aggregate.h +++ b/program/c/src/oracle/upd_aggregate.h @@ -175,7 +175,7 @@ static inline bool upd_aggregate( pc_price_t *ptr, uint64_t slot, int64_t timest // 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) && - slot_diff >= 0 && slot_diff <= PC_MAX_SEND_LATENCY ) { + slot_diff <= (ptr->max_latency_ ? ptr->max_latency_ : PC_MAX_SEND_LATENCY) ) { numv += 1; prcs[ nprcs++ ] = price - conf; prcs[ nprcs++ ] = price; diff --git a/program/rust/src/accounts/price.rs b/program/rust/src/accounts/price.rs index 5d6c7b497..eb2f48968 100644 --- a/program/rust/src/accounts/price.rs +++ b/program/rust/src/accounts/price.rs @@ -65,7 +65,10 @@ mod price_pythnet { /// Minimum valid publisher quotes for a succesful aggregation pub min_pub_: u8, pub message_sent_: u8, - pub unused_2_: i16, + /// Configurable max latency in slots between send and receive + pub max_latency_: u8, + /// Unused placeholder for alignment + pub unused_2_: i8, pub unused_3_: i32, /// Corresponding product account pub product_account: Pubkey, @@ -116,6 +119,7 @@ mod price_pythnet { self.agg_.price_, self.agg_.conf_, self.agg_.pub_slot_.saturating_sub(self.prev_slot_), + self.max_latency_, ); // pub_slot should always be >= prev_slot, but we protect ourselves against underflow just in case Ok(()) } else { @@ -172,11 +176,13 @@ mod price_pythnet { } impl PriceCumulative { - pub fn update(&mut self, price: i64, conf: u64, slot_gap: u64) { + pub fn update(&mut self, price: i64, conf: u64, slot_gap: u64, max_latency: i16) { self.price += i128::from(price) * i128::from(slot_gap); self.conf += u128::from(conf) * u128::from(slot_gap); + // Use PC_MAX_SEND_LATENCY if max_latency is 0, otherwise use max_latency + let latency = if max_latency == 0 { PC_MAX_SEND_LATENCY.into() } else { max_latency.into() }; // This is expected to saturate at 0 most of the time (while the feed is up). - self.num_down_slots += slot_gap.saturating_sub(PC_MAX_SEND_LATENCY.into()); + self.num_down_slots += slot_gap.saturating_sub(latency as u64); } } } @@ -225,7 +231,10 @@ mod price_solana { /// Whether the current aggregate price has been sent as a message to the message buffer. /// 0 = false, 1 = true. (this is a u8 to make the Pod trait happy) pub message_sent_: u8, - pub unused_2_: i16, + /// Configurable max latency in slots between send and receive + pub max_latency_: u8, + /// Unused placeholder for alignment + pub unused_2_: i8, pub unused_3_: i32, /// Corresponding product account pub product_account: Pubkey, From 796a76283234d36eae8ab41929f006b24ccccac0 Mon Sep 17 00:00:00 2001 From: Daniel Chew Date: Thu, 15 Feb 2024 15:59:23 +0900 Subject: [PATCH 02/10] fix precommit --- program/rust/src/accounts/price.rs | 10 +++-- program/rust/src/tests/test_twap.rs | 67 ++++++++++++++++++++--------- 2 files changed, 53 insertions(+), 24 deletions(-) diff --git a/program/rust/src/accounts/price.rs b/program/rust/src/accounts/price.rs index eb2f48968..f69926b5a 100644 --- a/program/rust/src/accounts/price.rs +++ b/program/rust/src/accounts/price.rs @@ -176,13 +176,17 @@ mod price_pythnet { } impl PriceCumulative { - pub fn update(&mut self, price: i64, conf: u64, slot_gap: u64, max_latency: i16) { + pub fn update(&mut self, price: i64, conf: u64, slot_gap: u64, max_latency: u8) { self.price += i128::from(price) * i128::from(slot_gap); self.conf += u128::from(conf) * u128::from(slot_gap); // Use PC_MAX_SEND_LATENCY if max_latency is 0, otherwise use max_latency - let latency = if max_latency == 0 { PC_MAX_SEND_LATENCY.into() } else { max_latency.into() }; + let latency = if max_latency == 0 { + u64::from(PC_MAX_SEND_LATENCY) + } else { + u64::from(max_latency) + }; // This is expected to saturate at 0 most of the time (while the feed is up). - self.num_down_slots += slot_gap.saturating_sub(latency as u64); + self.num_down_slots += slot_gap.saturating_sub(latency); } } } diff --git a/program/rust/src/tests/test_twap.rs b/program/rust/src/tests/test_twap.rs index d2bc87020..c98fd4654 100644 --- a/program/rust/src/tests/test_twap.rs +++ b/program/rust/src/tests/test_twap.rs @@ -21,18 +21,20 @@ use { #[derive(Clone, Debug, Copy)] pub struct DataEvent { - price: i64, - conf: u64, - slot_gap: u64, + price: i64, + conf: u64, + slot_gap: u64, + max_latency: u8, } impl Arbitrary for DataEvent { fn arbitrary(g: &mut quickcheck::Gen) -> Self { DataEvent { - slot_gap: u64::from(u8::arbitrary(g)) + 1, /* Slot gap is always > 1, because there - * has been a succesful aggregation */ - price: i64::arbitrary(g), - conf: u64::arbitrary(g), + slot_gap: u64::from(u8::arbitrary(g)) + 1, /* Slot gap is always > 1, because there + * has been a succesful aggregation */ + price: i64::arbitrary(g), + conf: u64::arbitrary(g), + max_latency: u8::arbitrary(g), } } } @@ -56,7 +58,12 @@ fn test_twap(input: Vec) -> bool { let mut data = Vec::::new(); for data_event in input { - price_cumulative.update(data_event.price, data_event.conf, data_event.slot_gap); + price_cumulative.update( + data_event.price, + data_event.conf, + data_event.slot_gap, + data_event.max_latency, + ); data.push(data_event); price_cumulative.check_price(data.as_slice()); price_cumulative.check_conf(data.as_slice()); @@ -112,35 +119,53 @@ fn test_twap_unit() { let data = vec![ DataEvent { - price: 1, - conf: 2, - slot_gap: 4, + price: 1, + conf: 2, + slot_gap: 4, + max_latency: 1, }, DataEvent { - price: i64::MAX, - conf: u64::MAX, - slot_gap: 1, + price: i64::MAX, + conf: u64::MAX, + slot_gap: 1, + max_latency: 2, }, DataEvent { - price: -10, - conf: 4, - slot_gap: 30, + price: -10, + conf: 4, + slot_gap: 30, + max_latency: 3, }, ]; - price_cumulative.update(data[0].price, data[0].conf, data[0].slot_gap); + price_cumulative.update( + data[0].price, + data[0].conf, + data[0].slot_gap, + data[0].max_latency, + ); assert_eq!(price_cumulative.price, 5); assert_eq!(price_cumulative.conf, 10); assert_eq!(price_cumulative.num_down_slots, 3); assert_eq!(price_cumulative.unused, 0); - price_cumulative.update(data[1].price, data[1].conf, data[1].slot_gap); + price_cumulative.update( + data[1].price, + data[1].conf, + data[1].slot_gap, + data[1].max_latency, + ); assert_eq!(price_cumulative.price, 9_223_372_036_854_775_812i128); assert_eq!(price_cumulative.conf, 18_446_744_073_709_551_625u128); assert_eq!(price_cumulative.num_down_slots, 3); assert_eq!(price_cumulative.unused, 0); - price_cumulative.update(data[2].price, data[2].conf, data[2].slot_gap); + price_cumulative.update( + data[2].price, + data[2].conf, + data[2].slot_gap, + data[2].max_latency, + ); assert_eq!(price_cumulative.price, 9_223_372_036_854_775_512i128); assert_eq!(price_cumulative.conf, 18_446_744_073_709_551_745u128); assert_eq!(price_cumulative.num_down_slots, 8); @@ -152,7 +177,7 @@ fn test_twap_unit() { num_down_slots: 0, unused: 0, }; - price_cumulative_overflow.update(i64::MIN, u64::MAX, u64::MAX); + price_cumulative_overflow.update(i64::MIN, u64::MAX, u64::MAX, u8::MAX); assert_eq!( price_cumulative_overflow.price, i128::MIN - i128::from(i64::MIN) From 74c63663b381708528b277a76f77b0fab5195070 Mon Sep 17 00:00:00 2001 From: Daniel Chew Date: Fri, 16 Feb 2024 10:31:39 +0900 Subject: [PATCH 03/10] fix tests --- program/rust/src/tests/test_twap.rs | 35 ++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/program/rust/src/tests/test_twap.rs b/program/rust/src/tests/test_twap.rs index c98fd4654..28b09a376 100644 --- a/program/rust/src/tests/test_twap.rs +++ b/program/rust/src/tests/test_twap.rs @@ -19,6 +19,8 @@ use { solana_program::pubkey::Pubkey, }; +pub const TEST_MAX_LATENCY: u8 = 5; + #[derive(Clone, Debug, Copy)] pub struct DataEvent { price: i64, @@ -34,7 +36,7 @@ impl Arbitrary for DataEvent { * has been a succesful aggregation */ price: i64::arbitrary(g), conf: u64::arbitrary(g), - max_latency: u8::arbitrary(g), + max_latency: 0, } } } @@ -94,12 +96,18 @@ impl PriceCumulative { } pub fn check_num_down_slots(&self, data: &[DataEvent]) { assert_eq!( - data.iter() - .fold(0, |acc, x| if x.slot_gap > PC_MAX_SEND_LATENCY.into() { - acc + (x.slot_gap - PC_MAX_SEND_LATENCY as u64) + data.iter().fold(0, |acc, x| { + let latency_threshold = if x.max_latency == 0 { + PC_MAX_SEND_LATENCY.into() + } else { + x.max_latency as u64 + }; + if x.slot_gap > latency_threshold { + acc + (x.slot_gap - latency_threshold) } else { acc - }), + } + }), self.num_down_slots ); } @@ -122,19 +130,19 @@ fn test_twap_unit() { price: 1, conf: 2, slot_gap: 4, - max_latency: 1, + max_latency: TEST_MAX_LATENCY, }, DataEvent { price: i64::MAX, conf: u64::MAX, slot_gap: 1, - max_latency: 2, + max_latency: TEST_MAX_LATENCY, }, DataEvent { price: -10, conf: 4, slot_gap: 30, - max_latency: 3, + max_latency: TEST_MAX_LATENCY, }, ]; @@ -168,7 +176,9 @@ fn test_twap_unit() { ); assert_eq!(price_cumulative.price, 9_223_372_036_854_775_512i128); assert_eq!(price_cumulative.conf, 18_446_744_073_709_551_745u128); - assert_eq!(price_cumulative.num_down_slots, 8); + // self.num_down_slots + (30 - TEST_MAX_LATENCY), using saturating subtraction to avoid negative values + // in this case, the result is 28 because TEST_MAX_LATENCY is 5 and self.num_down_slots is 3 + assert_eq!(price_cumulative.num_down_slots, 28); assert_eq!(price_cumulative.unused, 0); let mut price_cumulative_overflow = PriceCumulative { @@ -188,7 +198,12 @@ fn test_twap_unit() { ); assert_eq!( price_cumulative_overflow.num_down_slots, - u64::MAX - PC_MAX_SEND_LATENCY as u64 + u64::MAX + - if TEST_MAX_LATENCY == 0 { + u64::from(PC_MAX_SEND_LATENCY) + } else { + u64::from(u8::MAX) + } ); assert_eq!(price_cumulative_overflow.unused, 0); } From 79cb50994b3f8114f315c35fc86fe92c54c67a1f Mon Sep 17 00:00:00 2001 From: Daniel Chew Date: Mon, 19 Feb 2024 19:50:28 +0900 Subject: [PATCH 04/10] add more tests --- program/rust/src/tests/test_upd_aggregate.rs | 146 ++++++++++++++++++- 1 file changed, 145 insertions(+), 1 deletion(-) diff --git a/program/rust/src/tests/test_upd_aggregate.rs b/program/rust/src/tests/test_upd_aggregate.rs index c4a8f4977..81401ab01 100644 --- a/program/rust/src/tests/test_upd_aggregate.rs +++ b/program/rust/src/tests/test_upd_aggregate.rs @@ -59,6 +59,22 @@ fn test_upd_aggregate() { corp_act_status_: 0, }; + let p5: PriceInfo = PriceInfo { + price_: 500, + conf_: 50, + status_: PC_STATUS_TRADING, + pub_slot_: 1050, + corp_act_status_: 0, + }; + + let p6: PriceInfo = PriceInfo { + price_: 600, + conf_: 60, + status_: PC_STATUS_TRADING, + pub_slot_: 1074, + corp_act_status_: 0, + }; + let mut instruction_data = [0u8; size_of::()]; populate_instruction(&mut instruction_data, 42, 2, 1); @@ -77,6 +93,7 @@ fn test_upd_aggregate() { price_data.agg_.pub_slot_ = 1000; price_data.comp_[0].latest_ = p1; } + unsafe { assert!(c_upd_aggregate( price_account.try_borrow_mut_data().unwrap().as_mut_ptr(), @@ -134,7 +151,6 @@ fn test_upd_aggregate() { assert_eq!(price_data.prev_timestamp_, 1); } - // three publishers { let mut price_data = load_checked::(&price_account, PC_VERSION).unwrap(); @@ -236,6 +252,7 @@ fn test_upd_aggregate() { 10, )); } + { let price_data = load_checked::(&price_account, PC_VERSION).unwrap(); @@ -269,6 +286,133 @@ fn test_upd_aggregate() { assert_eq!(price_data.prev_conf_, 85); assert_eq!(price_data.prev_timestamp_, 5); } + + // check max_latency_ = 0 defaults to PC_MAX_SEND_LATENCY + { + let mut price_data = load_checked::(&price_account, PC_VERSION).unwrap(); + price_data.num_ = 1; + price_data.agg_.pub_slot_ = 1050; + price_data.comp_[0].latest_ = p5; + } + + unsafe { + assert!(c_upd_aggregate( + price_account.try_borrow_mut_data().unwrap().as_mut_ptr(), + 1051, + 13, + )); + } + + { + let price_data = load_checked::(&price_account, PC_VERSION).unwrap(); + + assert_eq!(price_data.max_latency_, 0); + assert_eq!(price_data.agg_.price_, 500); + assert_eq!(price_data.agg_.conf_, 50); + assert_eq!(price_data.twap_.val_, 177); + assert_eq!(price_data.twac_.val_, 34); + assert_eq!(price_data.num_qt_, 1); + assert_eq!(price_data.timestamp_, 13); + assert_eq!(price_data.prev_slot_, 1025); + assert_eq!(price_data.prev_price_, 245); + assert_eq!(price_data.prev_conf_, 85); + assert_eq!(price_data.prev_timestamp_, 5); + } + + // ensure the update occurs within the PC_MAX_SEND_LATENCY limit of 25 slots, allowing the aggregated price to reflect both p5 and p6 contributions + { + let mut price_data = load_checked::(&price_account, PC_VERSION).unwrap(); + price_data.num_ = 2; + price_data.last_slot_ = 1025; + price_data.agg_.pub_slot_ = 1050; + price_data.comp_[0].latest_ = p5; + price_data.comp_[1].latest_ = p6; + } + + unsafe { + assert!(c_upd_aggregate( + price_account.try_borrow_mut_data().unwrap().as_mut_ptr(), + 1075, + 14, + )); + } + + { + let price_data = load_checked::(&price_account, PC_VERSION).unwrap(); + + assert_eq!(price_data.max_latency_, 0); + assert_eq!(price_data.agg_.price_, 545); + assert_eq!(price_data.agg_.conf_, 55); + assert_eq!(price_data.twap_.val_, 212); + assert_eq!(price_data.twac_.val_, 36); + assert_eq!(price_data.num_qt_, 2); + assert_eq!(price_data.timestamp_, 14); + assert_eq!(price_data.prev_slot_, 1050); + assert_eq!(price_data.prev_price_, 500); + assert_eq!(price_data.prev_conf_, 50); + assert_eq!(price_data.prev_timestamp_, 13); + } + + // verify behavior when publishing halts for 1 slot, causing the slot difference from p5 to exceed the PC_MAX_SEND_LATENCY threshold of 25. + unsafe { + assert!(c_upd_aggregate( + price_account.try_borrow_mut_data().unwrap().as_mut_ptr(), + 1076, + 15, + )); + } + + { + let price_data = load_checked::(&price_account, PC_VERSION).unwrap(); + + assert_eq!(price_data.max_latency_, 0); + assert_eq!(price_data.agg_.price_, 600); + assert_eq!(price_data.agg_.conf_, 60); + assert_eq!(price_data.twap_.val_, 244); + assert_eq!(price_data.twac_.val_, 38); + assert_eq!(price_data.num_qt_, 1); + assert_eq!(price_data.timestamp_, 15); + assert_eq!(price_data.prev_slot_, 1075); + assert_eq!(price_data.prev_price_, 545); + assert_eq!(price_data.prev_conf_, 55); + assert_eq!(price_data.prev_timestamp_, 14); + } + + // verify behavior when max_latency_ is set to 5, this should result in PC_STATUS_UNKNOWN status + { + let mut price_data = load_checked::(&price_account, PC_VERSION).unwrap(); + price_data.max_latency_ = 5; + price_data.num_ = 2; + price_data.last_slot_ = 1025; + price_data.agg_.pub_slot_ = 1050; + price_data.comp_[0].latest_ = p5; + price_data.comp_[1].latest_ = p6; + } + + unsafe { + assert!(!c_upd_aggregate( + price_account.try_borrow_mut_data().unwrap().as_mut_ptr(), + 1080, + 16, + )); + } + + { + let price_data = load_checked::(&price_account, PC_VERSION).unwrap(); + + assert_eq!(price_data.max_latency_, 5); + assert_eq!(price_data.agg_.status_, PC_STATUS_UNKNOWN); + assert_eq!(price_data.agg_.price_, 600); + assert_eq!(price_data.agg_.conf_, 60); + assert_eq!(price_data.twap_.val_, 244); + assert_eq!(price_data.twac_.val_, 38); + assert_eq!(price_data.num_qt_, 0); + assert_eq!(price_data.timestamp_, 16); + assert_eq!(price_data.prev_slot_, 1050); + assert_eq!(price_data.prev_price_, 600); + assert_eq!(price_data.prev_conf_, 60); + assert_eq!(price_data.prev_timestamp_, 15); + } } // Create an upd_price instruction with the provided parameters From a189fbb1438e6d9f401806b69b09d5bc1c836956 Mon Sep 17 00:00:00 2001 From: Daniel Chew Date: Tue, 20 Feb 2024 17:23:31 +0900 Subject: [PATCH 05/10] Refactor test_twap.rs to use random values for max_latency --- program/rust/src/tests/test_twap.rs | 68 +++++++++++++++++++++-------- 1 file changed, 50 insertions(+), 18 deletions(-) diff --git a/program/rust/src/tests/test_twap.rs b/program/rust/src/tests/test_twap.rs index 28b09a376..64f41487a 100644 --- a/program/rust/src/tests/test_twap.rs +++ b/program/rust/src/tests/test_twap.rs @@ -19,8 +19,6 @@ use { solana_program::pubkey::Pubkey, }; -pub const TEST_MAX_LATENCY: u8 = 5; - #[derive(Clone, Debug, Copy)] pub struct DataEvent { price: i64, @@ -36,7 +34,7 @@ impl Arbitrary for DataEvent { * has been a succesful aggregation */ price: i64::arbitrary(g), conf: u64::arbitrary(g), - max_latency: 0, + max_latency: u8::arbitrary(g), } } } @@ -48,6 +46,7 @@ impl Arbitrary for DataEvent { /// - slot_gap is a random number between 1 and u8::MAX + 1 (256) /// - price is a random i64 /// - conf is a random u64 +/// - max_latency is a random u8 #[quickcheck] fn test_twap(input: Vec) -> bool { let mut price_cumulative = PriceCumulative { @@ -76,7 +75,6 @@ fn test_twap(input: Vec) -> bool { true } - impl PriceCumulative { pub fn check_price(&self, data: &[DataEvent]) { assert_eq!( @@ -100,7 +98,7 @@ impl PriceCumulative { let latency_threshold = if x.max_latency == 0 { PC_MAX_SEND_LATENCY.into() } else { - x.max_latency as u64 + x.max_latency.into() }; if x.slot_gap > latency_threshold { acc + (x.slot_gap - latency_threshold) @@ -130,19 +128,31 @@ fn test_twap_unit() { price: 1, conf: 2, slot_gap: 4, - max_latency: TEST_MAX_LATENCY, + max_latency: 0, }, DataEvent { price: i64::MAX, conf: u64::MAX, slot_gap: 1, - max_latency: TEST_MAX_LATENCY, + max_latency: 0, }, DataEvent { price: -10, conf: 4, slot_gap: 30, - max_latency: TEST_MAX_LATENCY, + max_latency: 0, + }, + DataEvent { + price: 1, + conf: 2, + slot_gap: 4, + max_latency: 5, + }, + DataEvent { + price: 6, + conf: 7, + slot_gap: 8, + max_latency: 5, }, ]; @@ -176,9 +186,7 @@ fn test_twap_unit() { ); assert_eq!(price_cumulative.price, 9_223_372_036_854_775_512i128); assert_eq!(price_cumulative.conf, 18_446_744_073_709_551_745u128); - // self.num_down_slots + (30 - TEST_MAX_LATENCY), using saturating subtraction to avoid negative values - // in this case, the result is 28 because TEST_MAX_LATENCY is 5 and self.num_down_slots is 3 - assert_eq!(price_cumulative.num_down_slots, 28); + assert_eq!(price_cumulative.num_down_slots, 8); assert_eq!(price_cumulative.unused, 0); let mut price_cumulative_overflow = PriceCumulative { @@ -198,14 +206,39 @@ fn test_twap_unit() { ); assert_eq!( price_cumulative_overflow.num_down_slots, - u64::MAX - - if TEST_MAX_LATENCY == 0 { - u64::from(PC_MAX_SEND_LATENCY) - } else { - u64::from(u8::MAX) - } + u64::MAX - u64::from(u8::MAX) ); + // u64::MAX - u64::from(u8::MAX) assert_eq!(price_cumulative_overflow.unused, 0); + + let mut price_cumulative_nonzero_max_latency = PriceCumulative { + price: 1, + conf: 2, + num_down_slots: 3, + unused: 0, + }; + + price_cumulative_nonzero_max_latency.update( + data[3].price, + data[3].conf, + data[3].slot_gap, + data[3].max_latency, + ); + assert_eq!(price_cumulative_nonzero_max_latency.price, 5); + assert_eq!(price_cumulative_nonzero_max_latency.conf, 10); + assert_eq!(price_cumulative_nonzero_max_latency.num_down_slots, 3); + assert_eq!(price_cumulative_nonzero_max_latency.unused, 0); + + price_cumulative_nonzero_max_latency.update( + data[4].price, + data[4].conf, + data[4].slot_gap, + data[4].max_latency, + ); + assert_eq!(price_cumulative_nonzero_max_latency.price, 53); + assert_eq!(price_cumulative_nonzero_max_latency.conf, 66); + assert_eq!(price_cumulative_nonzero_max_latency.num_down_slots, 6); + assert_eq!(price_cumulative_nonzero_max_latency.unused, 0); } #[test] @@ -264,7 +297,6 @@ fn test_twap_with_price_account() { Err(OracleError::NeedsSuccesfulAggregation) ); - assert_eq!(price_data.price_cumulative.price, 1 - 2 * 10); assert_eq!(price_data.price_cumulative.conf, 2 + 2 * 5); assert_eq!(price_data.price_cumulative.num_down_slots, 3); From f0b7d91338915edd329c476b7d94c8bf1c6d876e Mon Sep 17 00:00:00 2001 From: Daniel Chew Date: Tue, 20 Feb 2024 18:08:57 +0900 Subject: [PATCH 06/10] Refactor price aggregation test --- program/rust/src/tests/test_upd_aggregate.rs | 150 +++++++++---------- 1 file changed, 72 insertions(+), 78 deletions(-) diff --git a/program/rust/src/tests/test_upd_aggregate.rs b/program/rust/src/tests/test_upd_aggregate.rs index 81401ab01..237b6f63c 100644 --- a/program/rust/src/tests/test_upd_aggregate.rs +++ b/program/rust/src/tests/test_upd_aggregate.rs @@ -59,21 +59,14 @@ fn test_upd_aggregate() { corp_act_status_: 0, }; - let p5: PriceInfo = PriceInfo { + let mut p5: PriceInfo = PriceInfo { price_: 500, conf_: 50, status_: PC_STATUS_TRADING, - pub_slot_: 1050, + pub_slot_: 1024, corp_act_status_: 0, }; - let p6: PriceInfo = PriceInfo { - price_: 600, - conf_: 60, - status_: PC_STATUS_TRADING, - pub_slot_: 1074, - corp_act_status_: 0, - }; let mut instruction_data = [0u8; size_of::()]; populate_instruction(&mut instruction_data, 42, 2, 1); @@ -287,18 +280,52 @@ fn test_upd_aggregate() { assert_eq!(price_data.prev_timestamp_, 5); } - // check max_latency_ = 0 defaults to PC_MAX_SEND_LATENCY + // // check max_latency_ = 0 defaults to PC_MAX_SEND_LATENCY + // { + // let mut price_data = load_checked::(&price_account, PC_VERSION).unwrap(); + // price_data.num_ = 1; + // price_data.agg_.pub_slot_ = 1050; + // price_data.comp_[0].latest_ = p5; + // } + + // unsafe { + // assert!(c_upd_aggregate( + // price_account.try_borrow_mut_data().unwrap().as_mut_ptr(), + // 1051, + // 13, + // )); + // } + + // { + // let price_data = load_checked::(&price_account, PC_VERSION).unwrap(); + + // assert_eq!(price_data.max_latency_, 0); + // assert_eq!(price_data.agg_.price_, 500); + // assert_eq!(price_data.agg_.conf_, 50); + // assert_eq!(price_data.twap_.val_, 177); + // assert_eq!(price_data.twac_.val_, 34); + // assert_eq!(price_data.num_qt_, 1); + // assert_eq!(price_data.timestamp_, 13); + // assert_eq!(price_data.prev_slot_, 1025); + // assert_eq!(price_data.prev_price_, 245); + // assert_eq!(price_data.prev_conf_, 85); + // assert_eq!(price_data.prev_timestamp_, 5); + // } + + // ensure the update occurs within the PC_MAX_SEND_LATENCY limit of 25 slots, allowing the aggregated price to reflect both p4 and p5 contributions { let mut price_data = load_checked::(&price_account, PC_VERSION).unwrap(); - price_data.num_ = 1; - price_data.agg_.pub_slot_ = 1050; - price_data.comp_[0].latest_ = p5; + price_data.num_ = 2; + price_data.last_slot_ = 1000; + price_data.agg_.pub_slot_ = 1000; + price_data.comp_[0].latest_ = p4; + price_data.comp_[1].latest_ = p5; } unsafe { assert!(c_upd_aggregate( price_account.try_borrow_mut_data().unwrap().as_mut_ptr(), - 1051, + 1025, 13, )); } @@ -307,11 +334,11 @@ fn test_upd_aggregate() { let price_data = load_checked::(&price_account, PC_VERSION).unwrap(); assert_eq!(price_data.max_latency_, 0); - assert_eq!(price_data.agg_.price_, 500); - assert_eq!(price_data.agg_.conf_, 50); - assert_eq!(price_data.twap_.val_, 177); - assert_eq!(price_data.twac_.val_, 34); - assert_eq!(price_data.num_qt_, 1); + assert_eq!(price_data.agg_.price_, 445); + assert_eq!(price_data.agg_.conf_, 55); + assert_eq!(price_data.twap_.val_, 168); + assert_eq!(price_data.twac_.val_, 35); + assert_eq!(price_data.num_qt_, 2); assert_eq!(price_data.timestamp_, 13); assert_eq!(price_data.prev_slot_, 1025); assert_eq!(price_data.prev_price_, 245); @@ -319,20 +346,11 @@ fn test_upd_aggregate() { assert_eq!(price_data.prev_timestamp_, 5); } - // ensure the update occurs within the PC_MAX_SEND_LATENCY limit of 25 slots, allowing the aggregated price to reflect both p5 and p6 contributions - { - let mut price_data = load_checked::(&price_account, PC_VERSION).unwrap(); - price_data.num_ = 2; - price_data.last_slot_ = 1025; - price_data.agg_.pub_slot_ = 1050; - price_data.comp_[0].latest_ = p5; - price_data.comp_[1].latest_ = p6; - } - + // verify behavior when publishing halts for 1 slot, causing the slot difference from p5 to exceed the PC_MAX_SEND_LATENCY threshold of 25. unsafe { assert!(c_upd_aggregate( price_account.try_borrow_mut_data().unwrap().as_mut_ptr(), - 1075, + 1026, 14, )); } @@ -341,59 +359,35 @@ fn test_upd_aggregate() { let price_data = load_checked::(&price_account, PC_VERSION).unwrap(); assert_eq!(price_data.max_latency_, 0); - assert_eq!(price_data.agg_.price_, 545); - assert_eq!(price_data.agg_.conf_, 55); - assert_eq!(price_data.twap_.val_, 212); + assert_eq!(price_data.agg_.price_, 500); + assert_eq!(price_data.agg_.conf_, 50); + assert_eq!(price_data.twap_.val_, 203); assert_eq!(price_data.twac_.val_, 36); - assert_eq!(price_data.num_qt_, 2); - assert_eq!(price_data.timestamp_, 14); - assert_eq!(price_data.prev_slot_, 1050); - assert_eq!(price_data.prev_price_, 500); - assert_eq!(price_data.prev_conf_, 50); - assert_eq!(price_data.prev_timestamp_, 13); - } - - // verify behavior when publishing halts for 1 slot, causing the slot difference from p5 to exceed the PC_MAX_SEND_LATENCY threshold of 25. - unsafe { - assert!(c_upd_aggregate( - price_account.try_borrow_mut_data().unwrap().as_mut_ptr(), - 1076, - 15, - )); - } - - { - let price_data = load_checked::(&price_account, PC_VERSION).unwrap(); - - assert_eq!(price_data.max_latency_, 0); - assert_eq!(price_data.agg_.price_, 600); - assert_eq!(price_data.agg_.conf_, 60); - assert_eq!(price_data.twap_.val_, 244); - assert_eq!(price_data.twac_.val_, 38); assert_eq!(price_data.num_qt_, 1); - assert_eq!(price_data.timestamp_, 15); - assert_eq!(price_data.prev_slot_, 1075); - assert_eq!(price_data.prev_price_, 545); + assert_eq!(price_data.timestamp_, 14); + assert_eq!(price_data.prev_slot_, 1025); + assert_eq!(price_data.prev_price_, 445); assert_eq!(price_data.prev_conf_, 55); - assert_eq!(price_data.prev_timestamp_, 14); + assert_eq!(price_data.prev_timestamp_, 13); } - // verify behavior when max_latency_ is set to 5, this should result in PC_STATUS_UNKNOWN status + // verify behavior when max_latency_ is set to 5, and all components pub_slot_ gap is more than 5, this should result in PC_STATUS_UNKNOWN status { let mut price_data = load_checked::(&price_account, PC_VERSION).unwrap(); price_data.max_latency_ = 5; price_data.num_ = 2; - price_data.last_slot_ = 1025; - price_data.agg_.pub_slot_ = 1050; - price_data.comp_[0].latest_ = p5; - price_data.comp_[1].latest_ = p6; + price_data.last_slot_ = 1000; + price_data.agg_.pub_slot_ = 1000; + p5.pub_slot_ = 1004; + price_data.comp_[0].latest_ = p4; + price_data.comp_[1].latest_ = p5; } unsafe { assert!(!c_upd_aggregate( price_account.try_borrow_mut_data().unwrap().as_mut_ptr(), - 1080, - 16, + 1010, + 15, )); } @@ -402,16 +396,16 @@ fn test_upd_aggregate() { assert_eq!(price_data.max_latency_, 5); assert_eq!(price_data.agg_.status_, PC_STATUS_UNKNOWN); - assert_eq!(price_data.agg_.price_, 600); - assert_eq!(price_data.agg_.conf_, 60); - assert_eq!(price_data.twap_.val_, 244); - assert_eq!(price_data.twac_.val_, 38); + assert_eq!(price_data.agg_.price_, 500); + assert_eq!(price_data.agg_.conf_, 50); + assert_eq!(price_data.twap_.val_, 203); + assert_eq!(price_data.twac_.val_, 36); assert_eq!(price_data.num_qt_, 0); - assert_eq!(price_data.timestamp_, 16); - assert_eq!(price_data.prev_slot_, 1050); - assert_eq!(price_data.prev_price_, 600); - assert_eq!(price_data.prev_conf_, 60); - assert_eq!(price_data.prev_timestamp_, 15); + assert_eq!(price_data.timestamp_, 15); + assert_eq!(price_data.prev_slot_, 1000); + assert_eq!(price_data.prev_price_, 500); + assert_eq!(price_data.prev_conf_, 50); + assert_eq!(price_data.prev_timestamp_, 14); } } From f44eecb87d4738c68bc98748544c2212331a95f9 Mon Sep 17 00:00:00 2001 From: Daniel Chew Date: Tue, 20 Feb 2024 22:05:35 +0900 Subject: [PATCH 07/10] address comments --- program/rust/src/tests/test_twap.rs | 1 - program/rust/src/tests/test_upd_aggregate.rs | 38 -------------------- 2 files changed, 39 deletions(-) diff --git a/program/rust/src/tests/test_twap.rs b/program/rust/src/tests/test_twap.rs index 64f41487a..ff3b73790 100644 --- a/program/rust/src/tests/test_twap.rs +++ b/program/rust/src/tests/test_twap.rs @@ -208,7 +208,6 @@ fn test_twap_unit() { price_cumulative_overflow.num_down_slots, u64::MAX - u64::from(u8::MAX) ); - // u64::MAX - u64::from(u8::MAX) assert_eq!(price_cumulative_overflow.unused, 0); let mut price_cumulative_nonzero_max_latency = PriceCumulative { diff --git a/program/rust/src/tests/test_upd_aggregate.rs b/program/rust/src/tests/test_upd_aggregate.rs index 237b6f63c..a2f643fb9 100644 --- a/program/rust/src/tests/test_upd_aggregate.rs +++ b/program/rust/src/tests/test_upd_aggregate.rs @@ -280,38 +280,6 @@ fn test_upd_aggregate() { assert_eq!(price_data.prev_timestamp_, 5); } - // // check max_latency_ = 0 defaults to PC_MAX_SEND_LATENCY - // { - // let mut price_data = load_checked::(&price_account, PC_VERSION).unwrap(); - // price_data.num_ = 1; - // price_data.agg_.pub_slot_ = 1050; - // price_data.comp_[0].latest_ = p5; - // } - - // unsafe { - // assert!(c_upd_aggregate( - // price_account.try_borrow_mut_data().unwrap().as_mut_ptr(), - // 1051, - // 13, - // )); - // } - - // { - // let price_data = load_checked::(&price_account, PC_VERSION).unwrap(); - - // assert_eq!(price_data.max_latency_, 0); - // assert_eq!(price_data.agg_.price_, 500); - // assert_eq!(price_data.agg_.conf_, 50); - // assert_eq!(price_data.twap_.val_, 177); - // assert_eq!(price_data.twac_.val_, 34); - // assert_eq!(price_data.num_qt_, 1); - // assert_eq!(price_data.timestamp_, 13); - // assert_eq!(price_data.prev_slot_, 1025); - // assert_eq!(price_data.prev_price_, 245); - // assert_eq!(price_data.prev_conf_, 85); - // assert_eq!(price_data.prev_timestamp_, 5); - // } - // ensure the update occurs within the PC_MAX_SEND_LATENCY limit of 25 slots, allowing the aggregated price to reflect both p4 and p5 contributions { let mut price_data = load_checked::(&price_account, PC_VERSION).unwrap(); @@ -336,8 +304,6 @@ fn test_upd_aggregate() { assert_eq!(price_data.max_latency_, 0); assert_eq!(price_data.agg_.price_, 445); assert_eq!(price_data.agg_.conf_, 55); - assert_eq!(price_data.twap_.val_, 168); - assert_eq!(price_data.twac_.val_, 35); assert_eq!(price_data.num_qt_, 2); assert_eq!(price_data.timestamp_, 13); assert_eq!(price_data.prev_slot_, 1025); @@ -361,8 +327,6 @@ fn test_upd_aggregate() { assert_eq!(price_data.max_latency_, 0); assert_eq!(price_data.agg_.price_, 500); assert_eq!(price_data.agg_.conf_, 50); - assert_eq!(price_data.twap_.val_, 203); - assert_eq!(price_data.twac_.val_, 36); assert_eq!(price_data.num_qt_, 1); assert_eq!(price_data.timestamp_, 14); assert_eq!(price_data.prev_slot_, 1025); @@ -398,8 +362,6 @@ fn test_upd_aggregate() { assert_eq!(price_data.agg_.status_, PC_STATUS_UNKNOWN); assert_eq!(price_data.agg_.price_, 500); assert_eq!(price_data.agg_.conf_, 50); - assert_eq!(price_data.twap_.val_, 203); - assert_eq!(price_data.twac_.val_, 36); assert_eq!(price_data.num_qt_, 0); assert_eq!(price_data.timestamp_, 15); assert_eq!(price_data.prev_slot_, 1000); From c630caf0d33ff7231e7b40654472bac039ceb9e1 Mon Sep 17 00:00:00 2001 From: Daniel Chew Date: Wed, 21 Feb 2024 12:49:57 +0900 Subject: [PATCH 08/10] bump --- Cargo.lock | 2 +- program/rust/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c9cd9cd10..9ee6e49dc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2506,7 +2506,7 @@ dependencies = [ [[package]] name = "pyth-oracle" -version = "2.25.0" +version = "2.26.0" dependencies = [ "bincode", "bindgen", diff --git a/program/rust/Cargo.toml b/program/rust/Cargo.toml index ef20675aa..2b88bdab9 100644 --- a/program/rust/Cargo.toml +++ b/program/rust/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pyth-oracle" -version = "2.25.0" +version = "2.26.0" edition = "2021" license = "Apache 2.0" publish = false From d4682c643e6296ee38c2d9010c2916c942876fbf Mon Sep 17 00:00:00 2001 From: Daniel Chew Date: Thu, 22 Feb 2024 16:46:40 +0900 Subject: [PATCH 09/10] add comment --- program/c/src/oracle/upd_aggregate.h | 1 + 1 file changed, 1 insertion(+) diff --git a/program/c/src/oracle/upd_aggregate.h b/program/c/src/oracle/upd_aggregate.h index 114d35c90..56fece1f7 100644 --- a/program/c/src/oracle/upd_aggregate.h +++ b/program/c/src/oracle/upd_aggregate.h @@ -175,6 +175,7 @@ static inline bool upd_aggregate( pc_price_t *ptr, uint64_t slot, int64_t timest // 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) && + // 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 <= (ptr->max_latency_ ? ptr->max_latency_ : PC_MAX_SEND_LATENCY) ) { numv += 1; prcs[ nprcs++ ] = price - conf; From 6ed3cb083e568edd3073f2775386f897b9f31c1f Mon Sep 17 00:00:00 2001 From: Daniel Chew Date: Thu, 22 Feb 2024 16:48:16 +0900 Subject: [PATCH 10/10] refactor --- program/c/src/oracle/upd_aggregate.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/program/c/src/oracle/upd_aggregate.h b/program/c/src/oracle/upd_aggregate.h index 56fece1f7..9668a073a 100644 --- a/program/c/src/oracle/upd_aggregate.h +++ b/program/c/src/oracle/upd_aggregate.h @@ -171,12 +171,13 @@ static inline bool upd_aggregate( pc_price_t *ptr, uint64_t slot, int64_t timest int64_t slot_diff = ( int64_t )slot - ( int64_t )( iptr->agg_.pub_slot_ ); int64_t price = iptr->agg_.price_; 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) && // 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 <= (ptr->max_latency_ ? ptr->max_latency_ : PC_MAX_SEND_LATENCY) ) { + slot_diff <= max_latency ) { numv += 1; prcs[ nprcs++ ] = price - conf; prcs[ nprcs++ ] = price;