diff --git a/lazer/contracts/sui/sources/channel.move b/lazer/contracts/sui/sources/channel.move index 86f332b9a6..0f5e4ca50b 100644 --- a/lazer/contracts/sui/sources/channel.move +++ b/lazer/contracts/sui/sources/channel.move @@ -1,17 +1,14 @@ module pyth_lazer::channel; +// Error codes for channel parsing +const EInvalidChannel: u64 = 1; + public enum Channel has copy, drop { - Invalid, RealTime, FixedRate50ms, FixedRate200ms, } -/// Create a new Invalid channel -public fun new_invalid(): Channel { - Channel::Invalid -} - /// Create a new RealTime channel public fun new_real_time(): Channel { Channel::RealTime @@ -27,11 +24,16 @@ public fun new_fixed_rate_200ms(): Channel { Channel::FixedRate200ms } -/// Check if the channel is Invalid -public fun is_invalid(channel: &Channel): bool { - match (channel) { - Channel::Invalid => true, - _ => false, +/// Parse channel from a channel value byte +public fun from_u8(channel_value: u8): Channel { + if (channel_value == 1) { + new_real_time() + } else if (channel_value == 2) { + new_fixed_rate_50ms() + } else if (channel_value == 3) { + new_fixed_rate_200ms() + } else { + abort EInvalidChannel } } diff --git a/lazer/contracts/sui/sources/feed.move b/lazer/contracts/sui/sources/feed.move index 894f9faa94..a937e62f4b 100644 --- a/lazer/contracts/sui/sources/feed.move +++ b/lazer/contracts/sui/sources/feed.move @@ -1,7 +1,11 @@ module pyth_lazer::feed; -use pyth_lazer::i16::I16; -use pyth_lazer::i64::I64; +use pyth_lazer::i16::{Self, I16}; +use pyth_lazer::i64::{Self, I64}; +use sui::bcs; + +// Error codes for feed parsing +const EInvalidProperty: u64 = 2; /// The feed struct is based on the Lazer rust protocol definition defined here: /// https://github.com/pyth-network/pyth-crosschain/blob/main/lazer/sdk/rust/protocol/src/payload.rs @@ -56,7 +60,7 @@ public(package) fun new( confidence, funding_rate, funding_timestamp, - funding_rate_interval + funding_rate_interval, } } @@ -156,6 +160,115 @@ public(package) fun set_funding_timestamp(feed: &mut Feed, funding_timestamp: Op } /// Set the funding rate interval -public(package) fun set_funding_rate_interval(feed: &mut Feed, funding_rate_interval: Option>) { +public(package) fun set_funding_rate_interval( + feed: &mut Feed, + funding_rate_interval: Option>, +) { feed.funding_rate_interval = funding_rate_interval; } + +/// Parse a feed from a BCS cursor +public(package) fun parse_from_cursor(cursor: &mut bcs::BCS): Feed { + let feed_id = cursor.peel_u32(); + let mut feed = new( + feed_id, + option::none(), + option::none(), + option::none(), + option::none(), + option::none(), + option::none(), + option::none(), + option::none(), + option::none(), + ); + + let properties_count = cursor.peel_u8(); + let mut properties_i = 0; + + while (properties_i < properties_count) { + let property_id = cursor.peel_u8(); + + if (property_id == 0) { + // Price property + let price = cursor.peel_u64(); + if (price != 0) { + feed.set_price(option::some(option::some(i64::from_u64(price)))); + } else { + feed.set_price(option::some(option::none())); + } + } else if (property_id == 1) { + // Best bid price property + let best_bid_price = cursor.peel_u64(); + if (best_bid_price != 0) { + feed.set_best_bid_price( + option::some(option::some(i64::from_u64(best_bid_price))), + ); + } else { + feed.set_best_bid_price(option::some(option::none())); + } + } else if (property_id == 2) { + // Best ask price property + let best_ask_price = cursor.peel_u64(); + if (best_ask_price != 0) { + feed.set_best_ask_price( + option::some(option::some(i64::from_u64(best_ask_price))), + ); + } else { + feed.set_best_ask_price(option::some(option::none())); + } + } else if (property_id == 3) { + // Publisher count property + let publisher_count = cursor.peel_u16(); + feed.set_publisher_count(option::some(publisher_count)); + } else if (property_id == 4) { + // Exponent property + let exponent = cursor.peel_u16(); + feed.set_exponent(option::some(i16::from_u16(exponent))); + } else if (property_id == 5) { + // Confidence property + let confidence = cursor.peel_u64(); + if (confidence != 0) { + feed.set_confidence(option::some(option::some(i64::from_u64(confidence)))); + } else { + feed.set_confidence(option::some(option::none())); + } + } else if (property_id == 6) { + // Funding rate property + let exists = cursor.peel_bool(); + if (exists) { + let funding_rate = cursor.peel_u64(); + feed.set_funding_rate(option::some(option::some(i64::from_u64(funding_rate)))); + } else { + feed.set_funding_rate(option::some(option::none())); + } + } else if (property_id == 7) { + // Funding timestamp property + let exists = cursor.peel_bool(); + if (exists) { + let funding_timestamp = cursor.peel_u64(); + feed.set_funding_timestamp(option::some(option::some(funding_timestamp))); + } else { + feed.set_funding_timestamp(option::some(option::none())); + } + } else if (property_id == 8) { + // Funding rate interval property + let exists = cursor.peel_bool(); + if (exists) { + let funding_rate_interval = cursor.peel_u64(); + feed.set_funding_rate_interval( + option::some(option::some(funding_rate_interval)), + ); + } else { + feed.set_funding_rate_interval(option::some(option::none())); + } + } else { + // Unknown property - we cannot safely skip it without knowing its length + abort EInvalidProperty + }; + + properties_i = properties_i + 1; + }; + + feed +} diff --git a/lazer/contracts/sui/sources/pyth_lazer.move b/lazer/contracts/sui/sources/pyth_lazer.move index b10413c382..a70102644f 100644 --- a/lazer/contracts/sui/sources/pyth_lazer.move +++ b/lazer/contracts/sui/sources/pyth_lazer.move @@ -1,10 +1,6 @@ module pyth_lazer::pyth_lazer; -use pyth_lazer::channel; -use pyth_lazer::feed::{Self, Feed}; use pyth_lazer::state::{Self, State}; -use pyth_lazer::i64::{Self}; -use pyth_lazer::i16::{Self}; use pyth_lazer::update::{Self, Update}; use sui::bcs; use sui::clock::Clock; @@ -15,12 +11,10 @@ const UPDATE_MESSAGE_MAGIC: u32 = 1296547300; const PAYLOAD_MAGIC: u32 = 2479346549; // Error codes -const EInvalidUpdate: u64 = 1; const ESignerNotTrusted: u64 = 2; const ESignerExpired: u64 = 3; - -// TODO: -// error handling +const EInvalidMagic: u64 = 4; +const EInvalidPayloadLength: u64 = 6; /// The `PYTH_LAZER` resource serves as the one-time witness. /// It has the `drop` ability, allowing it to be consumed immediately after use. @@ -83,161 +77,39 @@ public(package) fun verify_le_ecdsa_message( /// * `update` - The LeEcdsa formatted Lazer update /// /// # Errors -/// * `EInvalidUpdate` - Failed to parse the update according to the protocol definition +/// * `EInvalidMagic` - Invalid magic number in update or payload +/// * `EInvalidPayloadLength` - Payload length doesn't match actual data /// * `ESignerNotTrusted` - The recovered public key is not in the trusted signers list +/// * `ESignerExpired` - The signer's certificate has expired public fun parse_and_verify_le_ecdsa_update(s: &State, clock: &Clock, update: vector): Update { let mut cursor = bcs::new(update); - // TODO: introduce helper functions to check data len before peeling. allows us to return more - // granular error messages. - + // Parse and validate message magic let magic = cursor.peel_u32(); - assert!(magic == UPDATE_MESSAGE_MAGIC, 0); + assert!(magic == UPDATE_MESSAGE_MAGIC, EInvalidMagic); + // Parse signature let mut signature = vector::empty(); - let mut sig_i = 0; while (sig_i < SECP256K1_SIG_LEN) { signature.push_back(cursor.peel_u8()); sig_i = sig_i + 1; }; + // Parse expected payload length and get remaining bytes as payload let payload_len = cursor.peel_u16(); - let payload = cursor.into_remainder_bytes(); - assert!((payload_len as u64) == payload.length(), 0); - - let mut cursor = bcs::new(payload); - let payload_magic = cursor.peel_u32(); - assert!(payload_magic == PAYLOAD_MAGIC, 0); + // Validate expectedpayload length + assert!((payload_len as u64) == payload.length(), EInvalidPayloadLength); - let timestamp = cursor.peel_u64(); + // Parse payload + let mut payload_cursor = bcs::new(payload); + let payload_magic = payload_cursor.peel_u32(); + assert!(payload_magic == PAYLOAD_MAGIC, EInvalidMagic); // Verify the signature against trusted signers verify_le_ecdsa_message(s, clock, &signature, &payload); - let channel_value = cursor.peel_u8(); - let channel = if (channel_value == 0) { - channel::new_invalid() - } else if (channel_value == 1) { - channel::new_real_time() - } else if (channel_value == 2) { - channel::new_fixed_rate_50ms() - } else if (channel_value == 3) { - channel::new_fixed_rate_200ms() - } else { - channel::new_invalid() // Default to Invalid for unknown values - }; - - let mut feeds = vector::empty(); - let mut feed_i = 0; - - let feed_count = cursor.peel_u8(); - - while (feed_i < feed_count) { - let feed_id = cursor.peel_u32(); - let mut feed = feed::new( - feed_id, - option::none(), - option::none(), - option::none(), - option::none(), - option::none(), - option::none(), - option::none(), - option::none(), - option::none(), - ); - - let properties_count = cursor.peel_u8(); - let mut properties_i = 0; - - while (properties_i < properties_count) { - let property_id = cursor.peel_u8(); - - if (property_id == 0) { - let price = cursor.peel_u64(); - if (price != 0) { - feed.set_price(option::some(option::some(i64::from_u64(price)))); - } else { - feed.set_price(option::some(option::none())); - } - } else if (property_id == 1) { - let best_bid_price = cursor.peel_u64(); - if (best_bid_price != 0) { - feed.set_best_bid_price( - option::some(option::some(i64::from_u64(best_bid_price))), - ); - } else { - feed.set_best_bid_price(option::some(option::none())); - } - } else if (property_id == 2) { - let best_ask_price = cursor.peel_u64(); - if (best_ask_price != 0) { - feed.set_best_ask_price( - option::some(option::some(i64::from_u64(best_ask_price))), - ); - } else { - feed.set_best_ask_price(option::some(option::none())); - } - } else if (property_id == 3) { - let publisher_count = cursor.peel_u16(); - feed.set_publisher_count(option::some(publisher_count)); - } else if (property_id == 4) { - let exponent = cursor.peel_u16(); - feed.set_exponent(option::some(i16::from_u16(exponent))); - } else if (property_id == 5) { - let confidence = cursor.peel_u64(); - if (confidence != 0) { - feed.set_confidence(option::some(option::some(i64::from_u64(confidence)))); - } else { - feed.set_confidence(option::some(option::none())); - } - } else if (property_id == 6) { - let exists = cursor.peel_u8(); - if (exists == 1) { - let funding_rate = cursor.peel_u64(); - feed.set_funding_rate(option::some(option::some(i64::from_u64(funding_rate)))); - } else { - feed.set_funding_rate(option::some(option::none())); - } - } else if (property_id == 7) { - let exists = cursor.peel_u8(); - - if (exists == 1) { - let funding_timestamp = cursor.peel_u64(); - feed.set_funding_timestamp(option::some(option::some(funding_timestamp))); - } else { - feed.set_funding_timestamp(option::some(option::none())); - } - } else if (property_id == 8) { - let exists = cursor.peel_u8(); - - if (exists == 1) { - let funding_rate_interval = cursor.peel_u64(); - feed.set_funding_rate_interval( - option::some(option::some(funding_rate_interval)), - ); - } else { - feed.set_funding_rate_interval(option::some(option::none())); - } - } else { - // When we have an unknown property, we do not know its length, and therefore - // we cannot ignore it and parse the next properties. - abort EInvalidUpdate // FIXME: return more granular error messages - }; - - properties_i = properties_i + 1; - }; - - vector::push_back(&mut feeds, feed); - - feed_i = feed_i + 1; - }; - - let remaining_bytes = cursor.into_remainder_bytes(); - assert!(remaining_bytes.length() == 0, 0); - - update::new(timestamp, channel, feeds) + update::parse_from_cursor(payload_cursor) } diff --git a/lazer/contracts/sui/sources/update.move b/lazer/contracts/sui/sources/update.move index 60e51ec9be..f7845d5967 100644 --- a/lazer/contracts/sui/sources/update.move +++ b/lazer/contracts/sui/sources/update.move @@ -1,7 +1,11 @@ module pyth_lazer::update; -use pyth_lazer::channel::Channel; -use pyth_lazer::feed::Feed; +use pyth_lazer::channel::{Self, Channel}; +use pyth_lazer::feed::{Self, Feed}; +use sui::bcs; + +// Error codes for update parsing +const EInvalidPayload: u64 = 3; public struct Update has copy, drop { timestamp: u64, @@ -9,11 +13,7 @@ public struct Update has copy, drop { feeds: vector, } -public(package) fun new( - timestamp: u64, - channel: Channel, - feeds: vector -): Update { +public(package) fun new(timestamp: u64, channel: Channel, feeds: vector): Update { Update { timestamp, channel, feeds } } @@ -31,3 +31,31 @@ public fun channel(update: &Update): Channel { public fun feeds(update: &Update): vector { update.feeds } + +/// Parse the update from a BCS cursor containing the payload data +/// This assumes the payload magic has already been validated and consumed +public(package) fun parse_from_cursor(mut cursor: bcs::BCS): Update { + // Parse timestamp + let timestamp = cursor.peel_u64(); + + // Parse channel + let channel_value = cursor.peel_u8(); + let channel = channel::from_u8(channel_value); + + // Parse feeds + let feed_count = cursor.peel_u8(); + let mut feeds = vector::empty(); + let mut feed_i = 0; + + while (feed_i < feed_count) { + let feed = feed::parse_from_cursor(&mut cursor); + vector::push_back(&mut feeds, feed); + feed_i = feed_i + 1; + }; + + // Verify no remaining bytes + let remaining_bytes = cursor.into_remainder_bytes(); + assert!(remaining_bytes.length() == 0, EInvalidPayload); + + Update { timestamp, channel, feeds } +} diff --git a/lazer/contracts/sui/tests/pyth_lazer_tests.move b/lazer/contracts/sui/tests/pyth_lazer_tests.move index 5d3a20f5be..ded4fc42a9 100644 --- a/lazer/contracts/sui/tests/pyth_lazer_tests.move +++ b/lazer/contracts/sui/tests/pyth_lazer_tests.move @@ -6,7 +6,7 @@ use pyth_lazer::admin; use pyth_lazer::channel::new_fixed_rate_200ms; use pyth_lazer::i16; use pyth_lazer::i64; -use pyth_lazer::pyth_lazer::{parse_and_verify_le_ecdsa_update, verify_le_ecdsa_message, ESignerNotTrusted, ESignerExpired}; +use pyth_lazer::pyth_lazer::{parse_and_verify_le_ecdsa_update, verify_le_ecdsa_message, ESignerNotTrusted, ESignerExpired, EInvalidMagic, EInvalidPayloadLength}; use pyth_lazer::state; use sui::clock; @@ -227,3 +227,112 @@ public fun test_verify_le_ecdsa_message_multiple_signers() { admin::destroy_for_test(admin_cap); clock::destroy_for_testing(clock); } + +// === NEGATIVE PARSING TESTS === + +#[test, expected_failure(abort_code = EInvalidMagic)] +public fun test_parse_invalid_update_magic() { + let mut ctx = tx_context::dummy(); + let mut s = state::new_for_test(&mut ctx); + let admin_cap = admin::mint_for_test(&mut ctx); + let clock = clock::create_for_testing(&mut ctx); + + // Add the trusted signer + let trusted_pubkey = TEST_TRUSTED_SIGNER_PUBKEY; + let expiry_time = 2000000000000000; // Far in the future + state::update_trusted_signer(&admin_cap, &mut s, trusted_pubkey, expiry_time); + + // Create update with invalid magic (first 4 bytes corrupted) + let mut invalid_update = TEST_LAZER_UPDATE; + *vector::borrow_mut(&mut invalid_update, 0) = 0xFF; // Corrupt the magic + + // This should fail with EInvalidMagic + parse_and_verify_le_ecdsa_update(&s, &clock, invalid_update); + + // Clean up + state::destroy_for_test(s); + admin::destroy_for_test(admin_cap); + clock::destroy_for_testing(clock); +} + +#[test, expected_failure(abort_code = EInvalidMagic)] +public fun test_parse_invalid_payload_magic() { + let mut ctx = tx_context::dummy(); + let mut s = state::new_for_test(&mut ctx); + let admin_cap = admin::mint_for_test(&mut ctx); + let clock = clock::create_for_testing(&mut ctx); + + // Add the trusted signer + let trusted_pubkey = TEST_TRUSTED_SIGNER_PUBKEY; + let expiry_time = 2000000000000000; // Far in the future + state::update_trusted_signer(&admin_cap, &mut s, trusted_pubkey, expiry_time); + + // Create update with invalid payload magic + // The payload magic starts at byte 69 (4 bytes magic + 65 bytes signature + 2 payload length) + let mut invalid_update = TEST_LAZER_UPDATE; + *vector::borrow_mut(&mut invalid_update, 71) = 0xFF; // Corrupt the payload magic + + // This corrupts the payload magic, so expect EInvalidMagic + parse_and_verify_le_ecdsa_update(&s, &clock, invalid_update); + + // Clean up + state::destroy_for_test(s); + admin::destroy_for_test(admin_cap); + clock::destroy_for_testing(clock); +} + +#[test, expected_failure(abort_code = EInvalidPayloadLength)] +public fun test_parse_invalid_payload_length() { + let mut ctx = tx_context::dummy(); + let mut s = state::new_for_test(&mut ctx); + let admin_cap = admin::mint_for_test(&mut ctx); + let clock = clock::create_for_testing(&mut ctx); + + // Add the trusted signer + let trusted_pubkey = TEST_TRUSTED_SIGNER_PUBKEY; + let expiry_time = 2000000000000000; // Far in the future + state::update_trusted_signer(&admin_cap, &mut s, trusted_pubkey, expiry_time); + + // Create update with wrong payload length + // Layout: magic(4) + signature(65) + payload_len(2) + payload... + // So payload length is at bytes 69-70 + let mut invalid_update = TEST_LAZER_UPDATE; + *vector::borrow_mut(&mut invalid_update, 69) = 0xFF; // Set payload length too high + + // This should fail with EInvalidPayloadLength because payload length validation happens before signature verification + parse_and_verify_le_ecdsa_update(&s, &clock, invalid_update); + + // Clean up + state::destroy_for_test(s); + admin::destroy_for_test(admin_cap); + clock::destroy_for_testing(clock); +} + +#[test, expected_failure(abort_code = 0, location = sui::bcs)] +public fun test_parse_truncated_data() { + let mut ctx = tx_context::dummy(); + let mut s = state::new_for_test(&mut ctx); + let admin_cap = admin::mint_for_test(&mut ctx); + let clock = clock::create_for_testing(&mut ctx); + + // Add the trusted signer + let trusted_pubkey = TEST_TRUSTED_SIGNER_PUBKEY; + let expiry_time = 2000000000000000; // Far in the future + state::update_trusted_signer(&admin_cap, &mut s, trusted_pubkey, expiry_time); + + // Create truncated update (only first 50 bytes) + let mut truncated_update = vector::empty(); + let mut i = 0; + while (i < 50) { + vector::push_back(&mut truncated_update, *vector::borrow(&TEST_LAZER_UPDATE, i)); + i = i + 1; + }; + + // This should fail with BCS EOutOfRange error when trying to read beyond available data + parse_and_verify_le_ecdsa_update(&s, &clock, truncated_update); + + // Clean up + state::destroy_for_test(s); + admin::destroy_for_test(admin_cap); + clock::destroy_for_testing(clock); +}