Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix audit changes #320

Merged
merged 1 commit into from
Jun 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions contracts/interfaces/ICurvePoolFactory.sol
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// SPDX-License-Identifier: AGPL-3.0
pragma solidity ^0.8.10;

interface ICurvePoolFactory {
function get_coins(address _pool) external view returns (address[4] memory);

Expand Down
6 changes: 6 additions & 0 deletions contracts/interfaces/IIncurDebt.sol
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,10 @@ interface IIncurDebt {
* @param _borrower the account to seize
*/
function seize(address _borrower) external;

/// @notice lets governor withdraw tokens incase of airdrop or error
/// - onlyOwner (or governance)
/// @param _tokenAddress the address of the token
/// @param _amount amount of tokens to withdraw
function withdrawToken(address _tokenAddress, uint256 _amount) external;
}
1 change: 1 addition & 0 deletions contracts/interfaces/IStrategy.sol
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// SPDX-License-Identifier: AGPL-3.0
pragma solidity ^0.8.10;

/**
Expand Down
27 changes: 21 additions & 6 deletions contracts/peripheral/IncurDebt.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ error IncurDebt_AmountAboveBorrowerBalance(uint256 _amount);
error IncurDebt_OHMAmountMoreThanAvailableLoan(uint256 _amount);
error IncurDebt_BorrowerHasNoOutstandingDebt(address _borrower);
error IncurDebt_BorrowerStillHasOutstandingDebt(address _borrower);
error IncurDebt_InvalidAddress();

/// @title IncurDebt
/// @notice Contract that allows users to use the treasury's incurdebt function.
Expand Down Expand Up @@ -107,6 +108,13 @@ contract IncurDebt is OlympusAccessControlledV2, IIncurDebt {
address _treasury,
address _olympusAuthority
) OlympusAccessControlledV2(IOlympusAuthority(_olympusAuthority)) {
if (_OHM == address(0)) revert IncurDebt_InvalidAddress();
if (_gOHM == address(0)) revert IncurDebt_InvalidAddress();
if (_sOHM == address(0)) revert IncurDebt_InvalidAddress();
if (_staking == address(0)) revert IncurDebt_InvalidAddress();
if (_treasury == address(0)) revert IncurDebt_InvalidAddress();
if (_olympusAuthority == address(0)) revert IncurDebt_InvalidAddress();

OHM = _OHM;
gOHM = _gOHM;
sOHM = _sOHM;
Expand Down Expand Up @@ -151,7 +159,6 @@ contract IncurDebt is OlympusAccessControlledV2, IIncurDebt {

/// @notice lets a user become a LP borrower
/// - onlyOwner (or governance)
/// - user must not be borrower
/// @param _borrower the address that will interact with contract
function allowLPBorrower(address _borrower) external override onlyGovernor {
if (borrowers[_borrower].isNonLpBorrower) revert IncurDebt_AlreadyBorrower(_borrower);
Expand All @@ -163,7 +170,6 @@ contract IncurDebt is OlympusAccessControlledV2, IIncurDebt {

/// @notice lets a user become a Non LP borrower
/// - onlyOwner (or governance)
/// - user must not be borrower
/// @param _borrower the address that will interact with contract
function allowNonLPBorrower(address _borrower) external override onlyGovernor {
if (borrowers[_borrower].isLpBorrower) revert IncurDebt_AlreadyBorrower(_borrower);
Expand All @@ -176,7 +182,6 @@ contract IncurDebt is OlympusAccessControlledV2, IIncurDebt {
/// @notice sets the maximum debt limit for a borrower
/// - onlyOwner (or governance)
/// - limit must be greater than or equal to borrower's outstanding debt
/// - limit must be less than or equal to the global debt limit
/// @param _borrower the address that will interact with contract
/// @param _limit borrower's debt limit in OHM
function setBorrowerDebtLimit(address _borrower, uint256 _limit)
Expand Down Expand Up @@ -227,7 +232,7 @@ contract IncurDebt is OlympusAccessControlledV2, IIncurDebt {
/// - will burn all collateral, including excess of debt
/// - onlyGovernance
/// @param _borrower the account to seize collateral
function seize(address _borrower) external override onlyGovernor isBorrower(_borrower) {
function seize(address _borrower) external override onlyGovernor {
(uint256 seizedCollateral, uint256 paidDebt) = _repay(_borrower);
borrowers[_borrower].collateralInGOHM = 0;

Expand All @@ -237,6 +242,14 @@ contract IncurDebt is OlympusAccessControlledV2, IIncurDebt {
emit DebtPaidWithCollateralAndBurnTheRest(_borrower, paidDebt, totalOutstandingGlobalDebt, seizedCollateral);
}

/// @notice lets governor withdraw tokens incase of airdrop or error
/// - onlyOwner (or governance)
/// @param _tokenAddress the address of the token
/// @param _amount amount of tokens to withdraw
function withdrawToken(address _tokenAddress, uint256 _amount) external override onlyGovernor {
IERC20(_tokenAddress).safeTransfer(msg.sender, _amount);
}

/************************
* Borrower Functions
************************/
Expand Down Expand Up @@ -379,7 +392,6 @@ contract IncurDebt is OlympusAccessControlledV2, IIncurDebt {
revert IncurDebt_AmountAboveBorrowerBalance(_amount);

if (_amount > borrower.collateralInGOHM - borrower.unwrappedGOHM) {
// Does uint256 > uint128 cause problem?
uint256 amountGOHMToWrap = borrower.unwrappedGOHM + _amount - borrower.collateralInGOHM;
borrower.unwrappedGOHM -= uint128(amountGOHMToWrap);
uint256 amountOHMNeededToWrap = IgOHM(gOHM).balanceFrom(amountGOHMToWrap);
Expand Down Expand Up @@ -413,7 +425,7 @@ contract IncurDebt is OlympusAccessControlledV2, IIncurDebt {

IERC20(gOHM).transfer(msg.sender, collateralRemaining);

emit DebtPaidWithCollateralAndWithdrawTheRest( //Change event
emit DebtPaidWithCollateralAndWithdrawTheRest(
msg.sender,
paidDebt,
totalOutstandingGlobalDebt,
Expand All @@ -430,6 +442,9 @@ contract IncurDebt is OlympusAccessControlledV2, IIncurDebt {
Borrower storage borrower = borrowers[msg.sender];

if (borrower.debt == 0) revert IncurDebt_BorrowerHasNoOutstandingDebt(msg.sender);
if (_ohmAmount > borrower.debt) {
_ohmAmount = borrower.debt;
}

totalOutstandingGlobalDebt -= _ohmAmount;
borrower.debt -= uint128(_ohmAmount);
Expand Down
29 changes: 12 additions & 17 deletions contracts/peripheral/Strategies/UniSwap.sol
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,8 @@ contract UniSwapStrategy is IStrategy {
uint256 amountADesired,
uint256 amountBDesired,
uint256 amountAMin,
uint256 amountBMin,
uint256 slippage
) = abi.decode(_data, (address, address, uint256, uint256, uint256, uint256, uint256));
uint256 amountBMin
) = abi.decode(_data, (address, address, uint256, uint256, uint256, uint256));

if (tokenA == ohmAddress) {
if (_ohmAmount != amountADesired) revert UniswapStrategy_AmountDoesNotMatch();
Expand All @@ -74,8 +73,8 @@ contract UniSwapStrategy is IStrategy {
if (_ohmAmount != amountBDesired) revert UniswapStrategy_AmountDoesNotMatch();

IERC20(tokenB).safeTransferFrom(incurDebtAddress, address(this), _ohmAmount);
IERC20(tokenA).safeTransferFrom(_user, address(this), amountBDesired);
IERC20(tokenA).approve(address(router), amountBDesired);
IERC20(tokenA).safeTransferFrom(_user, address(this), amountADesired);
IERC20(tokenA).approve(address(router), amountADesired);
} else {
revert UniswapStrategy_OhmAddressNotFound();
}
Expand All @@ -88,8 +87,8 @@ contract UniSwapStrategy is IStrategy {
tokenB,
amountADesired,
amountBDesired,
(amountAMin * slippage) / 1000,
(amountBMin * slippage) / 1000,
amountAMin,
amountBMin,
incurDebtAddress,
block.timestamp
);
Expand Down Expand Up @@ -128,14 +127,10 @@ contract UniSwapStrategy is IStrategy {
address _user
) external returns (uint256 ohmRecieved) {
if (msg.sender != incurDebtAddress) revert UniswapStrategy_NotIncurDebtAddress();
(
address tokenA,
address tokenB,
uint256 liquidity,
uint256 amountAMin,
uint256 amountBMin,
uint256 slippage
) = abi.decode(_data, (address, address, uint256, uint256, uint256, uint256));
(address tokenA, address tokenB, uint256 liquidity, uint256 amountAMin, uint256 amountBMin) = abi.decode(
_data,
(address, address, uint256, uint256, uint256)
);

address lpTokenAddress = IUniswapV2Factory(factory).getPair(tokenA, tokenB);

Expand All @@ -149,8 +144,8 @@ contract UniSwapStrategy is IStrategy {
tokenA,
tokenB,
liquidity,
(amountAMin * slippage) / 1000,
(amountBMin * slippage) / 1000,
amountAMin,
amountBMin,
address(this),
block.timestamp
);
Expand Down
55 changes: 26 additions & 29 deletions test/debt/IncurDebt.js
Original file line number Diff line number Diff line change
Expand Up @@ -725,11 +725,10 @@ describe("IncurDebt", async () => {
const daiAmount = "1000000000000000000000";
const token0 = olympus.ohm;
const token1 = "0x6B175474E89094C44Da98b954EedeAC495271d0F";
const slippage = 900;

const data = ethers.utils.defaultAbiCoder.encode(
["address", "address", "uint256", "uint256", "uint256", "uint256", "uint256"],
[token0, token1, ohmAmount, daiAmount, ohmAmount, daiAmount, slippage]
["address", "address", "uint256", "uint256", "uint256", "uint256"],
[token0, token1, ohmAmount, daiAmount, "30000000000", "100000000000000000000"]
);

it("Should fail if borrower isNonLpBorrower", async () => {
Expand Down Expand Up @@ -1010,17 +1009,14 @@ describe("IncurDebt", async () => {
const token0 = olympus.ohm;
const token1 = "0x6B175474E89094C44Da98b954EedeAC495271d0F";

const slippage = 900;
const slippage1 = 900;

const fakeData = ethers.utils.defaultAbiCoder.encode(
["address", "address", "uint256", "uint256", "uint256", "uint256"],
[token0, token1, ohmAmount, daiAmount, ohmAmount, slippage]
["address", "address", "uint256", "uint256", "uint256"],
[token0, token1, ohmAmount, daiAmount, ohmAmount]
);

const data1 = ethers.utils.defaultAbiCoder.encode(
["address", "address", "uint256", "uint256", "uint256", "uint256", "uint256"],
[token0, token1, ohmAmount, daiAmount, ohmAmount, daiAmount, slippage]
["address", "address", "uint256", "uint256", "uint256", "uint256"],
[token0, token1, ohmAmount, daiAmount, "30000000000", "100000000000000000000"]
);

it("Should fail if borrower isNonLpBorrower", async () => {
Expand Down Expand Up @@ -1068,23 +1064,12 @@ describe("IncurDebt", async () => {
const totalOutstandingGlobalDebtBeforeTx = await incurDebt.totalOutstandingGlobalDebt();
assert.equal(borrowerInfoBeforeTx.debt, totalOutstandingGlobalDebtBeforeTx.toString());

const token0PoolBalance = await ohm_token.balanceOf(uniOhmDaiLpAddress);
const token1PoolBalance = await daiContract.balanceOf(uniOhmDaiLpAddress);
const poolTotalSupply = await uniswapLpContract.totalSupply();

const amount1Min = (token0PoolBalance * borrowerLpBeforeTx) / poolTotalSupply;
const amount2Min = (token1PoolBalance * borrowerLpBeforeTx) / poolTotalSupply;
const amount1Min = "30000000000";
const amount2Min = "100000000000000000000";

const data = ethers.utils.defaultAbiCoder.encode(
["address", "address", "uint256", "uint256", "uint256", "uint256"],
[
token0,
token1,
borrowerLpBeforeTx,
amount1Min.toString(),
amount2Min.toString(),
slippage1,
]
["address", "address", "uint256", "uint256", "uint256"],
[token0, token1, borrowerLpBeforeTx, amount1Min, amount2Min]
);
await expect(
incurDebt
Expand Down Expand Up @@ -1240,11 +1225,9 @@ describe("IncurDebt", async () => {
const token0 = olympus.ohm;
const token1 = "0x6B175474E89094C44Da98b954EedeAC495271d0F";

const slippage = 900;

const data = ethers.utils.defaultAbiCoder.encode(
["address", "address", "uint256", "uint256", "uint256", "uint256", "uint256"],
[token0, token1, ohmAmount, daiAmount, ohmAmount, daiAmount, slippage]
["address", "address", "uint256", "uint256", "uint256", "uint256"],
[token0, token1, ohmAmount, daiAmount, "30000000000", "100000000000000000000"]
);

it("Should fail if borrower isNonLpBorrower", async () => {
Expand Down Expand Up @@ -1390,6 +1373,20 @@ describe("IncurDebt", async () => {
});
});

describe("Withdraw Tokens as Governor", async () => {
it("Should be able to sweep tokens", async () => {
await daiContract
.connect(daiHolder)
.transfer(incurDebt.address, "10000000000000000000");
let before = await daiContract.balanceOf(governor.address);
await incurDebt
.connect(governor)
.withdrawToken(daiContract.address, "10000000000000000000");
let after = await daiContract.balanceOf(governor.address);
await expect(after.sub(before)).to.equal("10000000000000000000");
});
});

async function impersonate(address) {
await impersonateAccount(address);
const owner = await ethers.getSigner(address);
Expand Down