[L] There is no two way transfer on setting Governance setGov() and setting Operator setOperator()
In Market.sol there is a modifier named onlyGov which control the market crucial function. To update this onlyGov address, there is only one function setGov() without two step transfer pattern. This can cause a problem if the function is being set with wrong gov address because it can't revert the value. Consider to use two step transfer patter for this kind of ownership function.
there is also an instance of this issue, setOperator inside BorrowController.sol contract
File: Market.sol
130: function setGov(address _gov) public onlyGov { gov = _gov; }
File: BorrowController.sol
26: function setOperator(address _operator) public onlyOperator { operator = _operator; }
[L] Borrow function does not check if amount > 0
In Market.sol contract, borrowInternal() function, the amount value is not being check if it's greater than 0. Thus if function call with 0 amount, the transfer will process with 0 transfer token, which is a waste of gas, it's better to check condition and revert.
[L] No upper bound for replenishmentPriceBps in DBR.sol contract
File: DBR.sol
62: function setReplenishmentPriceBps(uint newReplenishmentPriceBps_) public onlyOperator {
63: require(newReplenishmentPriceBps_ > 0, "replenishment price must be over 0");
64: replenishmentPriceBps = newReplenishmentPriceBps_;
65: }
[L] Pay function is not check for availability of token
In GovTokenEscrow.sol and SimpleERC20Escrow.sol contracts, the pay() function is not checking if the balance of the token inside the contract is enough to transfer based on the amount input. (Unlike INVEscrow.sol)
[L] There is no two way transfer on setting Governance
setGov()
and setting OperatorsetOperator()
In
Market.sol
there is a modifier namedonlyGov
which control the market crucial function. To update thisonlyGov
address, there is only one functionsetGov()
without two step transfer pattern. This can cause a problem if the function is being set with wrong gov address because it can't revert the value. Consider to use two step transfer patter for this kind ofownership
function.there is also an instance of this issue,
setOperator
insideBorrowController.sol
contract[L] Borrow function does not check if amount > 0
In
Market.sol
contract,borrowInternal()
function, the amount value is not being check if it's greater than 0. Thus if function call with 0 amount, the transfer will process with 0 transfer token, which is a waste of gas, it's better to check condition and revert.[L] No upper bound for
replenishmentPriceBps
inDBR.sol
contract[L]
Pay
function is not check for availability of tokenIn
GovTokenEscrow.sol
andSimpleERC20Escrow.sol
contracts, thepay()
function is not checking if the balance of the token inside the contract is enough to transfer based on theamount
input. (Unlike INVEscrow.sol)[NC] Open TODO comment
For production, there should not be any
TODO
left on the code[NC] Inconsistency of using uint vs uint256
Even though both is similar but it's good to have standard for quality code example:
[NC] Maximum line length is 120 characters
There are some of code > 120 characters per line in (Market.sol, Oracle.sol)
[NC] No emit event on
setReplenishmentPriceBps
It's best to emit an event if function change some settings