# A Whole Afternoon Lost Debugging

> Yesterday, the Ymechs and some strategists spent some long hours debugging what now looks like a stupid bug. I am writing these lines to make sure it doesn't happen to you in the future.

## Context
Yearn is in the process of abstracting swaps outside of the strategy's contract. The reasons behind this are outside of the scope of this doc, but the gist is: an agregator like cowswap is giving better swapping results than a hardcoded path in a smart contract. To reduce the logic in the contract, we abstracted trades using ySwaps. ySwaps is a layer where a strategy says:

**I have X and I want Y, find me the best result.**

If the trade is simple, we go through an agregator like zrx or cowswap but we sometimes have more complex logic. Let's go through the new yfi/eth curve lp vault **(not released yet)**.

yfi/eth strategy deposits in convex and the reward is CRV and CVX. In this case, we have two reward tokens which we dump for the lp token to reinvest.

Strategy tells yswap:
- I have x crv and I want yfi/eth lp token
- I have y cvx and I want yfi/eth lp token

We can't send this complex order to an agregator, so we are settling the trade with a small script with multi-calls. The actions executed are:

- Sweep crv+cvx from the strategy to yswap's swapper
- Trade crv for weth
- Trade cvx for weth
- Create the yfi/weth lp with weth
- Send it back to the strategy

After the script passed in a fork, we sent it to mainnet and this was the result: https://ethtx.info/mainnet/0xa987975e5aa694598128c2426c19ca05aad281160f5ff37d5358ed980a2b7abc/

If you check the account balances you will notice that the strategy got the reward tokens sweeped, but it never got the lp and there was weth left in the swapper. 0.0364 weth:


## The code
Instead of going through all the code, let's analyze the problematic action which is creating the lp and sending it to the strategy. At this point, the swapper has the weth and it's ready to add liquidity.

    const multicallSwapperSigner = await impersonate(this.multicallSwapperAddress);
    const weth = IWETH__factory.connect(this.wethAddress, multicallSwapperSigner);
    const curveSwap = ICurveFi__factory.connect(this.curveSwapAddress, multicallSwapperSigner);

    const wethBalance = await weth.balanceOf(this.multicallSwapperAddress);
    await curveSwap.add_liquidity([wethBalance, 0], 0, false, this.strategyAddress);

The curve interface is created with typechained. The interface used is:

interface ICurveFi {

  function add_liquidity(
    uint256[] calldata,
    uint256,
    bool,
    address
  ) external payable;
}

The curve code is here: https://etherscan.io/address/0xC26b89A667578ec7b3f11b2F98d6Fd15C07C54ba#code
The method signature is the following:

@payable
@external
@nonreentrant('lock')
def add_liquidity(amounts: uint256[N_COINS], min_mint_amount: uint256,
                  use_eth: bool = False, receiver: address = msg.sender) -> uint256:

The first problem, **weth was never approved**. Yes, that was my bad, very stupid but how can we explain that the transaction didn't revert?

To make things more weird, we did a small test in brownie and it did revert!

>>> ms = Contract("0xFEB4acf3df3cDEA7399794D0869ef76A6EfAff52")
weth = Contract("0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2")
weth.deposit({"from": ms, 'value': ms.balance()})
crv_swap = Contract("0xC26b89A667578ec7b3f11b2F98d6Fd15C07C54ba")
tx = crv_swap.add_liquidity([weth.balanceOf(ms), 0], 0, False, ms, {"from": ms})
Transaction sent: 0x2eea2958fbb3541d87e2e9786b248f9342edb60bbf0b6cf24d05f7aee42f0722
  Gas price: 0.0 gwei   Gas limit: 12000000   Nonce: 1
  WETH9.deposit confirmed   Block: 14199803   Gas used: 43738 (0.36%)

Transaction sent: 0x9e73197ab7ff267b08f1860116054ee0ee9ccb92c2c8af3dd95dc42032949cd5
  Gas price: 0.0 gwei   Gas limit: 12000000   Nonce: 2
  FixedICurveFi.add_liquidity confirmed (reverted)   Block: 14199804   Gas used: 61630 (0.51%)

After a lot of failed hypothesis, someone finally realized the difference. The interface used was not correct. We used dynamic arrays, which vyper doesn't support yet, and the method signature uses a fixed array size. `uint256[] calldata` is not the same as `uint256[2] calldata`. Brownie works as expected because it downloads the proper interface from etherscan.

At this point, the issue can be fixed but we couldn't explain why the code didn't revert. Let's dig in a bit more.

When an interface is used to call a contract, the method signature is hashed and that value is used to do the call. Check the hashes of the two interfaces versions:

>>> web3.Web3.keccak(text="add_liquidity(uint256[],uint256,bool,address)")[:4]
HexBytes('0xe5ff94f4')
>>> web3.Web3.keccak(text="add_liquidity(uint256[2],uint256,bool,address)")[:4]
HexBytes('0x7328333b')

I remember seeing that `0xe5ff94f4` before... here: https://ethtx.info/mainnet/0xa987975e5aa694598128c2426c19ca05aad281160f5ff37d5358ed980a2b7abc/


ethtx.info couldn't decode `0xe5ff94f4` to a function, because it doesn't exist!
What happens when the method doesn't exist in the table? It goes to the fallback. In Vyper, the [default method](https://vyper.readthedocs.io/en/stable/control-structures.html?highlight=fallback#the-default-function). If we go to the curve contract again, we see the fallback is there:

@payable
@external
def __default__():
    pass

That's the full explanation, function was not found, fallback method is called and nothing happens.
One more very important corollary, if we did the same thing with native eth, we would actually lose it. Here's a sample code done with brownie, using the fixed array interface, and the dynamic array interface.

me = a[0]
weth = Contract("0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2", owner=me)
weth.deposit({"value": me.balance()/2})
dynamic = interface.DynamicICurveFi("0xC26b89A667578ec7b3f11b2F98d6Fd15C07C54ba", owner=me)
fixed = interface.FixedICurveFi("0xC26b89A667578ec7b3f11b2F98d6Fd15C07C54ba", owner=me)
yfieth = Contract("0x29059568bB40344487d62f7450E78b8E6C74e0e5", owner=me)
amount = Wei("1 ether")

print(f"eth balance: {me.balance()/1e18:_}")
print(f"yfieth balance {yfieth.balanceOf(me)/1e18:_}")
print(f"Weth balance: {weth.balanceOf(me)/1e18:_}.")

print(f"WETH")
print(f"Calling dynamic add liquidity with weth")
tx = dynamic.add_liquidity([amount, 0], 0, False, me)
print(f"eth balance: {me.balance()/1e18:_}")
print(f"yfieth balance {yfieth.balanceOf(me)/1e18:_}")
print(f"Weth balance: {weth.balanceOf(me)/1e18:_}.")
print(f"Approve was not done but the tx passes.")
print(f"I guess it's because there is no method matching and it goes to the fallback.")

print(f"Calling fixed add liquidity with weth")
tx = fixed.add_liquidity([amount, 0], 0, False, me)
print(f"eth balance: {me.balance()/1e18:_}")
print(f"yfieth balance {yfieth.balanceOf(me)/1e18:_}")
print(f"Weth balance: {weth.balanceOf(me)/1e18:_}.")
print(f"Approve was not done and the tx reverts.")

print(f"Native ETH")

print(f"Calling dynamic add liquidity with native eth")
tx = dynamic.add_liquidity([amount, 0], 0, True, me, {"value": amount})
print(f"eth balance: {me.balance()/1e18:_}")
print(f"yfieth balance {yfieth.balanceOf(me)/1e18:_}")
print(f"Weth balance: {weth.balanceOf(me)/1e18:_}.")
print(f"Even though I am using the dynamic interface, the tx passes.")


print(f"Calling fixed add liquidity with native eth")
tx = fixed.add_liquidity([amount, 0], 0, True, me, {"value": amount})
print(f"eth balance: {me.balance()/1e18:_}")
print(f"yfieth balance {yfieth.balanceOf(me)/1e18:_}")
print(f"Weth balance: {weth.balanceOf(me)/1e18:_}.")
print(f"Approve was not done but tx goes through.")

Output

Transaction sent: 0xc9befa03a5723db61193cf223fdf6fcf2c60bc787ebcc81ab6337761652429af
  Gas price: 0.0 gwei   Gas limit: 12000000   Nonce: 2
  WETH9.deposit confirmed   Block: 14194930   Gas used: 43738 (0.36%)

eth balance: 50.0
yfieth balance 0.0
Weth balance: 50.0.
WETH
Calling dynamic add liquidity with weth
Transaction sent: 0x3ef8c3bc7f6a8f3f52ae9733e4779d66eb2304788b34dd3181fb479398a3c9a3
  Gas price: 0.0 gwei   Gas limit: 12000000   Nonce: 3
  Transaction confirmed   Block: 14194931   Gas used: 24556 (0.20%)

eth balance: 50.0
yfieth balance 0.0
Weth balance: 50.0.
Approve was not done but the tx passes.
I guess it's because there is no method matching and it goes to the fallback.
Calling fixed add liquidity with weth
Transaction sent: 0x850f700b3e1871af455a6a6d2d7e6c26fc6ab7ed0cf2f7c1fdead92a524f6c7d
  Gas price: 0.0 gwei   Gas limit: 12000000   Nonce: 4
  FixedICurveFi.add_liquidity confirmed (reverted)   Block: 14194932   Gas used: 61606 (0.51%)

eth balance: 50.0
yfieth balance 0.0
Weth balance: 50.0.
Approve was not done and the tx reverts.
Native ETH
Calling dynamic add liquidity with native eth
Transaction sent: 0x75beb768cd97e96f8e7a4de6a32ed0841c919521df0d032f2f3101050602febd
  Gas price: 0.0 gwei   Gas limit: 12000000   Nonce: 5
  Transaction confirmed   Block: 14194933   Gas used: 24568 (0.20%)

eth balance: 49.0
yfieth balance 0.0
Weth balance: 50.0.
Even though I am using the dynamic interface, the tx passes.
Calling fixed add liquidity with native eth
Transaction sent: 0xe0ce56331af8f92549e4befbf0de2313c99406a0d6e8127d31a609603465e5d2
  Gas price: 0.0 gwei   Gas limit: 12000000   Nonce: 6
  FixedICurveFi.add_liquidity confirmed   Block: 14194934   Gas used: 292363 (2.44%)

eth balance: 48.0
yfieth balance 0.17679032583925655
Weth balance: 50.0.
Approve was not done but tx goes through.

As you can see, calling add liquidity with the dynamic array interface with native eth, consumes the eth, but we don't get any yfieth.

## Recomendations

### Assert balances when you add liquidity or do swaps
Checking the balance of the lp token in the fork would have found this error before execution in mainnet.

### Be careful when sending native eth to a contract
Double check when using native eth. A mistake like this would have cost us all the strategy profit.

### Setup proper minOut and slippages when trading
We were rewriting the executor script code and we didn't setup slippages correctly. When the tx run, our allowed slippage was 0.