[L-1] Loss of precision due to rounding
https:
uint256 loanRatio = (debt * 10 ** 18) / collateral;
https:
uint256 fees = (debt * borrowerFee) / 10000;
https:
uint256 loanRatio = (totalDebt * 10 ** 18) / loan.collateral;
https:
uint256 currentAuctionRate = (MAX_INTEREST_RATE * timeElapsed) /
loan.auctionLength;
https:
uint256 govFee = (borrowerFee * loan.collateral) / 10000;
https:
uint256 loanRatio = (debt * 10 ** 18) / collateral;
https:
uint256 fee = (borrowerFee * (debt - debtToPay)) / 10000;
https:
interest = (l.interestRate * l.debt * timeElapsed) / 10000 / 365 days;
https:
fees = (lenderFee * interest) / 10000;
[L-2] Consider using OpenZeppelin’s SafeCast library to prevent unexpected overflows when casting from uint256.
Using the SafeCast library can help prevent unexpected errors in your Solidity code and make your contracts more secure
https:
uint256 fee = (borrowerFee * (debt - debtToPay)) / 10000;
https:
uint256 timeElapsed = block.timestamp - l.startTimestamp;
interest = (l.interestRate * l.debt * timeElapsed) / 10000 / 365 days;
fees = (lenderFee * interest) / 10000;
interest -= fees;
[L-3] Lack of Sanity/Threshold/Limit Checks
Devoid of sanity/threshold/limit checks, critical parameters can be configured to invalid values, causing a variety of issues and breaking expected interactions within/between contracts. Consider adding proper uint256 validation as well as zero address checks for critical changes. A worst case scenario would render the contract needing to be re-deployed in the event of human/accidental errors that involve value assignments to immutable variables. If the validation procedure is unclear or too complex to implement on-chain, document the potential issues that could produce invalid values.
https:
function setLenderFee(uint256 _fee) external onlyOwner {
if (_fee > 5000) revert FeeTooHigh();
lenderFee = _fee;
}
https:
function setBorrowerFee(uint256 _fee) external onlyOwner {
if (_fee > 500) revert FeeTooHigh();
borrowerFee = _fee;
}
https:
function setFeeReceiver(address _feeReceiver) external onlyOwner {
feeReceiver = _feeReceiver;
}
https:
constructor(address _weth, address _staking) {
WETH = _weth;
staking = _staking;
}
https:
function _afterTokenTransfer(address from, address to, uint256 amount)
internal
override(ERC20, ERC20Votes)
{
super._afterTokenTransfer(from, to, amount);
}
https:
function _mint(address to, uint256 amount)
internal
override(ERC20, ERC20Votes)
{
super._mint(to, amount);
}
https:
function _burn(address account, uint256 amount)
internal
override(ERC20, ERC20Votes)
{
super._burn(account, amount);
https:
function mint(address to, uint256 amount) external onlyOwner {
_mint(to, amount);
}
[L-4] Function Calls in Loop Could Lead to Denial of Service
Function calls made in unbounded loop are error-prone with potential resource exhaustion as it can trap the contract due to the gas limitations or failed transactions
https:
for (uint256 i = 0; i < borrows.length; i++) {
bytes32 poolId = borrows[i].poolId;
uint256 debt = borrows[i].debt;
uint256 collateral = borrows[i].collateral;
https:
for (uint256 i = 0; i < loanIds.length; i++) {
uint256 loanId = loanIds[i];
https:
for (uint256 i = 0; i < loanIds.length; i++) {
uint256 loanId = loanIds[i];
bytes32 poolId = poolIds[i];
https:
for (uint256 i = 0; i < loanIds.length; i++) {
uint256 loanId = loanIds[i];
https:
for (uint256 i = 0; i < loanIds.length; i++) {
uint256 loanId = loanIds[i];
https:
for (uint256 i = 0; i < refinances.length; i++) {
[NC-1] Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)
While the compiler knows to optimize away the exponentiation, it’s still better coding practice to use idioms that do not require compiler optimization, if they exist.
https:
uint256 loanRatio = (debt * 10 ** 18) / collateral;
https:
uint256 loanRatio = (totalDebt * 10 ** 18) / loan.collateral;
https:
uint256 loanRatio = (debt * 10 ** 18) / collateral;
[NC-2] Take advantage of Custom Error’s return value property
An important feature of Custom Error is that values such as address, tokenID, msg.value can be written inside the () sign, this kind of approach provides a serious advantage in debugging and examining the revert details of dapps such as tenderly
https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L183
if (pools[poolId].lender != msg.sender) revert Unauthorized();
https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L184
if (amount == 0) revert PoolConfig();
https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L247
if (loanRatio > pool.maxLoanRatio) revert RatioTooHigh();
https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L375
if (pool.auctionLength < loan.auctionLength) revert AuctionTooShort();
[NC-4] Use constants instead of using numbers directly without explanations.
https:
uint256 fees = (debt * borrowerFee) / 10000;
https:
uint256 govFee = (borrowerFee * loan.collateral) / 10000;
https:
uint256 fee = (borrowerFee * (debt - debtToPay)) / 10000;
https:
interest = (l.interestRate * l.debt * timeElapsed) / 10000 / 365 days;
fees = (lenderFee * interest) / 10000;
