Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions program/rust/src/processor/upd_price.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use {
utils::{
check_valid_funding_account,
check_valid_writable_account,
get_status_for_update,
get_status_for_conf_price_ratio,
is_component_update,
pyth_assert,
try_convert,
Expand Down Expand Up @@ -271,8 +271,11 @@ pub fn upd_price(

// Try to update the publisher's price
if is_component_update(cmd_args)? {
// IMPORTANT: If the publisher does not meet the price/conf
// ratio condition, its price will not count for the next
// aggregate.
let status: u32 =
get_status_for_update(cmd_args.price, cmd_args.confidence, cmd_args.status)?;
get_status_for_conf_price_ratio(cmd_args.price, cmd_args.confidence, cmd_args.status)?;

{
let publisher_price = &mut price_data.comp_[publisher_index].latest_;
Expand Down
1 change: 1 addition & 0 deletions program/rust/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ mod test_del_price;
mod test_del_product;
mod test_del_publisher;
mod test_ema;
mod test_full_publisher_set;
mod test_init_mapping;
mod test_init_price;
mod test_message;
Expand Down
18 changes: 12 additions & 6 deletions program/rust/src/tests/pyth_simulator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ impl PythSimulator {

for (key, price_account) in price_accounts {
let cmd = UpdPriceArgs {
header: OracleCommand::UpdPriceNoFailOnError.into(),
header: OracleCommand::UpdPrice.into(),
status: quotes[key].status,
unused_: 0,
price: quotes[key].price,
Expand Down Expand Up @@ -536,15 +536,17 @@ impl PythSimulator {
/// TODO : this fixture doesn't set the product metadata
pub async fn setup_product_fixture(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: comment should say publishers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this comment is correct. there's stuff like symbol name in the test case assets, but we only make it available for lookup to distinguish symbols human-readably, without filling in the relevant field in the metadata k/v/ store.

&mut self,
publisher: Pubkey,
publishers: &[Pubkey],
security_authority: Pubkey,
) -> HashMap<String, Pubkey> {
let result_file =
File::open("./test_data/publish/products.json").expect("Test file not found");

self.airdrop(&publisher, 100 * LAMPORTS_PER_SOL)
.await
.unwrap();
for publisher in publishers {
self.airdrop(publisher, 100 * LAMPORTS_PER_SOL)
.await
.unwrap();
}

self.airdrop(&security_authority, 100 * LAMPORTS_PER_SOL)
.await
Expand All @@ -570,7 +572,11 @@ impl PythSimulator {
for symbol in product_metadatas.keys() {
let product_keypair = self.add_product(&mapping_keypair).await.unwrap();
let price_keypair = self.add_price(&product_keypair, -5).await.unwrap();
self.add_publisher(&price_keypair, publisher).await.unwrap();
for publisher in publishers.iter() {
self.add_publisher(&price_keypair, *publisher)
.await
.unwrap();
}
price_accounts.insert(symbol.to_string(), price_keypair.pubkey());
}
price_accounts
Expand Down
87 changes: 87 additions & 0 deletions program/rust/src/tests/test_full_publisher_set.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
use {
crate::{
accounts::PriceAccount,
c_oracle_header::{
PC_NUM_COMP,
PC_STATUS_TRADING,
},
tests::pyth_simulator::{
PythSimulator,
Quote,
},
},
solana_sdk::{
signature::Keypair,
signer::Signer,
},
};

// Verify that the whole publisher set participates in aggregate
// calculation. This is important for verifying that extra
// publisher slots on Pythnet are working. Here's how this works:
//
// * Fill all publisher slots on a price
// * Divide the price component array into two even halves: first_half, second_half
// * Publish two distinct price values to either half
// * Verify that the aggregate averages out to an expected value in the middle
#[tokio::test]
async fn test_full_publisher_set() -> Result<(), Box<dyn std::error::Error>> {
let mut sim = PythSimulator::new().await;
let pub_keypairs: Vec<_> = (0..PC_NUM_COMP).map(|_idx| Keypair::new()).collect();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you sure this constant has the right value on each network? (i think you may have added a separate test toggled by the feature flag that it does have the expected value, but i forget)

Copy link
Contributor Author

@drozdziak1 drozdziak1 Nov 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a couple potential problems to discuss:

  • The size is not computed right - we already explicitly verify in test_sizes.rs that Pythnet has 64 and Solana has 32 publishers.
  • In the horrifying case that there's still a mismatch between what the C and Rust code is actually using - it should become evident as part of this test:
    ** If C code is using 64 and Rust is giving it 32 pubs - we would likely see an invalid memory access crash the program. In the worst case where no mem violation happens, the aggregate would partly come from garbage data and fail to yield the desired price value.
    ** If C code is using 32 and Rust is giving it 64 pubs - there should be an assertion failure due to aggregation working only on the first half of the publisher set, and computing a trivial aggregate price of 100 with confidence 30, instead of the 110 with conf 20 that's supposed to come out of the two values "meeting in the middle'

let pub_pubkeys: Vec<_> = pub_keypairs.iter().map(|kp| kp.pubkey()).collect();

let security_authority = Keypair::new();
let price_accounts = sim
.setup_product_fixture(pub_pubkeys.as_slice(), security_authority.pubkey())
.await;
let price = price_accounts["LTC"];


let n_pubs = pub_keypairs.len();

// Divide publishers into two even parts (assuming the max PC_NUM_COMP size is even)
let (first_half, second_half) = pub_keypairs.split_at(n_pubs / 2);

// Starting with the first publisher in each half, publish an update
for (first_kp, second_kp) in first_half.iter().zip(second_half.iter()) {
let first_quote = Quote {
price: 100,
confidence: 30,
status: PC_STATUS_TRADING,
};

sim.upd_price(first_kp, price, first_quote).await?;

let second_quote = Quote {
price: 120,
confidence: 30,
status: PC_STATUS_TRADING,
};

sim.upd_price(second_kp, price, second_quote).await?;
}

// Advance slot once from 1 to 2
sim.warp_to_slot(2).await?;

// Final price update to trigger aggregation
let first_kp = pub_keypairs.first().unwrap();
let first_quote = Quote {
price: 100,
confidence: 30,
status: PC_STATUS_TRADING,
};
sim.upd_price(first_kp, price, first_quote).await?;

{
let price_data = sim
.get_account_data_as::<PriceAccount>(price)
.await
.unwrap();

assert_eq!(price_data.agg_.price_, 110);
assert_eq!(price_data.agg_.conf_, 20);
}

Ok(())
}
2 changes: 1 addition & 1 deletion program/rust/src/tests/test_publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ async fn test_publish() {
let publisher = Keypair::new();
let security_authority = Keypair::new();
let price_accounts = sim
.setup_product_fixture(publisher.pubkey(), security_authority.pubkey())
.setup_product_fixture(&[publisher.pubkey()], security_authority.pubkey())
.await;
let price = price_accounts["LTC"];

Expand Down
15 changes: 10 additions & 5 deletions program/rust/src/tests/test_publish_batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use {
PythSimulator,
Quote,
},
utils::get_status_for_update,
utils::get_status_for_conf_price_ratio,
},
solana_program::pubkey::Pubkey,
solana_sdk::{
Expand All @@ -26,7 +26,7 @@ async fn test_publish_batch() {
let publisher = Keypair::new();
let security_authority = Keypair::new();
let price_accounts = sim
.setup_product_fixture(publisher.pubkey(), security_authority.pubkey())
.setup_product_fixture(&[publisher.pubkey()], security_authority.pubkey())
.await;

for price in price_accounts.values() {
Expand Down Expand Up @@ -82,7 +82,7 @@ async fn test_publish_batch() {
assert_eq!(price_data.comp_[0].latest_.conf_, quote.confidence);
assert_eq!(
price_data.comp_[0].latest_.status_,
get_status_for_update(quote.price, quote.confidence, quote.status).unwrap()
get_status_for_conf_price_ratio(quote.price, quote.confidence, quote.status).unwrap()
);
assert_eq!(price_data.comp_[0].agg_.price_, 0);
assert_eq!(price_data.comp_[0].agg_.conf_, 0);
Expand Down Expand Up @@ -118,13 +118,18 @@ async fn test_publish_batch() {
assert_eq!(price_data.comp_[0].latest_.conf_, new_quote.confidence);
assert_eq!(
price_data.comp_[0].latest_.status_,
get_status_for_update(new_quote.price, new_quote.confidence, new_quote.status).unwrap()
get_status_for_conf_price_ratio(
new_quote.price,
new_quote.confidence,
new_quote.status
)
.unwrap()
);
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_update(quote.price, quote.confidence, quote.status).unwrap()
get_status_for_conf_price_ratio(quote.price, quote.confidence, quote.status).unwrap()
);
}
}
6 changes: 5 additions & 1 deletion program/rust/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,11 @@ pub fn is_component_update(cmd_args: &UpdPriceArgs) -> Result<bool, OracleError>
}

// Return PC_STATUS_IGNORED if confidence is bigger than price divided by MAX_CI_DIVISOR else returns status
pub fn get_status_for_update(price: i64, confidence: u64, status: u32) -> Result<u32, OracleError> {
pub fn get_status_for_conf_price_ratio(
price: i64,
confidence: u64,
status: u32,
) -> Result<u32, OracleError> {
let mut threshold_conf = price / MAX_CI_DIVISOR;

if threshold_conf < 0 {
Expand Down