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.

Prev

LibInternal.sol (LIL-C)

Next

Life.sol (LIF-C)

On this page