Uniswap V1 ABI "duplicate output parameter "out" in function removeLiquidity" error thrown

[kamescg]

When using ethers@v5 and loading the Uniswap V1 ABI I get "duplicate output parameter "out" in function removeLiquidity" error thrown.

ABI Culprit

{
    "name": "removeLiquidity",
    "outputs": [
      { "type": "uint256", "name": "out" },
      { "type": "uint256", "name": "out" }
    ],
    "inputs": [
      { "type": "uint256", "name": "amount" },
      { "type": "uint256", "name": "min_eth" },
      { "type": "uint256", "name": "min_tokens" },
      { "type": "uint256", "name": "deadline" }
    ],
    "constant": false,
    "payable": false,
    "type": "function",
    "gas": 116814
  },

Not sure if I should report this error here, because it seems like a Uniswap problem when Vyper compiles the ABI, but seeing as it's unlikely to change, I thought it would be worth reporting because Uniswap is obviously super popular and people are going to use the generated ABI when V5 gets released.

Reference to the Vyper Smart Contract

https://github.com/Uniswap/uniswap-v1/blob/master/contracts/uniswap_exchange.vy#L83

[ricmoo]

Yeah, that is definitely an ABI error. Otherwise, what should result.out refer to. :)

It's a good question what to do. It should certainly be a bug logged against Uniswap and possibly Vyper? Do you know if that is what Vyper injected, or whether Uniswap edited the ABI after it was dumped?

Someone posted an ABI cleaner script yesterday, perhaps I should post something similar and include cleaning output parameter names in the cookbook?

[kamescg]

Just ran the compiler on the contracts. Same output.

Could do out1 and out2 or spice it up with outBetter and outBetterer 😂

I guess I'll just report on the Uniswap contracts? Don't know any Vyper, so not sure how to rename the outputs on the following function?

def removeLiquidity(amount: uint256, min_eth: uint256(wei), min_tokens: uint256, deadline: timestamp) -> (uint256(wei), uint256):

[ricmoo]

Yeah, I was chatting with the Vyper peeps. Sounds like they are willing to make output names unique. But obviously that doesn’t help existing ABI.

But I will probably make it so if output names collide, they are available positionally only, and not by name. If they are unique, of course everything will work fine, positionally and named. :)

I’ll get to this tomorrow. :)

[ricmoo]

I was told that the dev version of Vyper no longer outputs names for outputs now.

Also, the fix I've put in place is a bit more forgiving. It simply drops names (internally; they are still available on the Fragment for inspection). So, if encoding, it will not allow named values if the name is ambiguous and for decoding it will not expose it by name if the name is ambiguous.

So, something like this (uint256 foo, uint256 foo, uint256 bar) with the values 1, 2, 3 would be the Result object [ 1, 2, 3, bar: 3 ]. Since foo is ambiguous, no foo is exported, but bar is safe so it is available positionally (i.e. result[2]) and by name (i.e. result.bar), while the two foo's are only available positionally (i.e. result[0], result[1]).

I think that should make everyone happy? Or as happy as is safe to make everyone?

[ricmoo]

This has been added to 5.0.0-beta.187. Try it out and let me know if you still have any problems. :)

[kamescg]

Awesome! Will test it this week when I get chance to circle back around to repo.

[ricmoo]

Coolio. I'll close this now then, but if you still have any problem, please re-open.

Thanks! :)