Omniscia Beanstalk Audit

LibMarket Code Style Findings

LibMarket Code Style Findings

LMT-01C: Code Duplication

Description:

The linked functions perform the exact same statements.

Example:

protocol/contracts/libraries/LibMarket.sol
55function buy(uint256 buyBeanAmount) internal returns (uint256 amount) {
56 (uint256 ethAmount, uint256 beanAmount) = _buy(buyBeanAmount, msg.value, msg.sender);
57 (bool success,) = msg.sender.call{ value: msg.value.sub(ethAmount) }("");
58 require(success, "Market: Refund failed.");
59 return beanAmount;
60}
61
62function buyAndDeposit(uint256 buyBeanAmount) internal returns (uint256 amount) {
63 (uint256 ethAmount, uint256 beanAmount) = _buy(buyBeanAmount, msg.value, address(this));
64 (bool success,) = msg.sender.call{ value: msg.value.sub(ethAmount) }("");
65 require(success, "Market: Refund failed.");
66 return beanAmount;
67}

Recommendation:

We advise either one to be removed or both to utilise a secondary private function to reduce the byte code size of the contract.

Alleviation:

The optimization outlined is no longer relevant as refunds are performed at a different level.

LMT-02C: Redundant Future Trade Timestamp

Description:

The linked addition of 1 to the current block.timestamp for the Uniswap trade execution is redundant as the trade system of Uniswap [simply evaluates that the value passed in is greater-than-or-equal (>=) the current block.timestamp].

Example:

protocol/contracts/libraries/LibMarket.sol
93function removeLiquidity(uint256 liqudity, uint256 minBeanAmount,uint256 minEthAmount)
94 internal
95 returns (uint256 beanAmount, uint256 ethAmount)
96{
97 DiamondStorage storage ds = diamondStorage();
98 return IUniswapV2Router02(ds.router).removeLiquidityETH(
99 ds.bean,
100 liqudity,
101 minBeanAmount,
102 minEthAmount,
103 msg.sender,
104 block.timestamp.add(1)
105 );
106}

Recommendation:

We advise the addition to be removed optimising the codebase.

Alleviation:

The block.timestamp value is now directly utilized as advised.

LMT-03C: Redundant Usage of SafeMath

Description:

The linked SafeMath operations are guaranteed to be performed safely by their surrounding if clauses and general logical constraints.

Example:

protocol/contracts/libraries/LibMarket.sol
318function transferAllocatedBeans(uint256 allocatedBeans, uint256 transferBeans) internal {
319 DiamondStorage storage ds = diamondStorage();
320 if (allocatedBeans == 0) {
321 IBean(ds.bean).transferFrom(msg.sender, address(this), transferBeans);
322 }
323 else if (allocatedBeans >= transferBeans) {
324 emit BeanAllocation(msg.sender, transferBeans);
325 if (allocatedBeans > transferBeans) IBean(ds.bean).transfer(msg.sender, allocatedBeans.sub(transferBeans));
326 } else {
327 emit BeanAllocation(msg.sender, allocatedBeans);
328 IBean(ds.bean).transferFrom(msg.sender, address(this), transferBeans.sub(allocatedBeans));
329 }
330}

Recommendation:

We advise them to be performed without the usage of SafeMath to optimise the code.

Alleviation:

The code was updated to omit all redundant SafeMath calculations thereby optimizing it extensively.