From cf06ea695b1706b168f085d88b6cd5b1d9777b0b Mon Sep 17 00:00:00 2001 From: faurdent Date: Wed, 13 Nov 2024 16:58:32 +0100 Subject: [PATCH] New events, two step ownership and assertions --- src/deposit.cairo | 80 ++++++++++++++++++++------- src/types.cairo | 2 +- tests/test_defispring.cairo | 2 +- tests/test_loop.cairo | 104 ++++++++++++++++++++++++++++++++---- 4 files changed, 156 insertions(+), 32 deletions(-) diff --git a/src/deposit.cairo b/src/deposit.cairo index 6209aac6..37de33f0 100644 --- a/src/deposit.cairo +++ b/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; + #[abi(embed_v0)] + impl OwnableTwoStepMixinImpl = + OwnableComponent::OwnableTwoStepMixinImpl; impl ReentrancyInternalImpl = ReentrancyGuardComponent::InternalImpl; impl UpgradeableInternalImpl = UpgradeableComponent::InternalImpl; #[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 { 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); } diff --git a/src/types.cairo b/src/types.cairo index be93bc5d..61df23ba 100644 --- a/src/types.cairo +++ b/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, diff --git a/tests/test_defispring.cairo b/tests/test_defispring.cairo index 98016f3e..55fd4615 100644 --- a/tests/test_defispring.cairo +++ b/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 ); diff --git a/tests/test_loop.cairo b/tests/test_loop.cairo index dd2654db..08698def 100644 --- a/tests/test_loop.cairo +++ b/tests/test_loop.cairo @@ -2,7 +2,10 @@ use alexandria_math::fast_power::fast_power; use core::panic_with_felt252; use ekubo::interfaces::core::{ICoreDispatcher, ICoreDispatcherTrait}; use ekubo::types::keys::{PoolKey}; -use openzeppelin_token::erc20::interface::{ERC20ABIDispatcher, ERC20ABIDispatcherTrait}; +use openzeppelin::access::ownable::interface::{ + OwnableTwoStepABIDispatcherTrait, OwnableTwoStepABIDispatcher +}; +use openzeppelin::token::erc20::interface::{ERC20ABIDispatcher, ERC20ABIDispatcherTrait}; use pragma_lib::abi::{IPragmaABIDispatcher, IPragmaABIDispatcherTrait}; use pragma_lib::types::{AggregationMode, DataType, PragmaPricesResponse}; @@ -263,7 +266,13 @@ fn test_loop_unauthorized() { + ERC20ABIDispatcher { contract_address: usdc_addr }.decimals()) .into() ); - let pool_price = ((1 * ZK_SCALE_DECIMALS * decimals_sum_power.into() / get_asset_price_pragma('ETH/USD').into()) / ZK_SCALE_DECIMALS).try_into().unwrap(); + let pool_price = ((1 + * ZK_SCALE_DECIMALS + * decimals_sum_power.into() + / get_asset_price_pragma('ETH/USD').into()) + / ZK_SCALE_DECIMALS) + .try_into() + .unwrap(); let disp = get_deposit_dispatcher(user); @@ -308,7 +317,13 @@ fn test_loop_position_exists() { (ERC20ABIDispatcher { contract_address: eth_addr }.decimals() + token_disp.decimals()) .into() ); - let pool_price = ((1 * ZK_SCALE_DECIMALS * decimals_sum_power.into() / get_asset_price_pragma('ETH/USD').into()) / ZK_SCALE_DECIMALS).try_into().unwrap(); + let pool_price = ((1 + * ZK_SCALE_DECIMALS + * decimals_sum_power.into() + / get_asset_price_pragma('ETH/USD').into()) + / ZK_SCALE_DECIMALS) + .try_into() + .unwrap(); let deposit_disp = get_deposit_dispatcher(user); start_cheat_caller_address(usdc_addr.try_into().unwrap(), user); token_disp.approve(deposit_disp.contract_address, 60000000); @@ -429,7 +444,13 @@ fn test_close_position_usdc_valid_time_passed() { (ERC20ABIDispatcher { contract_address: eth_addr }.decimals() + token_disp.decimals()) .into() ); - let pool_price = ((1 * ZK_SCALE_DECIMALS * decimals_sum_power.into() / get_asset_price_pragma('ETH/USD').into()) / ZK_SCALE_DECIMALS).try_into().unwrap(); + let pool_price = ((1 + * ZK_SCALE_DECIMALS + * decimals_sum_power.into() + / get_asset_price_pragma('ETH/USD').into()) + / ZK_SCALE_DECIMALS) + .try_into() + .unwrap(); let deposit_disp = get_deposit_dispatcher(user); start_cheat_caller_address(usdc_addr.try_into().unwrap(), user); @@ -500,7 +521,8 @@ fn test_close_position_amounts_cleared() { (ERC20ABIDispatcher { contract_address: eth_addr }.decimals() + token_disp.decimals()) .into() ); - let pool_price = (1 * ZK_SCALE_DECIMALS * decimals_sum_power.into() / quote_token_price.into()) / ZK_SCALE_DECIMALS; + let pool_price = (1 * ZK_SCALE_DECIMALS * decimals_sum_power.into() / quote_token_price.into()) + / ZK_SCALE_DECIMALS; let deposit_disp = get_deposit_dispatcher(user); start_cheat_caller_address(usdc_addr.try_into().unwrap(), user); @@ -575,7 +597,10 @@ fn test_close_position_partial_debt_utilization() { (ERC20ABIDispatcher { contract_address: usdc_addr }.decimals() + token_disp.decimals()) .into() ); - let quote_token_price = ((1 * ZK_SCALE_DECIMALS * decimals_sum_power.into() / pool_price.into()) / ZK_SCALE_DECIMALS).try_into().unwrap(); + let quote_token_price = ((1 * ZK_SCALE_DECIMALS * decimals_sum_power.into() / pool_price.into()) + / ZK_SCALE_DECIMALS) + .try_into() + .unwrap(); let deposit_disp = get_deposit_dispatcher(user); @@ -651,7 +676,10 @@ fn test_extra_deposit_valid() { (ERC20ABIDispatcher { contract_address: eth_addr }.decimals() + token_disp.decimals()) .into() ); - let pool_price = ((1 * ZK_SCALE_DECIMALS * decimals_sum_power.into() / quote_token_price) / ZK_SCALE_DECIMALS).try_into().unwrap(); + let pool_price = ((1 * ZK_SCALE_DECIMALS * decimals_sum_power.into() / quote_token_price) + / ZK_SCALE_DECIMALS) + .try_into() + .unwrap(); let deposit_disp = get_deposit_dispatcher(user); start_cheat_caller_address(usdc_addr.try_into().unwrap(), user); @@ -662,7 +690,7 @@ fn test_extra_deposit_valid() { deposit_disp .loop_liquidity( DepositData { - token: usdc_addr, amount: 1000000000, multiplier: 4, borrow_portion_percent: 98 + token: usdc_addr, amount: 1000000000, multiplier: 4, borrow_portion_percent: 95 }, pool_key, get_slippage_limits(pool_key), @@ -721,7 +749,10 @@ fn test_extra_deposit_supply_token_close_position_fuzz(extra_amount: u32) { (ERC20ABIDispatcher { contract_address: eth_addr }.decimals() + token_disp.decimals()) .into() ); - let pool_price = 1 * decimals_sum_power.into() / quote_token_price; + let pool_price = ((1 * ZK_SCALE_DECIMALS * decimals_sum_power.into() / quote_token_price) + / ZK_SCALE_DECIMALS) + .try_into() + .unwrap(); let deposit_disp = get_deposit_dispatcher(user); start_cheat_caller_address(usdc_addr.try_into().unwrap(), user); @@ -767,7 +798,7 @@ fn test_extra_deposit_supply_token_close_position_fuzz(extra_amount: u32) { get_slippage_limits(pool_key), 95, pool_price, - quote_token_price + quote_token_price.try_into().unwrap() ); stop_cheat_account_contract_address(deposit_disp.contract_address); @@ -810,7 +841,13 @@ fn test_withdraw_valid_fuzz(amount: u32) { (ERC20ABIDispatcher { contract_address: eth_addr }.decimals() + token_disp.decimals()) .into() ); - let pool_price = ((1 * ZK_SCALE_DECIMALS * decimals_sum_power.into() / get_asset_price_pragma('ETH/USD').into()) / ZK_SCALE_DECIMALS).try_into().unwrap(); + let pool_price = ((1 + * ZK_SCALE_DECIMALS + * decimals_sum_power.into() + / get_asset_price_pragma('ETH/USD').into()) + / ZK_SCALE_DECIMALS) + .try_into() + .unwrap(); let deposit_disp = get_deposit_dispatcher(user); start_cheat_caller_address(usdc_addr.try_into().unwrap(), user); @@ -1001,3 +1038,48 @@ fn test_withdraw_position_open() { deposit_disp.withdraw(eth_addr, 100000000000000); stop_cheat_caller_address(deposit_disp.contract_address); } + +#[test] +fn test_transfer_ownership_valid() { + let user = 0x123.try_into().unwrap(); + let new_owner = 0x456.try_into().unwrap(); + let deposit_disp = get_deposit_dispatcher(user); + let ownable_disp = OwnableTwoStepABIDispatcher { + contract_address: deposit_disp.contract_address + }; + start_cheat_caller_address(deposit_disp.contract_address, user); + ownable_disp.transfer_ownership(new_owner); + stop_cheat_caller_address(deposit_disp.contract_address); + + start_cheat_caller_address(deposit_disp.contract_address, new_owner); + ownable_disp.accept_ownership(); + stop_cheat_caller_address(deposit_disp.contract_address); + + assert(ownable_disp.owner() == new_owner, 'Owner did not change'); +} + +#[test] +#[should_panic(expected: 'Caller is not the pending owner')] +fn test_transfer_renounced_ownership() { + let user = 0x123.try_into().unwrap(); + let new_owner = 0x456.try_into().unwrap(); + let deposit_disp = get_deposit_dispatcher(user); + let ownable_disp = OwnableTwoStepABIDispatcher { + contract_address: deposit_disp.contract_address + }; + start_cheat_caller_address(deposit_disp.contract_address, user); + ownable_disp.transfer_ownership(new_owner); + stop_cheat_caller_address(deposit_disp.contract_address); + + assert(ownable_disp.pending_owner() == new_owner, 'Pending owner is incorrect'); + + start_cheat_caller_address(deposit_disp.contract_address, user); + ownable_disp.transfer_ownership(user); + stop_cheat_caller_address(deposit_disp.contract_address); + + start_cheat_caller_address(deposit_disp.contract_address, new_owner); + ownable_disp.accept_ownership(); + stop_cheat_caller_address(deposit_disp.contract_address); + + assert(ownable_disp.owner() == new_owner, 'Owner did not change'); +}