feat(lazer/sui): better parsing and error handling (#3003)

* feat(lazer/sui): better parsing and error handling

* nit: comments

* remove invalid channel enum variant

Author: tejasbadadare

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
     }
 }
 

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<Option<u64>>) {
+public(package) fun set_funding_rate_interval(
+    feed: &mut Feed,
+    funding_rate_interval: Option<Option<u64>>,
+) {
     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
+}

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<u8>): 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<u8>();
-
     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<Feed>();
-    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)
 }