audit feedback

MystenLabs · May 8, 2024 · 0b69c29 · 0b69c29
1 parent e94113a
commit 0b69c29
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 35 deletions.
diff --git a/crates/sui-bridge/src/server/mod.rs b/crates/sui-bridge/src/server/mod.rs
@@ -286,7 +286,7 @@ async fn handle_add_tokens_on_sui(
 
     if !chain_id.is_sui_chain() {
         return Err(BridgeError::InvalidBridgeClientRequest(
-            "handle_add_tokens_on_evm only expects Sui chain id".to_string(),
+            "handle_add_tokens_on_sui only expects Sui chain id".to_string(),
         ));
     }
 

diff --git a/crates/sui-framework/docs/bridge/treasury.md b/crates/sui-framework/docs/bridge/treasury.md
@@ -299,11 +299,20 @@ title: Module `0xb::treasury`
 ## Constants
 
 
+<a name="0xb_treasury_EInvalidNotionalValue"></a>
+
+
+
+<pre><code><b>const</b> <a href="treasury.md#0xb_treasury_EInvalidNotionalValue">EInvalidNotionalValue</a>: u64 = 4;
+</code></pre>
+
+
+
 <a name="0xb_treasury_EInvalidUpgradeCap"></a>
 
 
 
-<pre><code><b>const</b> <a href="treasury.md#0xb_treasury_EInvalidUpgradeCap">EInvalidUpgradeCap</a>: u64 = 1;
+<pre><code><b>const</b> <a href="treasury.md#0xb_treasury_EInvalidUpgradeCap">EInvalidUpgradeCap</a>: u64 = 2;
 </code></pre>
 
 
@@ -312,7 +321,7 @@ title: Module `0xb::treasury`
 
 
 
-<pre><code><b>const</b> <a href="treasury.md#0xb_treasury_ETokenSupplyNonZero">ETokenSupplyNonZero</a>: u64 = 2;
+<pre><code><b>const</b> <a href="treasury.md#0xb_treasury_ETokenSupplyNonZero">ETokenSupplyNonZero</a>: u64 = 3;
 </code></pre>
 
 
@@ -321,7 +330,7 @@ title: Module `0xb::treasury`
 
 
 
-<pre><code><b>const</b> <a href="treasury.md#0xb_treasury_EUnsupportedTokenType">EUnsupportedTokenType</a>: u64 = 0;
+<pre><code><b>const</b> <a href="treasury.md#0xb_treasury_EUnsupportedTokenType">EUnsupportedTokenType</a>: u64 = 1;
 </code></pre>
 
 
@@ -416,22 +425,30 @@ title: Module `0xb::treasury`
 <summary>Implementation</summary>
 
 
-<pre><code><b>public</b>(<a href="../sui-framework/package.md#0x2_package">package</a>) <b>fun</b> <a href="treasury.md#0xb_treasury_register_foreign_token">register_foreign_token</a>&lt;T&gt;(self: &<b>mut</b> <a href="treasury.md#0xb_treasury_BridgeTreasury">BridgeTreasury</a>, tc: TreasuryCap&lt;T&gt;, uc: UpgradeCap, metadata: &CoinMetadata&lt;T&gt;) {
+<pre><code><b>public</b>(<a href="../sui-framework/package.md#0x2_package">package</a>) <b>fun</b> <a href="treasury.md#0xb_treasury_register_foreign_token">register_foreign_token</a>&lt;T&gt;(
+    self: &<b>mut</b> <a href="treasury.md#0xb_treasury_BridgeTreasury">BridgeTreasury</a>,
+    tc: TreasuryCap&lt;T&gt;,
+    uc: UpgradeCap,
+    metadata: &CoinMetadata&lt;T&gt;,
+) {
     // Make sure TreasuryCap <b>has</b> not been minted before.
     <b>assert</b>!(<a href="../sui-framework/coin.md#0x2_coin_total_supply">coin::total_supply</a>(&tc) == 0, <a href="treasury.md#0xb_treasury_ETokenSupplyNonZero">ETokenSupplyNonZero</a>);
     <b>let</b> <a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a> = <a href="../move-stdlib/type_name.md#0x1_type_name_get">type_name::get</a>&lt;T&gt;();
     <b>let</b> address_bytes = <a href="../sui-framework/hex.md#0x2_hex_decode">hex::decode</a>(<a href="../move-stdlib/ascii.md#0x1_ascii_into_bytes">ascii::into_bytes</a>(<a href="../move-stdlib/type_name.md#0x1_type_name_get_address">type_name::get_address</a>(&<a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a>)));
     <b>let</b> coin_address = address::from_bytes(address_bytes);
     // Make sure upgrade cap is for the Coin <a href="../sui-framework/package.md#0x2_package">package</a>
     // FIXME: add test
-    <b>assert</b>!(<a href="../sui-framework/object.md#0x2_object_id_to_address">object::id_to_address</a>(&<a href="../sui-framework/package.md#0x2_package_upgrade_package">package::upgrade_package</a>(&uc)) == coin_address, <a href="treasury.md#0xb_treasury_EInvalidUpgradeCap">EInvalidUpgradeCap</a>);
+    <b>assert</b>!(
+        <a href="../sui-framework/object.md#0x2_object_id_to_address">object::id_to_address</a>(&<a href="../sui-framework/package.md#0x2_package_upgrade_package">package::upgrade_package</a>(&uc))
+            == coin_address, <a href="treasury.md#0xb_treasury_EInvalidUpgradeCap">EInvalidUpgradeCap</a>
+    );
     <b>let</b> registration = <a href="treasury.md#0xb_treasury_ForeignTokenRegistration">ForeignTokenRegistration</a> {
         <a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a>,
         uc,
         decimal: <a href="../sui-framework/coin.md#0x2_coin_get_decimals">coin::get_decimals</a>(metadata),
     };
-    <a href="../sui-framework/bag.md#0x2_bag_add">bag::add</a>(&<b>mut</b> self.waiting_room, <a href="../move-stdlib/type_name.md#0x1_type_name_into_string">type_name::into_string</a>(<a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a>), registration);
-    <a href="../sui-framework/object_bag.md#0x2_object_bag_add">object_bag::add</a>(&<b>mut</b> self.treasuries, <a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a>, tc);
+    self.waiting_room.add(<a href="../move-stdlib/type_name.md#0x1_type_name_into_string">type_name::into_string</a>(<a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a>), registration);
+    self.treasuries.add(<a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a>, tc);
 
     emit(<a href="treasury.md#0xb_treasury_TokenRegistrationEvent">TokenRegistrationEvent</a>{
         <a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a>,
@@ -460,21 +477,31 @@ title: Module `0xb::treasury`
 <summary>Implementation</summary>
 
 
-<pre><code><b>public</b>(<a href="../sui-framework/package.md#0x2_package">package</a>) <b>fun</b> <a href="treasury.md#0xb_treasury_add_new_token">add_new_token</a>(self: &<b>mut</b> <a href="treasury.md#0xb_treasury_BridgeTreasury">BridgeTreasury</a>, token_name: String, token_id:u8, native_token: bool, notional_value: u64) {
+<pre><code><b>public</b>(<a href="../sui-framework/package.md#0x2_package">package</a>) <b>fun</b> <a href="treasury.md#0xb_treasury_add_new_token">add_new_token</a>(
+    self: &<b>mut</b> <a href="treasury.md#0xb_treasury_BridgeTreasury">BridgeTreasury</a>,
+    token_name: String,
+    token_id:u8,
+    native_token: bool,
+    notional_value: u64,
+) {
     <b>if</b> (!native_token){
+        <b>assert</b>!(notional_value &gt; 0, <a href="treasury.md#0xb_treasury_EInvalidNotionalValue">EInvalidNotionalValue</a>);
         <b>let</b> <a href="treasury.md#0xb_treasury_ForeignTokenRegistration">ForeignTokenRegistration</a>{
             <a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a>,
             uc,
             decimal,
         } = <a href="../sui-framework/bag.md#0x2_bag_remove">bag::remove</a>&lt;String, <a href="treasury.md#0xb_treasury_ForeignTokenRegistration">ForeignTokenRegistration</a>&gt;(&<b>mut</b> self.waiting_room, token_name);
         <b>let</b> decimal_multiplier = <a href="../sui-framework/math.md#0x2_math_pow">math::pow</a>(10, decimal);
-        <a href="../sui-framework/vec_map.md#0x2_vec_map_insert">vec_map::insert</a>(&<b>mut</b> self.supported_tokens, <a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a>, <a href="treasury.md#0xb_treasury_BridgeTokenMetadata">BridgeTokenMetadata</a>{
-            id: token_id,
-            decimal_multiplier,
-            notional_value,
-            native_token
-        });
-        <a href="../sui-framework/vec_map.md#0x2_vec_map_insert">vec_map::insert</a>(&<b>mut</b> self.id_token_type_map, token_id, <a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a>);
+        self.supported_tokens.insert(
+            <a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a>,
+            <a href="treasury.md#0xb_treasury_BridgeTokenMetadata">BridgeTokenMetadata</a>{
+                id: token_id,
+                decimal_multiplier,
+                notional_value,
+                native_token
+            },
+        );
+        self.id_token_type_map.insert(token_id, <a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a>);
 
         // Freeze upgrade cap <b>to</b> prevent changes <b>to</b> the <a href="../sui-framework/coin.md#0x2_coin">coin</a>
         <a href="../sui-framework/transfer.md#0x2_transfer_public_freeze_object">transfer::public_freeze_object</a>(uc);
@@ -565,7 +592,11 @@ title: Module `0xb::treasury`
 <summary>Implementation</summary>
 
 
-<pre><code><b>public</b>(<a href="../sui-framework/package.md#0x2_package">package</a>) <b>fun</b> <a href="treasury.md#0xb_treasury_mint">mint</a>&lt;T&gt;(self: &<b>mut</b> <a href="treasury.md#0xb_treasury_BridgeTreasury">BridgeTreasury</a>, amount: u64, ctx: &<b>mut</b> TxContext): Coin&lt;T&gt; {
+<pre><code><b>public</b>(<a href="../sui-framework/package.md#0x2_package">package</a>) <b>fun</b> <a href="treasury.md#0xb_treasury_mint">mint</a>&lt;T&gt;(
+    self: &<b>mut</b> <a href="treasury.md#0xb_treasury_BridgeTreasury">BridgeTreasury</a>,
+    amount: u64,
+    ctx: &<b>mut</b> TxContext,
+): Coin&lt;T&gt; {
     <b>let</b> <a href="treasury.md#0xb_treasury">treasury</a> = &<b>mut</b> self.treasuries[<a href="../move-stdlib/type_name.md#0x1_type_name_get">type_name::get</a>&lt;T&gt;()];
     <a href="../sui-framework/coin.md#0x2_coin_mint">coin::mint</a>(<a href="treasury.md#0xb_treasury">treasury</a>, amount, ctx)
 }
@@ -590,9 +621,14 @@ title: Module `0xb::treasury`
 <summary>Implementation</summary>
 
 
-<pre><code><b>public</b>(<a href="../sui-framework/package.md#0x2_package">package</a>) <b>fun</b> <a href="treasury.md#0xb_treasury_update_asset_notional_price">update_asset_notional_price</a>(self: &<b>mut</b> <a href="treasury.md#0xb_treasury_BridgeTreasury">BridgeTreasury</a>, token_id: u8, new_usd_price: u64) {
+<pre><code><b>public</b>(<a href="../sui-framework/package.md#0x2_package">package</a>) <b>fun</b> <a href="treasury.md#0xb_treasury_update_asset_notional_price">update_asset_notional_price</a>(
+    self: &<b>mut</b> <a href="treasury.md#0xb_treasury_BridgeTreasury">BridgeTreasury</a>,
+    token_id: u8,
+    new_usd_price: u64,
+) {
     <b>let</b> <a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a> = self.id_token_type_map.try_get(&token_id);
     <b>assert</b>!(<a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a>.is_some(), <a href="treasury.md#0xb_treasury_EUnsupportedTokenType">EUnsupportedTokenType</a>);
+    <b>assert</b>!(new_usd_price &gt; 0, <a href="treasury.md#0xb_treasury_EInvalidNotionalValue">EInvalidNotionalValue</a>);
     <b>let</b> <a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a> = <a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a>.destroy_some();
     <b>let</b> metadata = self.supported_tokens.get_mut(&<a href="../move-stdlib/type_name.md#0x1_type_name">type_name</a>);
     metadata.notional_value = new_usd_price;

diff --git a/crates/sui-framework/packages/bridge/sources/treasury.move b/crates/sui-framework/packages/bridge/sources/treasury.move
@@ -20,9 +20,10 @@ module bridge::treasury {
     use sui::vec_map;
     use sui::vec_map::VecMap;
 
-    const EUnsupportedTokenType: u64 = 0;
-    const EInvalidUpgradeCap: u64 = 1;
-    const ETokenSupplyNonZero: u64 = 2;
+    const EUnsupportedTokenType: u64 = 1;
+    const EInvalidUpgradeCap: u64 = 2;
+    const ETokenSupplyNonZero: u64 = 3;
+    const EInvalidNotionalValue: u64 = 4;
 
     #[test_only]
     const USD_VALUE_MULTIPLIER: u64 = 100000000; // 8 DP accuracy
@@ -92,22 +93,30 @@ module bridge::treasury {
     // Internal functions
     //
 
-    public(package) fun register_foreign_token<T>(self: &mut BridgeTreasury, tc: TreasuryCap<T>, uc: UpgradeCap, metadata: &CoinMetadata<T>) {
+    public(package) fun register_foreign_token<T>(
+        self: &mut BridgeTreasury, 
+        tc: TreasuryCap<T>, 
+        uc: UpgradeCap, 
+        metadata: &CoinMetadata<T>,
+    ) {
         // Make sure TreasuryCap has not been minted before.
         assert!(coin::total_supply(&tc) == 0, ETokenSupplyNonZero);
         let type_name = type_name::get<T>();
         let address_bytes = hex::decode(ascii::into_bytes(type_name::get_address(&type_name)));
         let coin_address = address::from_bytes(address_bytes);
         // Make sure upgrade cap is for the Coin package
         // FIXME: add test
-        assert!(object::id_to_address(&package::upgrade_package(&uc)) == coin_address, EInvalidUpgradeCap);
+        assert!(
+            object::id_to_address(&package::upgrade_package(&uc)) 
+                == coin_address, EInvalidUpgradeCap
+        );
         let registration = ForeignTokenRegistration {
             type_name,
             uc,
             decimal: coin::get_decimals(metadata),
         };
-        bag::add(&mut self.waiting_room, type_name::into_string(type_name), registration);
-        object_bag::add(&mut self.treasuries, type_name, tc);
+        self.waiting_room.add(type_name::into_string(type_name), registration);
+        self.treasuries.add(type_name, tc);
 
         emit(TokenRegistrationEvent{
             type_name,
@@ -116,21 +125,31 @@ module bridge::treasury {
         });
     }
 
-    public(package) fun add_new_token(self: &mut BridgeTreasury, token_name: String, token_id:u8, native_token: bool, notional_value: u64) {
+    public(package) fun add_new_token(
+        self: &mut BridgeTreasury, 
+        token_name: String, 
+        token_id:u8, 
+        native_token: bool, 
+        notional_value: u64,
+    ) {
         if (!native_token){
+            assert!(notional_value > 0, EInvalidNotionalValue);
             let ForeignTokenRegistration{
                 type_name,
                 uc,
                 decimal,
             } = bag::remove<String, ForeignTokenRegistration>(&mut self.waiting_room, token_name);
             let decimal_multiplier = math::pow(10, decimal);
-            vec_map::insert(&mut self.supported_tokens, type_name, BridgeTokenMetadata{
-                id: token_id,
-                decimal_multiplier,
-                notional_value,
-                native_token
-            });
-            vec_map::insert(&mut self.id_token_type_map, token_id, type_name);
+            self.supported_tokens.insert(
+                type_name, 
+                BridgeTokenMetadata{
+                    id: token_id,
+                    decimal_multiplier,
+                    notional_value,
+                    native_token
+                },
+            );
+            self.id_token_type_map.insert(token_id, type_name);
 
             // Freeze upgrade cap to prevent changes to the coin
             transfer::public_freeze_object(uc);
@@ -161,14 +180,23 @@ module bridge::treasury {
         coin::burn(treasury, token);
     }
 
-    public(package) fun mint<T>(self: &mut BridgeTreasury, amount: u64, ctx: &mut TxContext): Coin<T> {
+    public(package) fun mint<T>(
+        self: &mut BridgeTreasury, 
+        amount: u64, 
+        ctx: &mut TxContext,
+    ): Coin<T> {
         let treasury = &mut self.treasuries[type_name::get<T>()];
         coin::mint(treasury, amount, ctx)
     }
 
-    public(package) fun update_asset_notional_price(self: &mut BridgeTreasury, token_id: u8, new_usd_price: u64) {
+    public(package) fun update_asset_notional_price(
+        self: &mut BridgeTreasury, 
+        token_id: u8, 
+        new_usd_price: u64,
+    ) {
         let type_name = self.id_token_type_map.try_get(&token_id);
         assert!(type_name.is_some(), EUnsupportedTokenType);
+        assert!(new_usd_price > 0, EInvalidNotionalValue);
         let type_name = type_name.destroy_some();
         let metadata = self.supported_tokens.get_mut(&type_name);
         metadata.notional_value = new_usd_price;