From 36df8e4fa20959744d06eb14e4bbb8ed6c529048 Mon Sep 17 00:00:00 2001 From: Ali Behjati Date: Wed, 24 May 2023 11:53:40 +0200 Subject: [PATCH 1/2] Add deserialization to price messages --- program/rust/Cargo.toml | 2 +- program/rust/src/accounts.rs | 1 + program/rust/src/accounts/price.rs | 151 +++++++++++++++++++++++- program/rust/src/lib.rs | 3 + program/rust/src/processor/upd_price.rs | 2 +- program/rust/src/tests/test_message.rs | 106 ++++------------- 6 files changed, 175 insertions(+), 90 deletions(-) diff --git a/program/rust/Cargo.toml b/program/rust/Cargo.toml index b8b940f33..6efb7923e 100644 --- a/program/rust/Cargo.toml +++ b/program/rust/Cargo.toml @@ -14,9 +14,9 @@ bytemuck = "1.11.0" thiserror = "1.0" num-derive = "0.3" num-traits = "0.2" +byteorder = "1.4.3" [dev-dependencies] -byteorder = "1.4.3" solana-program-test = "=1.13.3" solana-sdk = "=1.13.3" tokio = "1.14.1" diff --git a/program/rust/src/accounts.rs b/program/rust/src/accounts.rs index 115ae3424..71a7cbda4 100644 --- a/program/rust/src/accounts.rs +++ b/program/rust/src/accounts.rs @@ -49,6 +49,7 @@ pub use { mapping::MappingAccount, permission::PermissionAccount, price::{ + Message, PriceAccount, PriceAccountV2, PriceComponent, diff --git a/program/rust/src/accounts/price.rs b/program/rust/src/accounts/price.rs index 07323a753..095d920fc 100644 --- a/program/rust/src/accounts/price.rs +++ b/program/rust/src/accounts/price.rs @@ -17,8 +17,18 @@ use { Pod, Zeroable, }, + byteorder::{ + BigEndian, + ReadBytesExt, + }, solana_program::pubkey::Pubkey, - std::mem::size_of, + std::{ + io::{ + Cursor, + Read, + }, + mem::size_of, + }, }; #[repr(C)] @@ -213,6 +223,34 @@ impl PythAccount for PriceAccountV2 { /// Messages are forward-compatible. You may add new fields to messages after all previously /// defined fields. All code for parsing messages must ignore any extraneous bytes at the end of /// the message (which could be fields that the code does not yet understand). + +#[derive(Debug, Copy, Clone, PartialEq)] +pub enum Message { + PriceFeedMessage(PriceFeedMessage), + TwapMessage(TwapMessage), +} + +#[allow(dead_code)] +impl Message { + pub fn try_from_bytes(bytes: &[u8]) -> Result { + match bytes[0] { + PriceFeedMessage::DISCRIMINATOR => Ok(Self::PriceFeedMessage( + PriceFeedMessage::try_from_bytes(bytes)?, + )), + TwapMessage::DISCRIMINATOR => { + Ok(Self::TwapMessage(TwapMessage::try_from_bytes(bytes)?)) + } + _ => Err(OracleError::DeserializationError), + } + } + pub fn to_bytes(self) -> Vec { + match self { + Self::PriceFeedMessage(msg) => msg.to_bytes().to_vec(), + Self::TwapMessage(msg) => msg.to_bytes().to_vec(), + } + } +} + #[repr(C)] #[derive(Debug, Copy, Clone, PartialEq)] pub struct PriceFeedMessage { @@ -273,7 +311,7 @@ impl PriceFeedMessage { /// Note that it would be more idiomatic to return a `Vec`, but that approach adds /// to the size of the compiled binary (which is already close to the size limit). #[allow(unused_assignments)] - pub fn as_bytes(&self) -> [u8; Self::MESSAGE_SIZE] { + pub fn to_bytes(self) -> [u8; Self::MESSAGE_SIZE] { let mut bytes = [0u8; Self::MESSAGE_SIZE]; let mut i: usize = 0; @@ -307,6 +345,60 @@ impl PriceFeedMessage { bytes } + + /// Try to deserialize a message from an array of bytes (including the discriminator). + /// This method is forward-compatible and allows the size to be larger than the + /// size of the struct. As a side-effect, it will ignore newer fields that are + /// not yet present in the struct. + pub fn try_from_bytes(bytes: &[u8]) -> Result { + let mut cursor = Cursor::new(bytes); + + let discriminator = cursor + .read_u8() + .map_err(|_| OracleError::DeserializationError)?; + if discriminator != 0 { + return Err(OracleError::DeserializationError); + } + + let mut id = [0u8; 32]; + cursor + .read_exact(&mut id) + .map_err(|_| OracleError::DeserializationError)?; + + let price = cursor + .read_i64::() + .map_err(|_| OracleError::DeserializationError)?; + let conf = cursor + .read_u64::() + .map_err(|_| OracleError::DeserializationError)?; + let exponent = cursor + .read_i32::() + .map_err(|_| OracleError::DeserializationError)?; + let publish_time = cursor + .read_i64::() + .map_err(|_| OracleError::DeserializationError)?; + let prev_publish_time = cursor + .read_i64::() + .map_err(|_| OracleError::DeserializationError)?; + let ema_price = cursor + .read_i64::() + .map_err(|_| OracleError::DeserializationError)?; + let ema_conf = cursor + .read_u64::() + .map_err(|_| OracleError::DeserializationError)?; + + + Ok(Self { + id, + price, + conf, + exponent, + publish_time, + prev_publish_time, + ema_price, + ema_conf, + }) + } } /// Message format for sending Twap data via the accumulator program @@ -353,7 +445,7 @@ impl TwapMessage { /// Note that it would be more idiomatic to return a `Vec`, but that approach adds /// to the size of the compiled binary (which is already close to the size limit). #[allow(unused_assignments)] - pub fn as_bytes(&self) -> [u8; Self::MESSAGE_SIZE] { + pub fn to_bytes(self) -> [u8; Self::MESSAGE_SIZE] { let mut bytes = [0u8; Self::MESSAGE_SIZE]; let mut i: usize = 0; @@ -387,4 +479,57 @@ impl TwapMessage { bytes } + + /// Try to deserialize a message from an array of bytes (including the discriminator). + /// This method is forward-compatible and allows the size to be larger than the + /// size of the struct. As a side-effect, it will ignore newer fields that are + /// not yet present in the struct. + pub fn try_from_bytes(bytes: &[u8]) -> Result { + let mut cursor = Cursor::new(bytes); + + let discriminator = cursor + .read_u8() + .map_err(|_| OracleError::DeserializationError)?; + if discriminator != 1 { + return Err(OracleError::DeserializationError); + } + + let mut id = [0u8; 32]; + cursor + .read_exact(&mut id) + .map_err(|_| OracleError::DeserializationError)?; + + let cumulative_price = cursor + .read_i128::() + .map_err(|_| OracleError::DeserializationError)?; + let cumulative_conf = cursor + .read_u128::() + .map_err(|_| OracleError::DeserializationError)?; + let num_down_slots = cursor + .read_u64::() + .map_err(|_| OracleError::DeserializationError)?; + let exponent = cursor + .read_i32::() + .map_err(|_| OracleError::DeserializationError)?; + let publish_time = cursor + .read_i64::() + .map_err(|_| OracleError::DeserializationError)?; + let prev_publish_time = cursor + .read_i64::() + .map_err(|_| OracleError::DeserializationError)?; + let publish_slot = cursor + .read_u64::() + .map_err(|_| OracleError::DeserializationError)?; + + Ok(Self { + id, + cumulative_price, + cumulative_conf, + num_down_slots, + exponent, + publish_time, + prev_publish_time, + publish_slot, + }) + } } diff --git a/program/rust/src/lib.rs b/program/rust/src/lib.rs index 332b737d7..75ac958d1 100644 --- a/program/rust/src/lib.rs +++ b/program/rust/src/lib.rs @@ -27,13 +27,16 @@ mod log; pub use accounts::{ AccountHeader, MappingAccount, + Message, PermissionAccount, PriceAccount, PriceComponent, PriceEma, + PriceFeedMessage, PriceInfo, ProductAccount, PythAccount, + TwapMessage, }; use { crate::error::OracleError, diff --git a/program/rust/src/processor/upd_price.rs b/program/rust/src/processor/upd_price.rs index 2ce1d9611..833fab9a7 100644 --- a/program/rust/src/processor/upd_price.rs +++ b/program/rust/src/processor/upd_price.rs @@ -228,7 +228,7 @@ pub fn upd_price( let message = vec![ PriceFeedMessage::from_price_account(price_account.key, &price_data) - .as_bytes() + .to_bytes() .to_vec(), ]; diff --git a/program/rust/src/tests/test_message.rs b/program/rust/src/tests/test_message.rs index 3555613ce..ffdc4705f 100644 --- a/program/rust/src/tests/test_message.rs +++ b/program/rust/src/tests/test_message.rs @@ -1,18 +1,11 @@ use { crate::accounts::{ + Message, PriceFeedMessage, TwapMessage, }, - byteorder::{ - BigEndian, - ReadBytesExt, - }, quickcheck::Arbitrary, quickcheck_macros::quickcheck, - std::io::{ - Cursor, - Read, - }, }; impl Arbitrary for PriceFeedMessage { @@ -37,55 +30,15 @@ impl Arbitrary for PriceFeedMessage { } } -/// TODO: move this parsing implementation to a separate data format library. -impl PriceFeedMessage { - fn from_bytes(bytes: &[u8]) -> Option { - let mut cursor = Cursor::new(bytes); - - let discriminator = cursor.read_u8().ok()?; - if discriminator != 0 { - return None; - } - - let mut id = [0u8; 32]; - cursor.read_exact(&mut id).ok()?; - - let price = cursor.read_i64::().ok()?; - let conf = cursor.read_u64::().ok()?; - let exponent = cursor.read_i32::().ok()?; - let publish_time = cursor.read_i64::().ok()?; - let prev_publish_time = cursor.read_i64::().ok()?; - let ema_price = cursor.read_i64::().ok()?; - let ema_conf = cursor.read_u64::().ok()?; - - - if cursor.position() as usize != bytes.len() { - // The message bytes are longer than expected - None - } else { - Some(Self { - id, - price, - conf, - exponent, - publish_time, - prev_publish_time, - ema_price, - ema_conf, - }) - } - } -} - #[quickcheck] fn test_price_feed_message_roundtrip(input: PriceFeedMessage) -> bool { - let reconstructed = PriceFeedMessage::from_bytes(&input.as_bytes()); + let reconstructed = PriceFeedMessage::try_from_bytes(&input.to_bytes()); println!("Failed test case:"); println!("{:?}", input); println!("{:?}", reconstructed); - reconstructed.is_some() && reconstructed.unwrap() == input + reconstructed == Ok(input) } @@ -111,51 +64,34 @@ impl Arbitrary for TwapMessage { } } -impl TwapMessage { - fn from_bytes(bytes: &[u8]) -> Option { - let mut cursor = Cursor::new(bytes); +#[quickcheck] +fn test_twap_message_roundtrip(input: TwapMessage) -> bool { + let reconstructed = TwapMessage::try_from_bytes(&input.to_bytes()); + + println!("Failed test case:"); + println!("{:?}", input); + println!("{:?}", reconstructed); - let discriminator = cursor.read_u8().ok()?; - if discriminator != 1 { - return None; - } + reconstructed == Ok(input) +} - let mut id = [0u8; 32]; - cursor.read_exact(&mut id).ok()?; - - let cumulative_price = cursor.read_i128::().ok()?; - let cumulative_conf = cursor.read_u128::().ok()?; - let num_down_slots = cursor.read_u64::().ok()?; - let exponent = cursor.read_i32::().ok()?; - let publish_time = cursor.read_i64::().ok()?; - let prev_publish_time = cursor.read_i64::().ok()?; - let publish_slot = cursor.read_u64::().ok()?; - - if cursor.position() as usize != bytes.len() { - // The message bytes are longer than expected - None - } else { - Some(Self { - id, - cumulative_price, - cumulative_conf, - num_down_slots, - exponent, - publish_time, - prev_publish_time, - publish_slot, - }) + +impl Arbitrary for Message { + fn arbitrary(g: &mut quickcheck::Gen) -> Self { + match u8::arbitrary(g) % 2 { + 0 => Message::PriceFeedMessage(Arbitrary::arbitrary(g)), + _ => Message::TwapMessage(Arbitrary::arbitrary(g)), } } } #[quickcheck] -fn test_twap_message_roundtrip(input: TwapMessage) -> bool { - let reconstructed = TwapMessage::from_bytes(&input.as_bytes()); +fn test_message_roundtrip(input: Message) -> bool { + let reconstructed = Message::try_from_bytes(&input.to_bytes()); println!("Failed test case:"); println!("{:?}", input); println!("{:?}", reconstructed); - reconstructed.is_some() && reconstructed.unwrap() == input + reconstructed == Ok(input) } From ffa5045ba607f4cdce4769e4ada676cee7cc82d2 Mon Sep 17 00:00:00 2001 From: Ali Behjati Date: Thu, 25 May 2023 10:48:25 +0200 Subject: [PATCH 2/2] Update comments --- program/rust/src/accounts/price.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/program/rust/src/accounts/price.rs b/program/rust/src/accounts/price.rs index 095d920fc..62281e91b 100644 --- a/program/rust/src/accounts/price.rs +++ b/program/rust/src/accounts/price.rs @@ -223,6 +223,11 @@ impl PythAccount for PriceAccountV2 { /// Messages are forward-compatible. You may add new fields to messages after all previously /// defined fields. All code for parsing messages must ignore any extraneous bytes at the end of /// the message (which could be fields that the code does not yet understand). +/// +/// The oracle is not using the Message enum due to the contract size limit and +/// some of the methods for PriceFeedMessage and TwapMessage are not used by the oracle +/// for the same reason. Rust compiler doesn't include the unused methods in the contract. +/// Once we start using the unused structs and methods, the contract size will increase. #[derive(Debug, Copy, Clone, PartialEq)] pub enum Message {