hats-finance / Kintsu-0x7d70f9442af3a9a0a734fa6a1b4857f25518e9d2

Smart contracts for Kintsu
Other
0 stars 0 forks source link

`set_code()` function does not follow the `ink!` recommended implementation #19

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: @0xRizwan Twitter username: 0xRizwann Submission hash (on-chain): 0xffcdc269a83613bec3c086eaa67aa2991a71f3f1defe1a863153e0a9eb68b770 Severity: low

Description: Description\

set_code() is used to upgrade the ink! contract which is done by using ink env set_code_hash function. set_code() in Vault contract is implemented as :

        #[ink(message)]
        pub fn set_code(&mut self, code_hash: [u8; 32]) -> Result<(), VaultError> {
            let caller = Self::env().caller();

            if caller != self.data.role_owner {
                return Err(VaultError::InvalidPermissions);
            }

            ink::env::set_code_hash(&code_hash)?;

            Ok(())
        }

This function can only be accessed by contract owner. The issue is that, when the set_code() fails it wont revert with error message. This function also does not follow the officialn ink! implementation which is given in recommendation.

Recommendation to fix\ Check at https://use.ink/4.x/basics/upgradeable-contracts#example-2, to know how ink! has recommended to implement the set_code() function.

0xRizwan commented 1 month ago

Recommendation to fix:

-        #[ink(message)]
-        pub fn set_code(&mut self, code_hash: [u8; 32]) -> Result<(), VaultError> {
-            let caller = Self::env().caller();
-
-            if caller != self.data.role_owner {
-                return Err(VaultError::InvalidPermissions);
-            }
-
-            ink::env::set_code_hash(&code_hash)?;
-
-            Ok(())
-        }

+ #[ink(message)]
+ fn set_code(&mut self, code_hash: [u8; 32]) {
+            let caller = Self::env().caller();
+
+            if caller != self.data.role_owner {
+                return Err(VaultError::InvalidPermissions);
+            }

+    ink::env::set_code_hash(&code_hash).unwrap_or_else(|err| {
+        panic!(
+            "Failed to `set_code_hash` to {:?} due to {:?}",
+            code_hash, err
+        )
+    });
+    ink::env::debug_println!("Switched code hash to {:?}.", code_hash);
}
0xRizwan commented 1 month ago

Another issue instance in nomination_agent's set_code() which is partially as per ink! recommendation.

Ensure set_code() complelely as per ink! recommendation

0xmahdirostami commented 1 month ago

Thank you for your submission. Design choice, there is no issue here.