Skip to content

Commit

Permalink
fix: low / informational issues from the audit (#57)
Browse files Browse the repository at this point in the history
* add burn function to ensure no gOHM is stuck
* rename `isDefaulted` to `hasExpired`
* ensure both tokens have 18 decimals
* ownership change removes callbacks
  • Loading branch information
0xrusowsky authored Sep 12, 2023
1 parent f1f7f74 commit 008697f
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 65 deletions.
9 changes: 9 additions & 0 deletions src/Clearinghouse.sol
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,15 @@ contract Clearinghouse is Policy, RolesConsumer, CoolerCallback {
token_.transfer(address(TRSRY), amount_);
}

/// @notice Burn any gOHM defaulted using the Cooler instead of the Clearinghouse.
function burn() external {
uint256 gohmBalance = gohm.balanceOf(address(this));

// Unstake and burn gOHM holdings.
gohm.approve(address(staking), gohmBalance);
MINTR.burnOhm(address(this), staking.unstake(address(this), gohmBalance, false, false));
}

/// @notice Deactivate the contract and return funds to treasury.
function emergencyShutdown() external onlyRole("emergency_shutdown") {
active = false;
Expand Down
8 changes: 5 additions & 3 deletions src/Cooler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,8 @@ contract Cooler is Clone {
// Update the load lender and the recipient.
loans[loanID_].lender = msg.sender;
loans[loanID_].recipient = msg.sender;
// Callbacks are disabled when transferring ownership.
loans[loanID_].callback = false;
// Clear transfer approvals.
approvals[loanID_] = address(0);
}
Expand Down Expand Up @@ -354,10 +356,10 @@ contract Cooler is Clone {
return (principle_ * interest) / DECIMALS_INTEREST;
}

/// @notice Check if given loan is in default.
/// @notice Check if given loan has expired.
/// @param loanID_ index of loan in loans[].
/// @return Defaulted status.
function isDefaulted(uint256 loanID_) external view returns (bool) {
/// @return Expiration status.
function hasExpired(uint256 loanID_) external view returns (bool) {
return block.timestamp > loans[loanID_].expiry;
}

Expand Down
48 changes: 27 additions & 21 deletions src/CoolerFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ import {Cooler} from "./Cooler.sol";
contract CoolerFactory {
using ClonesWithImmutableArgs for address;

// --- ERRORS ----------------------------------------------------

error NotFromFactory();
error DecimalsNot18();

// --- EVENTS ----------------------------------------------------

/// @notice A global event when a new loan request is created.
Expand Down Expand Up @@ -55,27 +60,28 @@ contract CoolerFactory {
/// @param collateral_ the token given as collateral.
/// @param debt_ the token to be lent. Interest is denominated in debt tokens.
/// @return cooler address of the contract.
function generateCooler(ERC20 collateral_, ERC20 debt_) external returns (address cooler) {
// Return address if cooler exists.
cooler = coolerFor[msg.sender][collateral_][debt_];

// Otherwise generate new cooler.
if (cooler == address(0)) {
// Clone the cooler implementation.
bytes memory coolerData = abi.encodePacked(
msg.sender, // owner
address(collateral_), // collateral
address(debt_), // debt
address(this) // factory
);
cooler = address(coolerImplementation).clone(coolerData);

// Update storage accordingly.
coolerFor[msg.sender][collateral_][debt_] = cooler;
coolersFor[collateral_][debt_].push(cooler);
created[cooler] = true;
}
function generateCooler(ERC20 collateral_, ERC20 debt_) external returns (address cooler) {
// Return address if cooler exists.
cooler = coolerFor[msg.sender][collateral_][debt_];

// Otherwise generate new cooler.
if (cooler == address(0)) {
if (collateral_.decimals() != 18 || debt_.decimals() != 18) revert DecimalsNot18();
// Clone the cooler implementation.
bytes memory coolerData = abi.encodePacked(
msg.sender, // owner
address(collateral_), // collateral
address(debt_), // debt
address(this) // factory
);
cooler = address(coolerImplementation).clone(coolerData);

// Update storage accordingly.
coolerFor[msg.sender][collateral_][debt_] = cooler;
coolersFor[collateral_][debt_].push(cooler);
created[cooler] = true;
}
}

// --- EMIT EVENTS -----------------------------------------------

Expand All @@ -93,7 +99,7 @@ contract CoolerFactory {
/// @param ev_ event type.
/// @param amount_ to be logged by the event.
function newEvent(uint256 id_, Events ev_, uint256 amount_) external {
require(created[msg.sender], "Only Created");
if (!created[msg.sender]) revert NotFromFactory();

if (ev_ == Events.RequestLoan) {
emit RequestLoan(msg.sender, address(Cooler(msg.sender).collateral()), address(Cooler(msg.sender).debt()), id_);
Expand Down
40 changes: 0 additions & 40 deletions src/test/Cooler.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -977,44 +977,4 @@ contract CoolerTest is Test {
vm.expectRevert(Cooler.Default.selector);
cooler.extendLoanTerms(loanID, 1);
}

function test_collateralFor_debtDecimalsHigh() public {
uint8 collateralDecimals = 6; // collateral: USDC
uint8 debtDecimals = 18; // debt: WETH

// Both values are in terms of debt decimals
uint256 loanToCollateral_ = (10 ** collateralDecimals) * 1e18 / 2000e6; // 1 WETH : 2000 USDC
uint256 amount_ = 1e18;

// Create the tokens
collateral = new MockGohm("Collateral", "COL", collateralDecimals);
debt = new MockERC20("Debt", "DEBT", debtDecimals);

// Instantiate a new cooler
cooler = _initCooler();

// collateralFor() should return the correct amount of collateral tokens
uint256 collateralFor = cooler.collateralFor(amount_, loanToCollateral_);
assertEq(collateralFor, 2000e6, "USDC");
}

function test_collateralFor_debtDecimalsLow() public {
uint8 collateralDecimals = 18; // collateral: WETH
uint8 debtDecimals = 6; // debt: USDC

// Both values are in terms of debt decimals
uint256 loanToCollateral_ = (10 ** collateralDecimals) * 2000e6 / 1e18; // 2000 USDC : 1 WETH
uint256 amount_ = 2000e6;

// Create the tokens
collateral = new MockGohm("Collateral", "COL", collateralDecimals);
debt = new MockERC20("Debt", "DEBT", debtDecimals);

// Instantiate a new cooler
cooler = _initCooler();

// collateralFor() should return the correct amount of collateral tokens
uint256 collateralFor = cooler.collateralFor(amount_, loanToCollateral_);
assertEq(collateralFor, 1e18, "ETH");
}
}
21 changes: 20 additions & 1 deletion src/test/CoolerFactory.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,25 @@ contract CoolerFactoryTest is Test {
assertEq(otherCoolerBob, coolerFactory.coolersFor(collateral, otherDebt, 0));
}

function testRevert_wrongDecimals() public {
// Create the wrong tokens
MockERC20 wrongCollateral = new MockERC20("Collateral", "COL", 6);
MockERC20 wrongDebt = new MockERC20("Debt", "DEBT", 6);

// Only tokens with 18 decimals are allowed
vm.startPrank(alice);

// Collateral with 6 decimals
vm.expectRevert(CoolerFactory.DecimalsNot18.selector);
coolerFactory.generateCooler(wrongCollateral, debt);
// Debt with 6 decimals
vm.expectRevert(CoolerFactory.DecimalsNot18.selector);
coolerFactory.generateCooler(collateral, wrongDebt);
// Both with 6 decimals
vm.expectRevert(CoolerFactory.DecimalsNot18.selector);
coolerFactory.generateCooler(wrongCollateral, wrongDebt);
}

function test_newEvent() public {
uint256 id = 0;
uint256 amount = 1234;
Expand Down Expand Up @@ -119,7 +138,7 @@ contract CoolerFactoryTest is Test {

// Only coolers can emit events
vm.prank(alice);
vm.expectRevert("Only Created");
vm.expectRevert(CoolerFactory.NotFromFactory.selector);
coolerFactory.newEvent(id, CoolerFactory.Events.ClearRequest, amount);
}
}

0 comments on commit 008697f

Please sign in to comment.