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.

Sign up for GitHub

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

optimizooor #7

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

optimizooor #7

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
70a004d
optimizooor
Nemusonaneko Oct 11, 2022
d9c7838
Merge branch 'master' into master
Nemusonaneko Oct 11, 2022
a51a029
readd missing iteration
Nemusonaneko Oct 11, 2022
d395180
change how currentDailyBorrows is added
Nemusonaneko Oct 11, 2022
aa970ff
remove optimizations
Nemusonaneko Oct 13, 2022
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
37 changes: 25 additions & 12 deletions contracts/LendingPool.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ contract LendingPool is OwnableUpgradeable, ERC721Upgradeable, Clone {
uint public totalBorrowed; // = 0;
string private constant baseURI = "https://nft.llamalend.com/nft/";
uint maxDailyBorrows; // IMPORTANT: an attacker can borrow up to 150% of this limit if they prepare beforehand
uint216 currentDailyBorrows;
uint216 currentDailyBorrows; // would have to borrow more eth than will ever exist daily to break
uint40 lastUpdateDailyBorrows;
mapping(address => bool) public liquidators;
mapping(address => uint) public liquidators;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have you verified that this is in fact cheaper? i think it should be but havent verified it

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saves 16168 gas on deployment and a whopping 12 gas on addLiquidator()


event Borrowed(uint currentDailyBorrows, uint newBorrowedAmount);
event ReducedDailyBorrows(uint currentDailyBorrows, uint amountReduced);
Expand Down Expand Up @@ -72,7 +72,10 @@ contract LendingPool is OwnableUpgradeable, ERC721Upgradeable, Clone {
if(toReduce > currentDailyBorrows){
currentDailyBorrows = toAdd;
} else {
currentDailyBorrows = uint216(currentDailyBorrows - toReduce) + toAdd;
unchecked {
// Resulting daily borrow has to be over 10^47 ETH
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is safu because we know toReduce <= currentDailyBorrows, so (currentDailyBorrows - toReduce) >= 0

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

llama agrees

currentDailyBorrows = uint216(currentDailyBorrows - toReduce) + toAdd;
Nemusonaneko marked this conversation as resolved.
Show resolved Hide resolved
}
}
require(currentDailyBorrows < maxDailyBorrows, "max daily borrow");
lastUpdateDailyBorrows = uint40(block.timestamp);
Expand Down Expand Up @@ -125,7 +128,10 @@ contract LendingPool is OwnableUpgradeable, ERC721Upgradeable, Clone {
require(borrowedNow == totalToBorrow, "ltv changed");
uint interest = calculateInterest(borrowedNow);
require(interest <= maxInterest);
totalBorrowed += borrowedNow;
unchecked {
// There isn't enough eth to overflow and probably will never be :bat: :loud_sound:
totalBorrowed += borrowedNow;
Nemusonaneko marked this conversation as resolved.
Show resolved Hide resolved
}
uint i = 0;
while(i<length){
_borrow(nftId[i], price, interest);
Expand Down Expand Up @@ -158,15 +164,20 @@ contract LendingPool is OwnableUpgradeable, ERC721Upgradeable, Clone {
if(sinceLoanStart > maxLoanLength){
interest += ((sinceLoanStart - maxLoanLength)*borrowed)/(1 days);
}
totalBorrowed -= borrowed;
unchecked {
totalBorrowed -= borrowed;
}

_burnWithoutBalanceChanges(loanId, from);

if(sinceLoanStart < (1 days)){
uint until24h;
uint toReduce;
unchecked {
until24h = (1 days) - sinceLoanStart;
// Impossible to overflow since borrowed would overflow before this would
toReduce = (borrowed*until24h)/(1 days);
0xngmi marked this conversation as resolved.
Show resolved Hide resolved
}
uint toReduce = (borrowed*until24h)/(1 days);
if(toReduce < currentDailyBorrows){
unchecked {
// toReduce < currentDailyBorrows always so it's fine to restrict to uint216 because currentDailyBorrows is uint216 already
Expand All @@ -188,9 +199,9 @@ contract LendingPool is OwnableUpgradeable, ERC721Upgradeable, Clone {
uint length = loansToRepay.length;
uint totalToRepay = 0;
uint i = 0;
while(i<length){
totalToRepay += _repay(loansToRepay[i], from);
while(i<length) {
unchecked {
totalToRepay += _repay(loansToRepay[i], from);
Nemusonaneko marked this conversation as resolved.
Show resolved Hide resolved
i++;
}
}
Expand All @@ -200,11 +211,13 @@ contract LendingPool is OwnableUpgradeable, ERC721Upgradeable, Clone {

// Liquidate expired loan
function doEffectiveAltruism(Loan calldata loan, address to) external {
require(liquidators[msg.sender] == true);
require(liquidators[msg.sender] == 1);
uint loanId = getLoanId(loan.nft, loan.interest, loan.startTime, loan.borrowed);
require(_exists(loanId), "loan closed");
require(block.timestamp > (loan.startTime + maxLoanLength), "not expired");
totalBorrowed -= loan.borrowed;
unchecked {
totalBorrowed -= loan.borrowed;
}
_burn(loanId);
nftContract.transferFrom(address(this), to, loan.nft);
}
Expand Down Expand Up @@ -299,12 +312,12 @@ contract LendingPool is OwnableUpgradeable, ERC721Upgradeable, Clone {
}

function addLiquidator(address liq) external onlyOwner {
liquidators[liq] = true;
liquidators[liq] = 1;
emit LiquidatorAdded(liq);
}

function removeLiquidator(address liq) external onlyOwner {
liquidators[liq] = false;
liquidators[liq] = 0;
emit LiquidatorRemoved(liq);
}

Expand Down