feat(cairo_contracts): Fixes from review

cairo/crates/pragma_dispatcher/src/contract.cairo

@@ -5,6 +5,7 @@ pub mod PragmaDispatcher {
     use hyperlane_starknet::interfaces::{IMailboxDispatcher, IMailboxDispatcherTrait};
     use openzeppelin::access::ownable::OwnableComponent;
     use openzeppelin::upgrades::{interface::IUpgradeable, upgradeable::UpgradeableComponent};
+    use pragma_dispatcher::errors;
     use pragma_dispatcher::interface::{
         IPragmaDispatcher, IHyperlaneMailboxWrapper, IPragmaOracleWrapper, ISummaryStatsWrapper,
         IPragmaFeedsRegistryWrapper,
@@ -241,7 +242,7 @@ pub mod PragmaDispatcher {
             hyperlane_mailbox_address: ContractAddress,
         ) {
             // [Check]
-            assert(!owner.is_zero(), 'Owner cannot be 0');
+            assert(!owner.is_zero(), errors::OWNER_IS_ZERO);
 
             // [Effect]
             self.ownable.initializer(owner);

cairo/crates/pragma_dispatcher/src/errors.cairo

@@ -0,0 +1 @@
+pub const OWNER_IS_ZERO: felt252 = 'Owner cannot be 0';

cairo/crates/pragma_dispatcher/src/lib.cairo

@@ -1,3 +1,4 @@
 pub mod contract;
+pub mod errors;
 pub mod interface;
 pub mod types;

cairo/crates/pragma_feed_types/src/asset_class.cairo

@@ -1,4 +1,4 @@
-#[derive(Debug, Drop, Clone, Serde, PartialEq, Hash)]
+#[derive(Debug, Drop, Copy, Serde, PartialEq, Hash)]
 pub enum AssetClass {
     Crypto,
 }

cairo/crates/pragma_feed_types/src/feed.cairo

@@ -1,6 +1,13 @@
 use pragma_feed_types::{AssetClass, AssetClassId, FeedType, FeedTypeId};
-use pragma_maths::felt252::{FeltBitAnd, FeltDiv};
+use pragma_maths::felt252::{FeltBitAnd, FeltDiv, FeltOrd};
 
+// Constants used for felt manipulations when decoding the FeedId.
+const ASSET_CLASS_SHIFT: felt252 = 0x100000000000000000000000000000000; // 2^128
+const FEED_TYPE_SHIFT: felt252 = 0x100000000000000; // 2^64
+const FEED_TYPE_MASK: felt252 = 0xFFFFFFFFFFFFFFFF; // 2^64 - 1
+const MAX_PAIR_ID: felt252 = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF; // 2^224 - 1 (28 bytes)
+
+// Type aliases for identifiers.
 pub type PairId = felt252;
 pub type FeedId = felt252;
 
@@ -11,44 +18,68 @@ pub struct Feed {
     pub pair_id: PairId,
 }
 
-pub trait FeedTrait {
-    /// Try to construct a Feed from the provided FeedId.
-    fn from_id(id: FeedId) -> Option<Feed>;
+#[derive(Drop, Copy, PartialEq)]
+pub enum FeedError {
+    Conversion: felt252,
+}
 
-    /// Returns the id of the Feed.
-    fn id(self: @Feed) -> FeedId;
+impl FeedErrorIntoFelt of Into<FeedError, felt252> {
+    fn into(self: FeedError) -> felt252 {
+        match self {
+            FeedError::Conversion(msg) => msg,
+        }
+    }
 }
 
+#[generate_trait]
 pub impl FeedTraitImpl of FeedTrait {
     /// Try to construct a Feed from the provided FeedId.
-    fn from_id(id: FeedId) -> Option<Feed> {
-        let asset_class_felt = id / 0x100000000000000000000000000000000;
-        let asset_class: AssetClass = asset_class_felt.try_into().unwrap();
+    fn from_id(id: FeedId) -> Result<Feed, FeedError> {
+        // Extract asset_class (first 2 bytes)
+        let asset_class_felt = id / ASSET_CLASS_SHIFT;
+        let asset_class_option: Option<AssetClass> = asset_class_felt.try_into();
+        if asset_class_option.is_none() {
+            return Result::Err(FeedError::Conversion('Invalid asset class encoding'));
+        }
 
-        // Extract feed_type (middle 64 bits)
-        let feed_type_felt = (id / 0x100000000000000) & 0xFFFFFFFFFFFFFFFF;
-        let feed_type: FeedType = feed_type_felt.try_into().unwrap();
+        // Extract feed_type (next 2 bytes)
+        let feed_type_felt = (id / FEED_TYPE_SHIFT) & FEED_TYPE_MASK;
+        let feed_type_option: Option<FeedType> = feed_type_felt.try_into();
+        if feed_type_option.is_none() {
+            return Result::Err(FeedError::Conversion('Invalid feed type encoding'));
+        }
 
-        // Extract pair_id (remaining bits, maximum 28 bytes)
+        // Extract pair_id (remaining bytes, maximum 28)
         let pair_id = id
-            - (asset_class_felt * 0x100000000000000000000000000000000)
-            - (feed_type_felt * 0x100000000000000);
+            - (asset_class_felt * ASSET_CLASS_SHIFT)
+            - (feed_type_felt * FEED_TYPE_SHIFT);
+
+        // Check if pair_id exceeds 28 bytes
+        if pair_id > MAX_PAIR_ID {
+            return Result::Err(FeedError::Conversion('Pair id greater than 28 bytes'));
+        }
 
-        Option::Some(Feed { asset_class, feed_type, pair_id })
+        Result::Ok(
+            Feed {
+                asset_class: asset_class_option.unwrap(), // safe unwrap
+                feed_type: feed_type_option.unwrap(), // safe unwrap
+                pair_id
+            }
+        )
     }
 
     /// Returns the id of the Feed.
     fn id(self: @Feed) -> FeedId {
-        let asset_class_id: AssetClassId = self.asset_class.clone().into();
+        let asset_class_id: AssetClassId = (*self.asset_class).into();
         let asset_class_felt: felt252 = asset_class_id.into();
 
-        let feed_type_id: FeedTypeId = self.feed_type.clone().into();
+        let feed_type_id: FeedTypeId = (*self.feed_type).into();
         let feed_type_felt: felt252 = feed_type_id.into();
 
         // Shift left by 128 bits
-        let shifted_asset_class = asset_class_felt * 0x100000000000000000000000000000000;
+        let shifted_asset_class = asset_class_felt * ASSET_CLASS_SHIFT;
         // Shift left by 64 bits
-        let shifted_feed_type = feed_type_felt * 0x100000000000000;
+        let shifted_feed_type = feed_type_felt * FEED_TYPE_SHIFT;
 
         // Combine all fields
         shifted_asset_class + shifted_feed_type + *self.pair_id

cairo/crates/pragma_feed_types/src/feed_type.cairo

@@ -1,4 +1,4 @@
-#[derive(Debug, Drop, Clone, Serde, PartialEq, Hash)]
+#[derive(Debug, Drop, Copy, Serde, PartialEq, Hash)]
 pub enum FeedType {
     SpotMedian,
     Twap,

cairo/crates/pragma_feed_types/tests/test_feed.cairo

@@ -18,39 +18,6 @@ fn test_valid_feed_id_conversion() {
     assert(out_feed == expected_feed.into(), 'Incorrect feed id');
 }
 
-#[test]
-fn test_no_collision_same_asset_class_different_feed_type() {
-    let feed1 = create_random_feed(AssetClass::Crypto, FeedType::SpotMedian, 'BTC/USD');
-    let feed2 = create_random_feed(AssetClass::Crypto, FeedType::Twap, 'BTC/USD');
-
-    let id1: FeedId = feed1.id();
-    let id2: FeedId = feed2.id();
-
-    assert(id1 != id2, 'IDs should be different');
-}
-
-#[test]
-fn test_no_collision_different_asset_class_same_feed_type() {
-    let feed1 = create_random_feed(AssetClass::Crypto, FeedType::SpotMedian, 'BTC/USD');
-    let feed2 = create_random_feed(AssetClass::Crypto, FeedType::SpotMedian, 'EUR/USD');
-
-    let id1: FeedId = feed1.id();
-    let id2: FeedId = feed2.id();
-
-    assert(id1 != id2, 'IDs should be different');
-}
-
-#[test]
-fn test_no_collision_different_pair_id() {
-    let feed1 = create_random_feed(AssetClass::Crypto, FeedType::SpotMedian, 'BTC/USD');
-    let feed2 = create_random_feed(AssetClass::Crypto, FeedType::SpotMedian, 'ETH/USD');
-
-    let id1: FeedId = feed1.id();
-    let id2: FeedId = feed2.id();
-
-    assert(id1 != id2, 'IDs should be different');
-}
-
 #[test]
 fn test_no_collision_random_feeds(
     pair_id1: felt252, feed_type_1: u8, pair_id2: felt252, feed_type_2: u8,

cairo/crates/pragma_feeds_registry/src/contract.cairo

@@ -1,5 +1,7 @@
 #[starknet::contract]
 pub mod PragmaFeedRegistry {
+    use core::num::traits::Zero;
+    use core::panic_with_felt252;
     use openzeppelin::access::ownable::OwnableComponent;
     use openzeppelin::upgrades::{UpgradeableComponent, interface::IUpgradeable};
     use pragma_feed_types::{FeedId, FeedTrait};
@@ -62,6 +64,9 @@ pub mod PragmaFeedRegistry {
 
     #[constructor]
     fn constructor(ref self: ContractState, owner: ContractAddress) {
+        // [Check]
+        assert(!owner.is_zero(), errors::OWNER_IS_ZERO);
+        // [Effect]
         self.ownable.initializer(owner);
     }
 
@@ -80,7 +85,10 @@ pub mod PragmaFeedRegistry {
             // [Check] Feed id not already registered
             assert(!self.feed_exists(feed_id), errors::FEED_ALREADY_REGISTERED);
             // [Check] Feed id format
-            assert(FeedTrait::from_id(feed_id).is_some(), errors::INVALID_FEED_FORMAT);
+            match FeedTrait::from_id(feed_id) {
+                Result::Ok(_) => {},
+                Result::Err(e) => panic_with_felt252(e.into())
+            };
 
             // [Effect] Insert new feed id
             let len_feed_ids = self.len_feed_ids.read();

cairo/crates/pragma_feeds_registry/src/errors.cairo

@@ -1,3 +1,4 @@
+pub const OWNER_IS_ZERO: felt252 = 'Owner cannot be 0';
 pub const FEED_ALREADY_REGISTERED: felt252 = 'Feed ID already registed';
 pub const INVALID_FEED_FORMAT: felt252 = 'Invalid feed ID format';
 pub const FEED_NOT_REGISTERED: felt252 = 'Feed ID not registered';

cairo/crates/pragma_maths/src/felt252.cairo

@@ -13,3 +13,30 @@ pub impl FeltDiv of Div<felt252> {
         (lhs256 / rhs256).try_into().unwrap()
     }
 }
+
+fn felt252_le(lhs: felt252, rhs: felt252) -> bool {
+    Into::<felt252, u256>::into(lhs) <= rhs.into()
+}
+
+fn felt252_lt(lhs: felt252, rhs: felt252) -> bool {
+    Into::<felt252, u256>::into(lhs) < rhs.into()
+}
+
+pub impl FeltOrd of PartialOrd<felt252> {
+    #[inline(always)]
+    fn le(lhs: felt252, rhs: felt252) -> bool {
+        felt252_le(lhs, rhs)
+    }
+    #[inline(always)]
+    fn ge(lhs: felt252, rhs: felt252) -> bool {
+        felt252_le(rhs, lhs)
+    }
+    #[inline(always)]
+    fn lt(lhs: felt252, rhs: felt252) -> bool {
+        felt252_lt(lhs, rhs)
+    }
+    #[inline(always)]
+    fn gt(lhs: felt252, rhs: felt252) -> bool {
+        felt252_lt(rhs, lhs)
+    }
+}