From 6fcc18b4d600821fcb814f62c466eb3a39ff52ba Mon Sep 17 00:00:00 2001 From: Emmanuel Ozeh Date: Thu, 11 Jul 2024 03:43:33 +0100 Subject: [PATCH 1/3] Abstract out Liquidation function from Vault module --- src/interfaces/ILiquidator.sol | 39 +++++++++ src/interfaces/IVault.sol | 6 +- src/liquidator.sol | 88 +++++++++++++++++++ src/vault.sol | 151 ++++++++++++++++++++------------- 4 files changed, 218 insertions(+), 66 deletions(-) create mode 100644 src/interfaces/ILiquidator.sol create mode 100644 src/liquidator.sol diff --git a/src/interfaces/ILiquidator.sol b/src/interfaces/ILiquidator.sol new file mode 100644 index 0000000..eb77848 --- /dev/null +++ b/src/interfaces/ILiquidator.sol @@ -0,0 +1,39 @@ +//SPDX-Licence-Identifier: GPL-3.0 +pragma solidity 0.8.21; + +import {Vault} from "../vault.sol"; +import {ERC20Token} from "../mocks/ERC20Token.sol"; + +interface ILiquidator { + // ------------------------------------------------ CUSTOM ERRORS ------------------------------------------------ + error CollateralDoesNotExist(); + error PositionIsSafe(); + error CollateralRatioNotImproved(); + + // ------------------------------------------------ EVENTS ------------------------------------------------ + event Liquidated( + address indexed vault, address indexed owner, address liquidator, uint256 currencyAmountPaid, uint256 collateralAmountCovered + ); + + /** + * @notice liquidates a vault making sure the liquidation strictly improves the collateral ratio i.e doesn't leave it the same as before or decreases it (if that's possible) + * + * @param _vaultID contract address of vault to be liquidated + * @param _collateralToken contract address of collateral used by vault that is to be liquidate, also the token to recieved by the `_to` address after liquidation + * @param _owner owner of the vault to liquidate + * @param _to address to send the liquidated collateral (collateral covered) to + * @param _currencyAmountToPay the amount of currency tokens to pay back for `_owner` + * + * @dev updates fees accrued for `_owner`'s vault since last fee update, this is important as it ensures that the collateral-ratio check at the start and end of the function uses an updated total owed amount i.e (borrowedAmount + accruedFees) when checking `_owner`'s collateral-ratio + * @dev should revert if the collateral does not exist + * should revert if the vault is not under-water + * should revert if liqudiation did not strictly imporve the collateral ratio of the vault + */ + function liquidate( + Vault _vaultID, + ERC20Token _collateralToken, + address _owner, + address _to, + uint256 _currencyAmountToPay + ) external; +} \ No newline at end of file diff --git a/src/interfaces/IVault.sol b/src/interfaces/IVault.sol index 5d885c7..7e7b34c 100644 --- a/src/interfaces/IVault.sol +++ b/src/interfaces/IVault.sol @@ -6,13 +6,11 @@ interface IVault { error ZeroAddress(); error UnrecognizedParam(); error BadCollateralRatio(); - error PositionIsSafe(); error ZeroCollateral(); error TotalUserCollateralBelowFloor(); error CollateralAlreadyExists(); error CollateralDoesNotExist(); error NotOwnerOrReliedUpon(); - error CollateralRatioNotImproved(); error NotEnoughCollateralToPay(); error EthTransferFailed(); error GlobalDebtCeilingExceeded(); @@ -20,6 +18,7 @@ interface IVault { error InsufficientCurrencyAmountToPay(); error InvalidStabilityModule(); error NotFeedContract(); + error NotLiquidatorContract(); // ------------------------------------------------ EVENTS ------------------------------------------------ event CollateralTypeAdded(address collateralAddress); @@ -28,9 +27,6 @@ interface IVault { event CurrencyMinted(address indexed owner, uint256 amount); event CurrencyBurned(address indexed owner, uint256 amount); event FeesPaid(address indexed owner, uint256 amount); - event Liquidated( - address indexed owner, address liquidator, uint256 currencyAmountPaid, uint256 collateralAmountCovered - ); // ------------------------------------------------ CUSTOM TYPES ------------------------------------------------ struct RateInfo { diff --git a/src/liquidator.sol b/src/liquidator.sol new file mode 100644 index 0000000..156f5d8 --- /dev/null +++ b/src/liquidator.sol @@ -0,0 +1,88 @@ +//SPDX-License-Identifier: GPL-3.0 +pragma solidity 0.8.21; + +import {Ownable} from "solady/auth/Ownable.sol"; +import {ERC20Token} from "./mocks/ERC20Token.sol"; +import {Currency} from "./currency.sol"; +import {Vault} from "./vault.sol"; +import {ILiquidator} from "./interfaces/ILiquidator.sol"; + +contract Liquidator is ILiquidator, Ownable { + + constructor() { + _initializeOwner(msg.sender); + } + + /** + * @notice liquidates a vault making sure the liquidation strictly improves the collateral ratio i.e doesn't leave it the same as before or decreases it (if that's possible) + * + * @param _vault contract address of vault to be liquidated + * @param _collateralToken contract address of collateral used by vault that is to be liquidate, also the token to recieved by the `_to` address after liquidation + * @param _owner owner of the vault to liquidate + * @param _to address to send the liquidated collateral (collateral covered) to + * @param _currencyAmountToPay the amount of currency tokens to pay back for `_owner` + * + * @dev updates fees accrued for `_owner`'s vault since last fee update, this is important as it ensures that the collateral-ratio check at the start and end of the function uses an updated total owed amount i.e (borrowedAmount + accruedFees) when checking `_owner`'s collateral-ratio + * @dev should revert if the collateral does not exist + * should revert if the vault is not under-water + * should revert if liqudiation did not strictly imporve the collateral ratio of the vault + */ + function liquidate( + Vault _vault, + ERC20Token _collateralToken, + address _owner, + address _to, + uint256 _currencyAmountToPay + ) external { + // get collateral ratio + // require it's below liquidation threshold + // liquidate and take discount + // burn currency from caller + if (_vault.getCollateralRate(_collateralToken) == 0) { + revert CollateralDoesNotExist(); + } + + // need to accrue fees first in order to use updated fees for collateral ratio calculation below + _vault.accrueLiquidationFees(_collateralToken, _owner); + + (uint256 _preCollateralRatio, uint256 _liquidationThreshold) = _vault.getCollateralRatioAndLiquidationThreshold(_collateralToken, _owner); + + if (_preCollateralRatio <= _liquidationThreshold) { + revert PositionIsSafe(); + } + + (uint256 _depositedCollateral, uint256 _borrowedAmount, uint256 _accruedFees, ) = _vault.vaultMapping(_collateralToken, _owner); + + if (_currencyAmountToPay == type(uint256).max) { + // This is here to prevent frontrunning of full liquidation + // malicious owners can monitor the mempool and frontrun any attempt to liquidate their position by liquidating it + // themselves but partially, (by 1 wei of collateral is enough) which causes underflow when the liquidator's tx is to be executed' + // With this, liquidators can parse in type(uint256).max to liquidate everything regardless of the current borrowed amount. + _currencyAmountToPay = _borrowedAmount + _accruedFees; + } + + uint256 _collateralAmountCovered = _vault.getCollateralAmountFromCurrencyValue(_collateralToken, _currencyAmountToPay); + + (,,, uint256 _liquidationBonus,,,,,) = _vault.collateralMapping(_collateralToken); + uint256 _bonus = (_collateralAmountCovered * _liquidationBonus) / _vault.hundredPercentage(); + uint256 _total = _collateralAmountCovered + _bonus; + + // To make liquidations always possible, if _vault.depositedCollateral not enough to pay bonus, give out highest possible bonus + // For situations where the user's vault is insolvent, this would be called by the system stability module after a debt auction is used to raise the currency + if (_total > _depositedCollateral) { + _total = _depositedCollateral; + } + + emit Liquidated(address(_vault), _owner, msg.sender, _currencyAmountToPay, _total); + + _vault.withdrawCollateralL(_collateralToken, _owner, _to, _total); + _vault.burnCurrencyL(_collateralToken, _owner, msg.sender, _currencyAmountToPay); + + // collateral ratio must never increase or stay the same during a liquidation. + (uint256 _postCollateralRatio, ) = _vault.getCollateralRatioAndLiquidationThreshold(_collateralToken, _owner); + + if (_preCollateralRatio <= _postCollateralRatio) { + revert CollateralRatioNotImproved(); + } + } +} diff --git a/src/vault.sol b/src/vault.sol index bf41864..2a63621 100644 --- a/src/vault.sol +++ b/src/vault.sol @@ -10,6 +10,7 @@ import {Currency} from "./currency.sol"; import {Pausable} from "./helpers/pausable.sol"; import {ERC20Token} from "./mocks/ERC20Token.sol"; import {IRate} from "./interfaces/IRate.sol"; +import {Liquidator} from "./liquidator.sol"; contract Vault is IVault, Ownable, Pausable { uint256 private constant PRECISION_DEGREE = 18; @@ -25,6 +26,8 @@ contract Vault is IVault, Ownable, Pausable { address public feedModule; // feed contract address IRate public rateModule; // rate calculation module + Liquidator public liquidator; // liquidator contract address + // Global parameters RateInfo public baseRateInfo; // base rate info uint256 public debtCeiling; // global debt ceiling @@ -37,7 +40,7 @@ contract Vault is IVault, Ownable, Pausable { mapping(ERC20Token => mapping(address => VaultInfo)) public vaultMapping; // collateral address => user address => vault data mapping(address => mapping(address => bool)) public relyMapping; // borrower -> addresss -> is allowed to take actions on borrowers vaults on their behalf - constructor(Currency _currencyToken, uint256 _baseRate, uint256 _debtCeiling) { + constructor(Currency _currencyToken, uint256 _baseRate, uint256 _debtCeiling, Liquidator _liquidatorContract) { _initializeOwner(msg.sender); CURRENCY_TOKEN = _currencyToken; @@ -45,6 +48,7 @@ contract Vault is IVault, Ownable, Pausable { baseRateInfo.rate = _baseRate; debtCeiling = _debtCeiling; + liquidator = _liquidatorContract; } /** @@ -76,6 +80,16 @@ contract Vault is IVault, Ownable, Pausable { _; } + /** + * @dev reverts if the msg.sender is not the 'liquidator' contract + */ + modifier onlyLiquidator() { + if (address(liquidator) != msg.sender) { + revert NotLiquidatorContract(); + } + _; + } + /** * @notice allows interactions with functions having `whenNotPaused` modifier and prevents interactions with ones with the `whenPaused` modifier * @@ -454,66 +468,94 @@ contract Vault is IVault, Ownable, Pausable { _burnCurrency(_collateral, _vault, _owner, msg.sender, _amount); } + // ------------------------------------------------ LIQUIDATOR-ONLY FUNCTIONS------------------------------------------------ /** - * @notice liquidates a vault making sure the liquidation strictly improves the collateral ratio i.e doesn't leave it the same as before or decreases it (if that's possible) - * - * @param _collateralToken contract address of collateral used by vault that is to be liquidate, also the token to recieved by the `_to` address after liquidation - * @param _owner owner of the vault to liquidate - * @param _to address to send the liquidated collateral (collateral covered) to - * @param _currencyAmountToPay the amount of currency tokens to pay back for `_owner` - * - * @dev updates fees accrued for `_owner`'s vault since last fee update, this is important as it ensures that the collateral-ratio check at the start and end of the function uses an updated total owed amount i.e (borrowedAmount + accruedFees) when checking `_owner`'s collateral-ratio - * @dev should revert if the collateral does not exist - * should revert if the vault is not under-water - * should revert if liqudiation did not strictly imporve the collateral ratio of the vault + * @dev increments accrued fees of a vault by it's accrued fees since it's`_vault.lastUpdateTime`. + * @dev should never revert! + * @dev exposes '_accrueFees' for external access by 'liquidator' contract + * @dev only 'liquidator' contract can call */ - function liquidate(ERC20Token _collateralToken, address _owner, address _to, uint256 _currencyAmountToPay) - external - collateralExists(_collateralToken) - { - // get collateral ratio - // require it's below liquidation threshold - // liquidate and take discount - // burn currency from caller - + function accrueLiquidationFees(ERC20Token _collateralToken, address _owner) external onlyLiquidator { VaultInfo storage _vault = vaultMapping[_collateralToken][_owner]; CollateralInfo storage _collateral = collateralMapping[_collateralToken]; - // need to accrue fees first in order to use updated fees for collateral ratio calculation below _accrueFees(_collateral, _vault); + } - uint256 _preCollateralRatio = _getCollateralRatio(_collateral, _vault); - if (_preCollateralRatio <= _collateral.liquidationThreshold) { - revert PositionIsSafe(); - } + /** + * @dev returns the collateral ratio of a vault where anything below 1e18 is liquidatable + * @dev should never revert! + * @dev exposes '_getCollateralRatio' for external access by 'liquidator' contract + * @dev only 'liquidator' contract can call + */ + function getCollateralRatioAndLiquidationThreshold(ERC20Token _collateralToken, address _owner) + external + view + onlyLiquidator + returns(uint256 collateralRatio, uint256 liquidationThreshold) + { + VaultInfo storage _vault = vaultMapping[_collateralToken][_owner]; + CollateralInfo storage _collateral = collateralMapping[_collateralToken]; - if (_currencyAmountToPay == type(uint256).max) { - // This is here to prevent frontrunning of full liquidation - // malicious owners can monitor the mempool and frontrun any attempt to liquidate their position by liquidating it - // themselves but partially, (by 1 wei of collateral is enough) which causes underflow when the liquidator's tx is to be executed - // With this, liquidators can parse in type(uint256).max to liquidate everything regardless of the current borrowed amount. - _currencyAmountToPay = _vault.borrowedAmount + _vault.accruedFees; - } + collateralRatio = _getCollateralRatio(_collateral, _vault); + liquidationThreshold = _collateral.liquidationThreshold; + } - uint256 _collateralAmountCovered = _getCollateralAmountFromCurrencyValue(_collateral, _currencyAmountToPay); - uint256 _bonus = (_collateralAmountCovered * _collateral.liquidationBonus) / HUNDRED_PERCENTAGE; - uint256 _total = _collateralAmountCovered + _bonus; + /** + * @dev returns the conversion of an amount of currency to a given supported collateral + * @dev only 'liquidator' contract can call + * @dev should never revert! + */ + function getCollateralAmountFromCurrencyValue(ERC20Token _collateralToken, uint256 _amount) + external + view + onlyLiquidator + returns (uint256) + { + CollateralInfo storage _collateral = collateralMapping[_collateralToken]; - // To make liquidations always possible, if _vault.depositedCollateral not enough to pay bonus, give out highest possible bonus - // For situations where the user's vault is insolvent, this would be called by the system stability module after a debt auction is used to raise the currency - if (_total > _vault.depositedCollateral) { - _total = _vault.depositedCollateral; - } + return (_amount * PRECISION) + / (_collateral.price * ADDITIONAL_FEED_PRECISION * (10 ** _collateral.additionalCollateralPrecision)); + } - emit Liquidated(_owner, msg.sender, _currencyAmountToPay, _total); + /** + * @dev exposes '_withdrawCollateral' function for external access by liquidator contract + * @dev only liquidator contract can call + */ + function withdrawCollateralL(ERC20Token _collateralToken, address _owner, address _to, uint256 _amount) external onlyLiquidator { + _withdrawCollateral(_collateralToken, _owner, _to, _amount); + } - _withdrawCollateral(_collateralToken, _owner, _to, _total); - _burnCurrency(_collateral, _vault, _owner, msg.sender, _currencyAmountToPay); + /** + * @dev burns currency from `_from` or/and tranfers currency from `_from` to this contract + * @dev if `_amount` > the vaults borrowed amount, the rest is used to pay accrued fees (if it is too big for that too, it reverts). else it's only used to pay the borrowed amount + * @dev borrowed amount paid back is burnt while accrued fees paid back is sent to this contract + * @dev reverts if CURRENCY_TOKEN.burn() fails + * reverts if _payAccruedFees() fails + * @dev exposes '_burnCurrency' function for external access by liquidator contract + * @dev only 'liquidator' contract can call + */ + function burnCurrencyL( + ERC20Token _collateralToken, + address _owner, + address _from, + uint256 _amount + ) external onlyLiquidator { + VaultInfo storage _vault = vaultMapping[_collateralToken][_owner]; + CollateralInfo storage _collateral = collateralMapping[_collateralToken]; + + _burnCurrency(_collateral, _vault, _owner, _from, _amount); + } - // collateral ratio must never increase or stay the same during a liquidation. - if (_preCollateralRatio <= _getCollateralRatio(_collateral, _vault)) { - revert CollateralRatioNotImproved(); - } + function getCollateralRate(ERC20Token _collateralToken) external view returns(uint256 rate) { + return collateralMapping[_collateralToken].rateInfo.rate; + } + + /** + * @dev exposes 'HUNDRED_PERCENTAGE' value for liquidator contract + */ + function hundredPercentage() external pure returns(uint256) { + return HUNDRED_PERCENTAGE; } // ------------------------------------------------ INTERNAL FUNCTIONS ------------------------------------------------ @@ -668,19 +710,6 @@ contract Vault is IVault, Ownable, Pausable { return _currencyValueOfCollateral; } - /** - * @dev returns the conversion of an amount of currency to a given supported collateral - * @dev should never revert! - */ - function _getCollateralAmountFromCurrencyValue(CollateralInfo storage _collateral, uint256 _amount) - private - view - returns (uint256) - { - return (_amount * PRECISION) - / (_collateral.price * ADDITIONAL_FEED_PRECISION * (10 ** _collateral.additionalCollateralPrecision)); - } - /** * @dev returns the fees accrued by a user's vault since `_vault.lastUpdateTime` * @dev should never revert! From ba4a7ccbb1225936ff133146e03493dc88bceed1 Mon Sep 17 00:00:00 2001 From: Emmanuel Ozeh Date: Thu, 11 Jul 2024 03:46:52 +0100 Subject: [PATCH 2/3] Refactor tests for Liquidation --- script/deploy.s.sol | 9 ++++++-- test/base.t.sol | 7 +++++- test/fuzz/vault/liquidate/liquidate.t.sol | 26 +++++++++++------------ test/helpers/ErrorsAndEvents.sol | 2 +- test/invariant/baseInvariant.t.sol | 2 +- test/invariant/collateralInvariant.t.sol | 12 +++++------ test/invariant/handlers/vaultHandler.sol | 12 ++++++----- 7 files changed, 41 insertions(+), 29 deletions(-) diff --git a/script/deploy.s.sol b/script/deploy.s.sol index aa7d1a7..d76a4e2 100644 --- a/script/deploy.s.sol +++ b/script/deploy.s.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.21; import {Vault} from "../src/vault.sol"; import {Currency} from "../src/currency.sol"; +import {Liquidator} from "../src/liquidator.sol"; import {Feed} from "../src/modules/feed.sol"; import {BaseScript, stdJson, console2} from "./base.s.sol"; @@ -12,7 +13,7 @@ import {SimpleInterestRate, IRate} from "../src/modules/rate.sol"; contract DeployScript is BaseScript { using stdJson for string; - function run() external broadcast returns (Currency xNGN, Vault vault, Feed feed, IRate rate) { + function run() external broadcast returns (Currency xNGN, Liquidator liquidator, Vault vault, Feed feed, IRate rate) { string memory deployConfigJson = getDeployConfigJson(); uint256 baseRate = deployConfigJson.readUint(".baseRate"); uint256 debtCeiling = deployConfigJson.readUint(".debtCeiling"); @@ -21,8 +22,12 @@ contract DeployScript is BaseScript { xNGN = new Currency("xNGN", "xNGN"); console2.log("xNGN deployed successfully at address:", address(xNGN)); + console2.log("\n Deploying Liquidator contract"); + liquidator = new Liquidator(); + console2.log("liquidator deployed successfully at address:", address(liquidator)); + console2.log("\n Deploying vault contract"); - vault = new Vault(xNGN, baseRate, debtCeiling); + vault = new Vault(xNGN, baseRate, debtCeiling, liquidator); console2.log("Vault deployed successfully at address:", address(vault)); console2.log("\n Deploying feed contract"); diff --git a/test/base.t.sol b/test/base.t.sol index ef36109..b15a8dd 100644 --- a/test/base.t.sol +++ b/test/base.t.sol @@ -5,6 +5,7 @@ import {Test, StdInvariant, console2} from "forge-std/Test.sol"; import {Vault, IVault, Currency} from "../src/vault.sol"; import {Feed, IOSM} from "../src/modules/feed.sol"; import {ERC20Token} from "../src/mocks/ERC20Token.sol"; +import {Liquidator} from "../src/liquidator.sol"; import {ErrorsAndEvents} from "./helpers/ErrorsAndEvents.sol"; import {IRate, SimpleInterestRate} from "../src/modules/rate.sol"; import {Median} from "descent-collective/oracle-module/median.sol"; @@ -28,6 +29,7 @@ contract BaseTest is Test, ErrorsAndEvents { Vault vault; Currency xNGN; ERC20Token usdc; + Liquidator liquidatorContract; Feed feed; IRate simpleInterestRate; OSM osm; @@ -59,6 +61,7 @@ contract BaseTest is Test, ErrorsAndEvents { vm.label(address(usdc), "USDC"); vm.label(testStabilityModule, "Test stability module"); vm.label(address(simpleInterestRate), "Simple interest module"); + vm.label(address(liquidatorContract), "Liquidator"); } function setUp() public virtual { @@ -70,7 +73,9 @@ contract BaseTest is Test, ErrorsAndEvents { usdc = new ERC20Token("Circle USD", "USDC", 6); // changing the last parameter her i.e decimals and running th tests shows that it works for all token decimals <= 18 - vault = new Vault(xNGN, onePercentPerSecondInterestRate, type(uint256).max); + liquidatorContract = new Liquidator(); + + vault = new Vault(xNGN, onePercentPerSecondInterestRate, type(uint256).max, liquidatorContract); feed = new Feed(vault); diff --git a/test/fuzz/vault/liquidate/liquidate.t.sol b/test/fuzz/vault/liquidate/liquidate.t.sol index 8718c0b..c409ea4 100644 --- a/test/fuzz/vault/liquidate/liquidate.t.sol +++ b/test/fuzz/vault/liquidate/liquidate.t.sol @@ -35,7 +35,7 @@ contract LiquidateTest is BaseTest { vm.expectRevert(CollateralDoesNotExist.selector); // call with non existing collateral - vault.liquidate(collateral, user, user2, amount); + liquidatorContract.liquidate(vault, collateral, user, user2, amount); } modifier whenCollateralExists() { @@ -52,7 +52,7 @@ contract LiquidateTest is BaseTest { // it should revert with custom error PositionIsSafe() vm.expectRevert(PositionIsSafe.selector); - vault.liquidate(usdc, user1, user2, amount); + liquidatorContract.liquidate(vault, usdc, user1, user2, amount); } modifier whenTheVaultIsNotSafe() { @@ -71,7 +71,7 @@ contract LiquidateTest is BaseTest { // it should revert with underflow error vm.expectRevert(INTEGER_UNDERFLOW_OVERFLOW_PANIC_ERROR); - vault.liquidate(usdc, user1, user2, amount); + liquidatorContract.liquidate(vault, usdc, user1, user2, amount); } modifier whenTheCurrencyAmountToBurnIsLessThanOrEqualToTheOwnersBorrowedAmountAndAccruedFees() { @@ -88,7 +88,7 @@ contract LiquidateTest is BaseTest { // it should revert with custom error CollateralRatioNotImproved() vm.expectRevert(CollateralRatioNotImproved.selector); - vault.liquidate(usdc, user1, user2, 1); + liquidatorContract.liquidate(vault, usdc, user1, user2, 1); } modifier whenVaultsCollateralRatioImprovesAfterLiquidation() { @@ -135,8 +135,8 @@ contract LiquidateTest is BaseTest { uint256 initialUser2Bal = usdc.balanceOf(user2); // it should emit Liquidated() event with with expected indexed and unindexed parameters - vm.expectEmit(true, false, false, true, address(vault)); - emit Liquidated(user1, user2, totalCurrencyPaid, collateralToPayOut); + vm.expectEmit(true, true, false, true, address(liquidatorContract)); + emit Liquidated(address(vault), user1, user2, totalCurrencyPaid, collateralToPayOut); // it should emit CurrencyBurned() event with with expected indexed and unindexed parameters vm.expectEmit(true, false, false, true, address(vault)); @@ -148,7 +148,7 @@ contract LiquidateTest is BaseTest { // liquidate uint256 amount = useUintMax ? type(uint256).max : totalCurrencyPaid; - vault.liquidate(usdc, user1, user2, amount); + liquidatorContract.liquidate(vault, usdc, user1, user2, amount); IVault.VaultInfo memory afterUserVaultInfo = getVaultMapping(usdc, user1); IVault.CollateralInfo memory afterCollateralInfo = getCollateralMapping(usdc); @@ -210,15 +210,15 @@ contract LiquidateTest is BaseTest { uint256 initialUser2Bal = usdc.balanceOf(user2); // it should emit Liquidated() event with with expected indexed and unindexed parameters - vm.expectEmit(true, false, false, true, address(vault)); - emit Liquidated(user1, user2, amountToLiquidate, collateralToPayOut); + vm.expectEmit(true, true, false, true, address(liquidatorContract)); + emit Liquidated(address(vault), user1, user2, amountToLiquidate, collateralToPayOut); // it should emit CurrencyBurned() event with with expected indexed and unindexed parameters vm.expectEmit(true, false, false, true, address(vault)); emit CurrencyBurned(user1, amountToLiquidate); // liquidate - vault.liquidate(usdc, user1, user2, amountToLiquidate); + liquidatorContract.liquidate(vault, usdc, user1, user2, amountToLiquidate); IVault.VaultInfo memory afterUserVaultInfo = getVaultMapping(usdc, user1); IVault.CollateralInfo memory afterCollateralInfo = getCollateralMapping(usdc); @@ -281,8 +281,8 @@ contract LiquidateTest is BaseTest { uint256 initialUser2Bal = usdc.balanceOf(user2); // it should emit Liquidated() event with with expected indexed and unindexed parameters - vm.expectEmit(true, false, false, true, address(vault)); - emit Liquidated(user1, user2, amountToLiquidate, collateralToPayOut); + vm.expectEmit(true, true, false, true, address(liquidatorContract)); + emit Liquidated(address(vault), user1, user2, amountToLiquidate, collateralToPayOut); // it should emit CurrencyBurned() event with with expected indexed and unindexed parameters vm.expectEmit(true, false, false, true, address(vault)); @@ -293,7 +293,7 @@ contract LiquidateTest is BaseTest { emit FeesPaid(user1, amountToLiquidate - initialUserVaultInfo.borrowedAmount); // liquidate - vault.liquidate(usdc, user1, user2, amountToLiquidate); + liquidatorContract.liquidate(vault, usdc, user1, user2, amountToLiquidate); IVault.VaultInfo memory afterUserVaultInfo = getVaultMapping(usdc, user1); IVault.CollateralInfo memory afterCollateralInfo = getCollateralMapping(usdc); diff --git a/test/helpers/ErrorsAndEvents.sol b/test/helpers/ErrorsAndEvents.sol index 543c207..d6fadde 100644 --- a/test/helpers/ErrorsAndEvents.sol +++ b/test/helpers/ErrorsAndEvents.sol @@ -9,7 +9,7 @@ contract ErrorsAndEvents { event CurrencyBurned(address indexed owner, uint256 amount); event FeesPaid(address indexed owner, uint256 amount); event Liquidated( - address indexed owner, address liquidator, uint256 currencyAmountPaid, uint256 collateralAmountCovered + address indexed vault, address indexed owner, address liquidator, uint256 currencyAmountPaid, uint256 collateralAmountCovered ); error ZeroAddress(); diff --git a/test/invariant/baseInvariant.t.sol b/test/invariant/baseInvariant.t.sol index b9ca89b..53ce4ac 100644 --- a/test/invariant/baseInvariant.t.sol +++ b/test/invariant/baseInvariant.t.sol @@ -30,7 +30,7 @@ contract BaseInvariantTest is BaseTest { timeManager = new TimeManager(); vaultGetters = new VaultGetters(); - vaultHandler = new VaultHandler(vault, usdc, xNGN, vaultGetters, timeManager); + vaultHandler = new VaultHandler(vault, usdc, xNGN, vaultGetters, timeManager, liquidatorContract); usdcHandler = new ERC20Handler(Currency(address(usdc)), timeManager); xNGNHandler = new ERC20Handler(xNGN, timeManager); osmHandler = new OSMHandler(osm, timeManager); diff --git a/test/invariant/collateralInvariant.t.sol b/test/invariant/collateralInvariant.t.sol index e528f99..7440275 100644 --- a/test/invariant/collateralInvariant.t.sol +++ b/test/invariant/collateralInvariant.t.sol @@ -80,20 +80,20 @@ contract CollateralInvariantTest is BaseInvariantTest { vm.startPrank(liquidator); if (vaultGetters.getHealthFactor(vault, usdc, user1)) vm.expectRevert(PositionIsSafe.selector); - vault.liquidate(usdc, user1, address(this), type(uint256).max); + liquidatorContract.liquidate(vault, usdc, user1, address(this), type(uint256).max); if (vaultGetters.getHealthFactor(vault, usdc, user2)) vm.expectRevert(PositionIsSafe.selector); - vault.liquidate(usdc, user2, address(this), type(uint256).max); + liquidatorContract.liquidate(vault, usdc, user2, address(this), type(uint256).max); if (vaultGetters.getHealthFactor(vault, usdc, user3)) vm.expectRevert(PositionIsSafe.selector); - vault.liquidate(usdc, user3, address(this), type(uint256).max); + liquidatorContract.liquidate(vault, usdc, user3, address(this), type(uint256).max); if (vaultGetters.getHealthFactor(vault, usdc, user4)) vm.expectRevert(PositionIsSafe.selector); - vault.liquidate(usdc, user4, address(this), type(uint256).max); + liquidatorContract.liquidate(vault, usdc, user4, address(this), type(uint256).max); if (vaultGetters.getHealthFactor(vault, usdc, user5)) vm.expectRevert(PositionIsSafe.selector); - vault.liquidate(usdc, user5, address(this), type(uint256).max); - } + liquidatorContract.liquidate(vault, usdc, user5, address(this), type(uint256).max); + } function invariant_collateral_rateInfo_rate() external useCurrentTime { assertGt(getCollateralMapping(usdc).rateInfo.rate, 0); diff --git a/test/invariant/handlers/vaultHandler.sol b/test/invariant/handlers/vaultHandler.sol index 48bba09..710feea 100644 --- a/test/invariant/handlers/vaultHandler.sol +++ b/test/invariant/handlers/vaultHandler.sol @@ -1,7 +1,8 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity 0.8.21; -import {Test, ERC20Token, IVault, Vault, console2, Currency} from "../../base.t.sol"; +import {Test, ERC20Token, IVault, Vault, console2, Currency, Liquidator} from "../../base.t.sol"; +import {ILiquidator} from "../../../src/interfaces/ILiquidator.sol"; import {VaultGetters} from "../helpers/vaultGetters.sol"; import {TimeManager} from "../helpers/timeManager.sol"; @@ -11,6 +12,7 @@ contract VaultHandler is Test { Vault vault; ERC20Token usdc; Currency xNGN; + Liquidator liquidatorContract; address owner = vm.addr(uint256(keccak256("OWNER"))); address user1 = vm.addr(uint256(keccak256("User1"))); address user2 = vm.addr(uint256(keccak256("User2"))); @@ -29,13 +31,14 @@ contract VaultHandler is Test { uint256 public totalMints; uint256 public totalBurns; - constructor(Vault _vault, ERC20Token _usdc, Currency _xNGN, VaultGetters _vaultGetters, TimeManager _timeManager) { + constructor(Vault _vault, ERC20Token _usdc, Currency _xNGN, VaultGetters _vaultGetters, TimeManager _timeManager, Liquidator _liquidatorContract) { timeManager = _timeManager; vault = _vault; usdc = _usdc; vaultGetters = _vaultGetters; xNGN = _xNGN; + liquidatorContract = _liquidatorContract; actors[0] = user1; actors[1] = user2; @@ -174,9 +177,8 @@ contract VaultHandler is Test { { vm.startPrank(liquidator); - if (vaultGetters.getHealthFactor(vault, usdc, currentOwner)) vm.expectRevert(IVault.PositionIsSafe.selector); - vault.liquidate(usdc, currentOwner, address(this), type(uint256).max); - + if (vaultGetters.getHealthFactor(vault, usdc, currentOwner)) vm.expectRevert(ILiquidator.PositionIsSafe.selector); + liquidatorContract.liquidate(vault, usdc, currentOwner, address(this), type(uint256).max); vm.stopPrank(); } From 47cad307320fc69e9d9c6e9f3a8ca6e55ebe3d85 Mon Sep 17 00:00:00 2001 From: Emmanuel Ozeh Date: Wed, 17 Jul 2024 19:01:49 +0100 Subject: [PATCH 3/3] extract liquidatorHandler from vaultHandler in tests --- test/invariant/baseInvariant.t.sol | 14 +++- test/invariant/handlers/liquidatorHandler.sol | 68 +++++++++++++++++++ test/invariant/handlers/vaultHandler.sol | 33 +-------- 3 files changed, 81 insertions(+), 34 deletions(-) create mode 100644 test/invariant/handlers/liquidatorHandler.sol diff --git a/test/invariant/baseInvariant.t.sol b/test/invariant/baseInvariant.t.sol index 53ce4ac..6f0172d 100644 --- a/test/invariant/baseInvariant.t.sol +++ b/test/invariant/baseInvariant.t.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.21; import {BaseTest, IVault, Currency} from "../base.t.sol"; import {VaultHandler} from "./handlers/vaultHandler.sol"; +import {LiquidatorHandler} from "./handlers/liquidatorHandler.sol"; import {ERC20Handler} from "./handlers/erc20Handler.sol"; import {OSMHandler} from "./handlers/osmHandler.sol"; import {MedianHandler} from "./handlers/medianHandler.sol"; @@ -14,6 +15,7 @@ contract BaseInvariantTest is BaseTest { TimeManager timeManager; VaultGetters vaultGetters; VaultHandler vaultHandler; + LiquidatorHandler liquidatorHandler; ERC20Handler usdcHandler; ERC20Handler xNGNHandler; OSMHandler osmHandler; @@ -30,7 +32,8 @@ contract BaseInvariantTest is BaseTest { timeManager = new TimeManager(); vaultGetters = new VaultGetters(); - vaultHandler = new VaultHandler(vault, usdc, xNGN, vaultGetters, timeManager, liquidatorContract); + vaultHandler = new VaultHandler(vault, usdc, xNGN, vaultGetters, timeManager); + liquidatorHandler = new LiquidatorHandler(vaultGetters, liquidatorContract, usdc, xNGN, vault, timeManager); usdcHandler = new ERC20Handler(Currency(address(usdc)), timeManager); xNGNHandler = new ERC20Handler(xNGN, timeManager); osmHandler = new OSMHandler(osm, timeManager); @@ -45,6 +48,7 @@ contract BaseInvariantTest is BaseTest { vm.label(address(osmHandler), "osmHandler"); vm.label(address(medianHandler), "medianHandler"); vm.label(address(feedHandler), "feedHandler"); + vm.label(address(liquidatorHandler), "liquidatorHandler"); // target handlers targetContract(address(vaultHandler)); @@ -53,8 +57,9 @@ contract BaseInvariantTest is BaseTest { targetContract(address(osmHandler)); targetContract(address(medianHandler)); targetContract(address(feedHandler)); + targetContract(address(liquidatorHandler)); - bytes4[] memory vaultSelectors = new bytes4[](11); + bytes4[] memory vaultSelectors = new bytes4[](10); vaultSelectors[0] = VaultHandler.depositCollateral.selector; vaultSelectors[1] = VaultHandler.withdrawCollateral.selector; vaultSelectors[2] = VaultHandler.mintCurrency.selector; @@ -65,7 +70,6 @@ contract BaseInvariantTest is BaseTest { vaultSelectors[7] = VaultHandler.deny.selector; vaultSelectors[8] = VaultHandler.updateBaseRate.selector; vaultSelectors[9] = VaultHandler.updateCollateralData.selector; - vaultSelectors[10] = VaultHandler.liquidate.selector; bytes4[] memory xNGNSelectors = new bytes4[](4); xNGNSelectors[0] = ERC20Handler.transfer.selector; @@ -89,6 +93,9 @@ contract BaseInvariantTest is BaseTest { bytes4[] memory feedSelectors = new bytes4[](1); feedSelectors[0] = FeedHandler.updatePrice.selector; + bytes4[] memory liquidatorSelectors = new bytes4[](1); + liquidatorSelectors[0] = LiquidatorHandler.liquidate.selector; + // target selectors of handlers targetSelector(FuzzSelector({addr: address(vaultHandler), selectors: vaultSelectors})); targetSelector(FuzzSelector({addr: address(xNGNHandler), selectors: xNGNSelectors})); @@ -96,6 +103,7 @@ contract BaseInvariantTest is BaseTest { targetSelector(FuzzSelector({addr: address(osmHandler), selectors: osmSelectors})); targetSelector(FuzzSelector({addr: address(medianHandler), selectors: medianSelectors})); targetSelector(FuzzSelector({addr: address(feedHandler), selectors: feedSelectors})); + targetSelector(FuzzSelector({addr: address(liquidatorHandler), selectors: liquidatorSelectors})); } // forgefmt: disable-start diff --git a/test/invariant/handlers/liquidatorHandler.sol b/test/invariant/handlers/liquidatorHandler.sol new file mode 100644 index 0000000..98d2508 --- /dev/null +++ b/test/invariant/handlers/liquidatorHandler.sol @@ -0,0 +1,68 @@ +//SPDX-Licence-Identifier: GPL-3.0 +pragma solidity 0.8.21; + +import {Test, Liquidator, ERC20Token, Vault, Currency} from "../../base.t.sol"; +import {ILiquidator} from "../../../src/interfaces/ILiquidator.sol"; +import {VaultGetters} from "../helpers/vaultGetters.sol"; +import {TimeManager} from "../helpers/timeManager.sol"; + +contract LiquidatorHandler is Test { + TimeManager timeManager; + Liquidator liquidatorContract; + VaultGetters vaultGetters; + Vault vault; + Currency xNGN; + ERC20Token usdc; + address owner = vm.addr(uint256(keccak256("OWNER"))); + address liquidator = vm.addr(uint256(keccak256("liquidator"))); + + address[5] actors; + address currentActor; + address currentOwner; // address to be used as owner variable in the calls to be made + + constructor(VaultGetters _vaultGetters, Liquidator _liquidatorContract, ERC20Token _usdc, Currency _xNGN, Vault _vault, TimeManager _timeManager) { + liquidatorContract = _liquidatorContract; + vault = _vault; + usdc = _usdc; + xNGN = _xNGN; + vaultGetters = _vaultGetters; + timeManager = _timeManager; + + // FOR LIQUIDATIONS BY LIQUIDATOR + // mint usdc to address(this) + vm.startPrank(owner); + Currency(address(usdc)).mint(liquidator, 100_000_000_000 * (10 ** usdc.decimals())); + vm.stopPrank(); + + // use address(this) to deposit so that it can borrow currency needed for liquidation below + vm.startPrank(liquidator); + usdc.approve(address(vault), type(uint256).max); + vault.depositCollateral(usdc, liquidator, 100_000_000_000 * (10 ** usdc.decimals())); + vault.mintCurrency(usdc, liquidator, liquidator, 500_000_000_000e18); + xNGN.approve(address(vault), type(uint256).max); + vm.stopPrank(); + } + + function liquidate(uint256 skipTimeSeed, uint256 ownerIndexSeed) + external + skipTime(skipTimeSeed) + setOwner(ownerIndexSeed) + { + vm.startPrank(liquidator); + + if (vaultGetters.getHealthFactor(vault, usdc, currentOwner)) vm.expectRevert(ILiquidator.PositionIsSafe.selector); + liquidatorContract.liquidate(vault, usdc, currentOwner, address(this), type(uint256).max); + vm.stopPrank(); + } + + modifier skipTime(uint256 skipTimeSeed) { + uint256 skipTimeBy = bound(skipTimeSeed, 0, 365 days); + timeManager.skipTime(skipTimeBy); + _; + } + + modifier setOwner(uint256 actorIndexSeed) { + currentOwner = actors[bound(actorIndexSeed, 0, actors.length - 1)]; + _; + } +} diff --git a/test/invariant/handlers/vaultHandler.sol b/test/invariant/handlers/vaultHandler.sol index 710feea..9d70237 100644 --- a/test/invariant/handlers/vaultHandler.sol +++ b/test/invariant/handlers/vaultHandler.sol @@ -1,8 +1,7 @@ // SPDX-License-Identifier: GPL-3.0 pragma solidity 0.8.21; -import {Test, ERC20Token, IVault, Vault, console2, Currency, Liquidator} from "../../base.t.sol"; -import {ILiquidator} from "../../../src/interfaces/ILiquidator.sol"; +import {Test, ERC20Token, IVault, Vault, console2, Currency} from "../../base.t.sol"; import {VaultGetters} from "../helpers/vaultGetters.sol"; import {TimeManager} from "../helpers/timeManager.sol"; @@ -12,7 +11,6 @@ contract VaultHandler is Test { Vault vault; ERC20Token usdc; Currency xNGN; - Liquidator liquidatorContract; address owner = vm.addr(uint256(keccak256("OWNER"))); address user1 = vm.addr(uint256(keccak256("User1"))); address user2 = vm.addr(uint256(keccak256("User2"))); @@ -31,34 +29,19 @@ contract VaultHandler is Test { uint256 public totalMints; uint256 public totalBurns; - constructor(Vault _vault, ERC20Token _usdc, Currency _xNGN, VaultGetters _vaultGetters, TimeManager _timeManager, Liquidator _liquidatorContract) { + constructor(Vault _vault, ERC20Token _usdc, Currency _xNGN, VaultGetters _vaultGetters, TimeManager _timeManager) { timeManager = _timeManager; vault = _vault; usdc = _usdc; vaultGetters = _vaultGetters; xNGN = _xNGN; - liquidatorContract = _liquidatorContract; actors[0] = user1; actors[1] = user2; actors[2] = user3; actors[3] = user4; actors[4] = user5; - - // FOR LIQUIDATIONS BY LIQUIDATOR - // mint usdc to address(this) - vm.startPrank(owner); - Currency(address(usdc)).mint(liquidator, 100_000_000_000 * (10 ** usdc.decimals())); - vm.stopPrank(); - - // use address(this) to deposit so that it can borrow currency needed for liquidation below - vm.startPrank(liquidator); - usdc.approve(address(vault), type(uint256).max); - vault.depositCollateral(usdc, liquidator, 100_000_000_000 * (10 ** usdc.decimals())); - vault.mintCurrency(usdc, liquidator, liquidator, 500_000_000_000e18); - xNGN.approve(address(vault), type(uint256).max); - vm.stopPrank(); } modifier skipTime(uint256 skipTimeSeed) { @@ -170,18 +153,6 @@ contract VaultHandler is Test { vault.burnCurrency(usdc, currentOwner, amount); } - function liquidate(uint256 skipTimeSeed, uint256 ownerIndexSeed) - external - skipTime(skipTimeSeed) - setOwner(ownerIndexSeed) - { - vm.startPrank(liquidator); - - if (vaultGetters.getHealthFactor(vault, usdc, currentOwner)) vm.expectRevert(ILiquidator.PositionIsSafe.selector); - liquidatorContract.liquidate(vault, usdc, currentOwner, address(this), type(uint256).max); - vm.stopPrank(); - } - function recoverToken(uint256 skipTimeSeed, bool isUsdc, address to) external skipTime(skipTimeSeed) { if (to == address(0)) to = address(uint160(uint256(keccak256(abi.encode(to))))); if (isUsdc) {