Omniscia AmpleSense Audit

BalancerTrader Static Analysis Findings

BalancerTrader Static Analysis Findings

BTR-01S: Improper Invocation of ERC-20 Transfers

Description:

The EIP-20 standard denotes that callers should never assume that false is not returned by the transfer / transferFrom functions.

Example:

contracts/BalancerTrader.sol
47/**
48* @dev Caller must transfer the right amount of tokens to the trader
49 */
50function sellAMPLForEth(uint256 amount) external override returns (uint256 ethAmount) {
51 amplToken.transferFrom(msg.sender, address(this), amount);
52 (ethAmount,) = amplEth.swapExactAmountIn(address(amplToken), amount, address(wethToken), 0, MAX_INT);
53 wethToken.withdraw(ethAmount);
54 msg.sender.transfer(ethAmount);
55 emit Sale_ETH(amount, ethAmount);
56}
57
58/**
59* @dev Caller must transfer the right amount of tokens to the trader (USDC will be replaced with ETH)
60 */
61function sellAMPLForEEFI(uint256 amount) external override returns (uint256 eefiAmount) {
62 amplToken.transferFrom(msg.sender, address(this), amount);
63 (uint256 usdcAmount,) = amplUsdc.swapExactAmountIn(address(amplToken), amount, address(usdcToken), 0, MAX_INT);
64 eefiAmount = vault.swap(IVault.SingleSwap(
65 eefiUsdcPoolID,
66 IVault.SwapKind.GIVEN_IN,
67 IAsset(address(usdcToken)),
68 IAsset(address(eefiToken)),
69 usdcAmount,
70 "0x"
71 ), IVault.FundManagement(
72 address(this),
73 false,
74 msg.sender,
75 false),
76 0, block.timestamp);
77 emit Sale_EEFI(amount, eefiAmount);
78}

Recommendation:

Depending on the implementation of the AMPL token, we advise either a require check to be imposed similarly to the approve invocations or a comment to be introduced denoting that the implementation does not return a bool that can be evaluated.

Alleviation:

The Amplesense team has properly introduced require checks mandating the successful execution of those transferFrom invocations.