Merge pull request djeck1432#201 from djeck1432/fix/minor-audit-updates

Audit small issues fixes

Scarb.lock

@@ -11,50 +11,123 @@ name = "ekubo"
 version = "0.1.0"
 source = "git+https://github.com/ekuboprotocol/abis?rev=edb6de8#edb6de8c9baf515f1053bbab3d86825d54a63bc3"
 
+[[package]]
+name = "openzeppelin"
+version = "0.19.0"
+source = "registry+https://scarbs.xyz/"
+checksum = "sha256:33174cc8f66cd2c1a527fd7f13a800dcb107d59f5c77e998e0c896a1da9cf1df"
+dependencies = [
+ "openzeppelin_access",
+ "openzeppelin_account",
+ "openzeppelin_finance",
+ "openzeppelin_governance",
+ "openzeppelin_introspection",
+ "openzeppelin_merkle_tree",
+ "openzeppelin_presets",
+ "openzeppelin_security",
+ "openzeppelin_token",
+ "openzeppelin_upgrades",
+ "openzeppelin_utils",
+]
+
+[[package]]
+name = "openzeppelin_access"
+version = "0.19.0"
+source = "registry+https://scarbs.xyz/"
+checksum = "sha256:0f5055ef443327bb613a56a812ccf31157abfd7d36a18739556f78b67f5b1116"
+dependencies = [
+ "openzeppelin_introspection",
+ "openzeppelin_utils",
+]
+
 [[package]]
 name = "openzeppelin_account"
-version = "0.18.0"
+version = "0.19.0"
 source = "registry+https://scarbs.xyz/"
-checksum = "sha256:83e6571cac4c67049c8d0ab4e3c7ad146d582d7605e7354248835833e1d26c4a"
+checksum = "sha256:0c92c856e44080e3280788d1c46f89ac707c64fa555eb02c343e492709a1ee50"
 dependencies = [
  "openzeppelin_introspection",
  "openzeppelin_utils",
 ]
 
+[[package]]
+name = "openzeppelin_finance"
+version = "0.19.0"
+source = "registry+https://scarbs.xyz/"
+checksum = "sha256:3d38c8aff02478431ddbb0538be5281a89eb159016105195bf6409bf6c3c4fc4"
+dependencies = [
+ "openzeppelin_access",
+ "openzeppelin_token",
+]
+
+[[package]]
+name = "openzeppelin_governance"
+version = "0.19.0"
+source = "registry+https://scarbs.xyz/"
+checksum = "sha256:fc6afb45e3cdcb5e843bbc80c6e12bb2536a34f557b74787c256872b86f2a81a"
+dependencies = [
+ "openzeppelin_access",
+ "openzeppelin_account",
+ "openzeppelin_introspection",
+ "openzeppelin_token",
+]
+
 [[package]]
 name = "openzeppelin_introspection"
-version = "0.18.0"
+version = "0.19.0"
+source = "registry+https://scarbs.xyz/"
+checksum = "sha256:a1dda07a91c447b83ccfcc4895897ec134917f0ff6d2ca876b93ea27466d7693"
+
+[[package]]
+name = "openzeppelin_merkle_tree"
+version = "0.19.0"
+source = "registry+https://scarbs.xyz/"
+checksum = "sha256:e7aaa00b9ea0f73938d3be6351aaa88efd21304bf6d5fd1b66c61e048a7a2375"
+
+[[package]]
+name = "openzeppelin_presets"
+version = "0.19.0"
 source = "registry+https://scarbs.xyz/"
-checksum = "sha256:46c4cc6c95c9baa4c7d5cc0ed2bdaf334f46c25a8c92b3012829fff936e3042b"
+checksum = "sha256:57d5c48724025072419c63a929903d71b949287338dc86d561e52b52d869c06f"
+dependencies = [
+ "openzeppelin_access",
+ "openzeppelin_account",
+ "openzeppelin_finance",
+ "openzeppelin_introspection",
+ "openzeppelin_token",
+ "openzeppelin_upgrades",
+ "openzeppelin_utils",
+]
 
 [[package]]
 name = "openzeppelin_security"
-version = "0.18.0"
+version = "0.19.0"
 source = "registry+https://scarbs.xyz/"
-checksum = "sha256:1db3a41e02ed48806587981340ed01ee7d552c3ad52cb33a6d81c1ed5cba9ee0"
+checksum = "sha256:0f1462d6de898cd28199cde0110304b4248fb19c7e788d4121d26c93b290e991"
 
 [[package]]
 name = "openzeppelin_token"
-version = "0.18.0"
+version = "0.19.0"
 source = "registry+https://scarbs.xyz/"
-checksum = "sha256:eafbe13f6a0487ce212459e25a81ae07f340ba76208ad4616626eb2d25a9625e"
+checksum = "sha256:9cba10f666ca6dd83b581367438d04b244bd5bbf0cfad6a28d193d373c0498b8"
 dependencies = [
+ "openzeppelin_access",
  "openzeppelin_account",
  "openzeppelin_introspection",
  "openzeppelin_utils",
 ]
 
 [[package]]
 name = "openzeppelin_upgrades"
-version = "0.18.0"
+version = "0.19.0"
 source = "registry+https://scarbs.xyz/"
-checksum = "sha256:33c9d0865364fc18a5e7b471fe53c3b0f3e0aec56a94f435089638fad2a4a35b"
+checksum = "sha256:3f2badf764a2219b0ea5b567b039daeb4c1707331f98e4f7b985ca2b562b4e10"
 
 [[package]]
 name = "openzeppelin_utils"
-version = "0.18.0"
+version = "0.19.0"
 source = "registry+https://scarbs.xyz/"
-checksum = "sha256:725b212839f3eddc32791408609099c5e808c167ca0cf331d8c1d778b07a4e21"
+checksum = "sha256:0e0e6f6b20b3c4075b92941a2c124430a59f1c207f8fbdfd56ce9239e6d666a8"
 
 [[package]]
 name = "pragma_lib"
@@ -80,9 +153,7 @@ version = "0.1.0"
 dependencies = [
  "alexandria_math",
  "ekubo",
- "openzeppelin_security",
- "openzeppelin_token",
- "openzeppelin_upgrades",
+ "openzeppelin",
  "pragma_lib",
  "snforge_std",
 ]

Scarb.toml

@@ -10,9 +10,7 @@ cairo-version = "2.8.2"
 starknet = "2.8.2"
 ekubo = { git = "https://github.com/ekuboprotocol/abis", rev = "edb6de8" }
 alexandria_math = { git = "https://github.com/keep-starknet-strange/alexandria.git", rev = "8208871" }
-openzeppelin_token = "0.18.0"
-openzeppelin_security = "0.18.0"
-openzeppelin_upgrades = "0.18.0"
+openzeppelin = "0.19.0"
 
 [dev-dependencies]
 pragma_lib = { git = "https://github.com/astraly-labs/pragma-lib", tag = "2.8.2" }

src/deposit.cairo

@@ -8,10 +8,11 @@ mod Deposit {
         types::{i129::i129, keys::PoolKey}
     };
 
-    use openzeppelin_security::ReentrancyGuardComponent;
-
-    use openzeppelin_token::erc20::interface::{ERC20ABIDispatcher, ERC20ABIDispatcherTrait};
-    use openzeppelin_upgrades::{UpgradeableComponent, interface::IUpgradeable};
+    use openzeppelin::{
+        security::ReentrancyGuardComponent,
+        token::erc20::interface::{ERC20ABIDispatcher, ERC20ABIDispatcherTrait},
+        upgrades::{UpgradeableComponent, interface::IUpgradeable}, access::ownable::OwnableComponent
+    };
     use spotnet::{
         constants::{ZK_SCALE_DECIMALS, STRK_ADDRESS},
         interfaces::{
@@ -33,18 +34,25 @@ mod Deposit {
         path: ReentrancyGuardComponent, storage: reentrancy_guard, event: ReentrancyGuardEvent
     );
     component!(path: UpgradeableComponent, storage: upgradeable, event: UpgradeableEvent);
+    component!(path: OwnableComponent, storage: ownable, event: OwnableEvent);
+
+    impl OwnableInternalImpl = OwnableComponent::InternalImpl<ContractState>;
+    #[abi(embed_v0)]
+    impl OwnableTwoStepMixinImpl =
+        OwnableComponent::OwnableTwoStepMixinImpl<ContractState>;
 
     impl ReentrancyInternalImpl = ReentrancyGuardComponent::InternalImpl<ContractState>;
     impl UpgradeableInternalImpl = UpgradeableComponent::InternalImpl<ContractState>;
 
     #[storage]
     struct Storage {
-        owner: ContractAddress,
         ekubo_core: ICoreDispatcher,
         zk_market: IMarketDispatcher,
         treasury: ContractAddress,
         is_position_open: bool,
         #[substorage(v0)]
+        ownable: OwnableComponent::Storage,
+        #[substorage(v0)]
         reentrancy_guard: ReentrancyGuardComponent::Storage,
         #[substorage(v0)]
         upgradeable: UpgradeableComponent::Storage
@@ -59,7 +67,7 @@ mod Deposit {
         treasury: ContractAddress
     ) {
         assert(owner.is_non_zero(), 'Owner address is zero');
-        self.owner.write(owner);
+        self.ownable.initializer(owner);
         self.ekubo_core.write(ekubo_core);
         self.zk_market.write(zk_market);
         self.treasury.write(treasury);
@@ -93,7 +101,8 @@ mod Deposit {
         supply_decimals: DecimalScale,
         debt_decimals: DecimalScale
     ) -> u256 {
-        let deposited = ((total_deposited * ZK_SCALE_DECIMALS * supply_token_price.into()) / supply_decimals.into());
+        let deposited = ((total_deposited * ZK_SCALE_DECIMALS * supply_token_price.into())
+            / supply_decimals.into());
         let free_amount = (((deposited * collateral_factor.into() / ZK_SCALE_DECIMALS)
             * borrow_factor.into()
             / ZK_SCALE_DECIMALS))
@@ -121,11 +130,27 @@ mod Deposit {
         repaid_amount: TokenAmount
     }
 
+    #[derive(starknet::Event, Drop)]
+    struct Withdraw {
+        token: ContractAddress,
+        amount: TokenAmount
+    }
+
+    #[derive(starknet::Event, Drop)]
+    struct ExtraDeposit {
+        token: ContractAddress,
+        amount: TokenAmount
+    }
+
     #[event]
     #[derive(Drop, starknet::Event)]
     enum Event {
         LiquidityLooped: LiquidityLooped,
         PositionClosed: PositionClosed,
+        Withdraw: Withdraw,
+        ExtraDeposit: ExtraDeposit,
+        #[flat]
+        OwnableEvent: OwnableComponent::Event,
         #[flat]
         ReentrancyGuardEvent: ReentrancyGuardComponent::Event,
         #[flat]
@@ -165,7 +190,7 @@ mod Deposit {
             pool_price: TokenPrice
         ) {
             let user_account = get_tx_info().unbox().account_contract_address;
-            assert(user_account == self.owner.read(), 'Caller is not the owner');
+            assert(user_account == self.ownable.owner(), 'Caller is not the owner');
             assert(!self.is_position_open.read(), 'Open position already exists');
             let DepositData { token, amount, multiplier, borrow_portion_percent } = deposit_data;
             assert(
@@ -193,7 +218,7 @@ mod Deposit {
                 (false, pool_key.token0, ekubo_limits.lower)
             };
 
-            token_dispatcher.transferFrom(self.owner.read(), curr_contract_address, amount);
+            token_dispatcher.transferFrom(self.ownable.owner(), curr_contract_address, amount);
             let (deposit_reserve_data, debt_reserve_data) = (
                 zk_market.get_reserve_data(token), zk_market.get_reserve_data(borrowing_token)
             );
@@ -203,6 +228,11 @@ mod Deposit {
                 debt_reserve_data.borrow_factor.into()
             );
 
+            assert(
+                deposit_reserve_data.enabled && debt_reserve_data.enabled,
+                'Reserves must be enabled'
+            );
+
             zk_market.enable_collateral(token);
 
             token_dispatcher.approve(zk_market.contract_address, amount);
@@ -293,7 +323,7 @@ mod Deposit {
             debt_price: TokenPrice
         ) {
             assert(
-                get_tx_info().unbox().account_contract_address == self.owner.read(),
+                get_tx_info().unbox().account_contract_address == self.ownable.owner(),
                 'Caller is not the owner'
             );
             assert(self.is_position_open.read(), 'Open position not exists');
@@ -309,6 +339,11 @@ mod Deposit {
                 debt_reserve_data.borrow_factor.into()
             );
 
+            assert(
+                deposit_reserve_data.enabled && debt_reserve_data.enabled,
+                'Reserves must be enabled'
+            );
+
             let z_token_disp = ERC20ABIDispatcher {
                 contract_address: deposit_reserve_data.z_token_address
             };
@@ -386,7 +421,7 @@ mod Deposit {
             zk_market.disable_collateral(supply_token);
             self.is_position_open.write(false);
             let withdrawn_amount = token_disp.balanceOf(contract_address);
-            token_disp.transfer(self.owner.read(), withdrawn_amount);
+            token_disp.transfer(self.ownable.owner(), withdrawn_amount);
             self
                 .emit(
                     PositionClosed {
@@ -465,10 +500,11 @@ mod Deposit {
             token_dispatcher.approve(zk_market.contract_address, amount);
             zk_market.enable_collateral(token);
             zk_market.deposit(token, amount.try_into().unwrap());
+            self.emit(ExtraDeposit { token, amount });
             self.reentrancy_guard.end();
         }
 
-        /// Withdraws tokens from zkLend if looped tokens are repaid
+        /// Withdraws tokens from zkLend
         ///
         /// # Panics
         /// address of account that started the transaction is not equal to `owner` storage variable
@@ -477,19 +513,25 @@ mod Deposit {
         /// `token`: TokenAddress - token address to withdraw from zkLend
         /// `amount`: TokenAmount - amount to withdraw. Pass `0` to withdraw all
         fn withdraw(ref self: ContractState, token: ContractAddress, amount: TokenAmount) {
-            assert(get_caller_address() == self.owner.read(), 'Caller is not the owner');
+            self.ownable.assert_only_owner();
             let zk_market = self.zk_market.read();
+
             let token_dispatcher = ERC20ABIDispatcher { contract_address: token };
+            let mut withdrawn_amount = amount;
             if amount == 0 {
+                let current_contract = get_contract_address();
+                let initial_balance = ERC20ABIDispatcher { contract_address: token }
+                    .balanceOf(current_contract);
                 zk_market.withdraw_all(token);
-                token_dispatcher
-                    .transfer(
-                        self.owner.read(), token_dispatcher.balanceOf(get_contract_address())
-                    );
+                let new_balance = ERC20ABIDispatcher { contract_address: token }
+                    .balanceOf(current_contract);
+                withdrawn_amount = new_balance - initial_balance;
+                token_dispatcher.transfer(self.ownable.owner(), new_balance);
             } else {
                 zk_market.withdraw(token, amount.try_into().unwrap());
-                token_dispatcher.transfer(self.owner.read(), amount);
+                token_dispatcher.transfer(self.ownable.owner(), amount);
             };
+            self.emit(Withdraw { token, amount: withdrawn_amount });
         }
     }
 
@@ -520,7 +562,7 @@ mod Deposit {
     impl UpgradeableImpl of IUpgradeable<ContractState> {
         fn upgrade(ref self: ContractState, new_class_hash: ClassHash) {
             // This function can only be called by the owner
-            assert(get_caller_address() == self.owner.read(), 'Caller is not the owner');
+            self.ownable.assert_only_owner();
 
             self.upgradeable.upgrade(new_class_hash);
         }

src/types.cairo

@@ -40,7 +40,7 @@ pub struct Claim {
 
 #[derive(Drop, Serde, starknet::Store)]
 pub struct MarketReserveData {
-    enabled: bool,
+    pub enabled: bool,
     pub decimals: felt252,
     pub z_token_address: ContractAddress,
     interest_rate_model: ContractAddress,

tests/test_defispring.cairo

@@ -92,7 +92,7 @@ fn test_claim_and_withdraw() {
     let storage_entry_for_hypothetical_owner = array![HYPOTHETICAL_OWNER_ADDR].span();
     store(
         address_eligible_for_zklend_rewards,
-        selector!("owner"),
+        selector!("Ownable_owner"),
         storage_entry_for_hypothetical_owner
     );