Uniswap / v1-contracts

🐍Uniswap V1 smart contracts
GNU General Public License v3.0
495 stars 318 forks source link

truffle tests erroring with "VM Exception while processing transaction: invalid JUMP" #25

Closed BlinkyStitt closed 5 years ago

BlinkyStitt commented 5 years ago

I'm trying to interact with uniswap through a truffle contract written in solidity 0.5.4. I have truffle migrate able to deploy the contract bytecode that is checked into the repo. The abi is giving me issues, but I've worked around that for now (https://github.com/trufflesuite/truffle/issues/1623). I have a test that AFAICT is successfully initializing the contract and setting up an exchange for a test ERC20 token. However, when I try to add liquidity, I get "invalid JUMP".

To ease debugging this, I should probably just push my whole repo up to GitHub, but I wasn't wanting to open source it just yet.

Here are some relevant pieces:

IUniswapExchange.sol

IUniswapFactory.sol

test/uniswap.js:

const assert = require("chai").assert;

const UniswapExchange = artifacts.require("uniswap_exchange");
const IUniswapExchange = artifacts.require("IUniswapExchange");

const UniswapFactory = artifacts.require("uniswap_factory");
const IUniswapFactory = artifacts.require("IUniswapFactory");

const TutorialToken = artifacts.require("TutorialToken");

contract("uniswap_factory", accounts => {

  it("... should be initializable through the interface", async () => {
    await UniswapFactory.new();
    const uniswapFactoryInstance = await IUniswapFactory.at(UniswapFactory.address);

    // TODO: is this right?
    await UniswapExchange.deployed();
    await uniswapFactoryInstance.initializeFactory(UniswapExchange.address);

    const savedExchangeTemplate = await uniswapFactoryInstance.exchangeTemplate();
    assert.equal(savedExchangeTemplate, UniswapExchange.address);

    const tokenCount = await uniswapFactoryInstance.tokenCount();
    assert.equal(tokenCount, 0);
  });

  it("... should have a pool for TutorialToken", async () => {
    await UniswapExchange.deployed();
    await UniswapFactory.deployed();
    const uniswapFactoryInstance = await IUniswapFactory.at(UniswapFactory.address);

    await TutorialToken.deployed();
    // TODO: return value of this is a tx receipt instead of the address like I expected. i'm doing something wrong
    await uniswapFactoryInstance.createExchange(TutorialToken.address);

    const tokenCount = await uniswapFactoryInstance.tokenCount();
    assert.equal(tokenCount, 1);

    const tutorialTokenUniswapAddress = await uniswapFactoryInstance.getExchange(TutorialToken.address);

    const tutorialTokenUniswapInstance = await IUniswapExchange.at(tutorialTokenUniswapAddress);

    const savedTutorialTokenAddress = await tutorialTokenUniswapInstance.tokenAddress();
    assert.equal(savedTutorialTokenAddress, TutorialToken.address);

    const savedFactoryAddress = await tutorialTokenUniswapInstance.factoryAddress();
    assert.equal(savedFactoryAddress, UniswapFactory.address);
  });

  it("... should add some TutorialToken to the Uniswap pool", async () => {
    await UniswapExchange.deployed();
    await UniswapFactory.deployed();
    const uniswapFactoryInstance = await IUniswapFactory.at(UniswapFactory.address);

    const tutorialTokenInstance = await TutorialToken.deployed();
    const tutorialTokenUniswapAddress = await uniswapFactoryInstance.getExchange(TutorialToken.address);

    await tutorialTokenInstance.approve(tutorialTokenUniswapAddress, 100000000);

    const tutorialTokenUniswapInstance = await IUniswapExchange.at(tutorialTokenUniswapAddress);

    const balanceETH = web3.utils.toBN(await web3.eth.getBalance(accounts[0]));
    assert(balanceETH.gt(1000));

    const balance = await tutorialTokenInstance.balanceOf(accounts[0]);
    assert.isAtLeast(balance.toNumber(), 1000);

    const current_block = await web3.eth.getBlock(await web3.eth.getBlockNumber());

    // TODO: why is this reverting?
    await tutorialTokenUniswapInstance.addLiquidity(1000, 1000, current_block.timestamp + 300, {value: 1000});
  });

});
  Contract: uniswap_factory
    ✓ ... should be initializable through the interface (356200 gas)
    ✓ ... should have a pool for TutorialToken (250727 gas)

    1) ... should add some TutorialToken to the Uniswap pool

    Events emitted during test:
    ---------------------------

    Approval(owner: <indexed>, spender: <indexed>, value: 100000000)

    ---------------------------

...

  1) Contract: uniswap_factory
       ... should add some TutorialToken to the Uniswap pool:
     Error: Returned error: VM Exception while processing transaction: invalid JUMP at c03c510f66214a6f07445cebf4484b987f6e99dba476ffd7e760307e3b57bcc3/c4954161e52bb1507cf4927b047cc0d904598510:39
      at Object.ErrorResponse (node_modules/truffle/build/webpack:/~/web3-eth/~/web3-core-helpers/src/errors.js:29:1)
      at node_modules/truffle/build/webpack:/~/web3-eth/~/web3-core-requestmanager/src/index.js:140:1
      at Object.<anonymous> (node_modules/truffle/build/webpack:/packages/truffle-provider/wrapper.js:112:1)
      at node_modules/truffle/build/webpack:/~/web3/~/web3-providers-ws/src/index.js:121:1
      at Array.forEach (<anonymous>)
      at W3CWebSocket.WebsocketProvider.connection.onmessage (node_modules/truffle/build/webpack:/~/web3/~/web3-providers-ws/src/index.js:98:1)
      at W3CWebSocket._dispatchEvent [as dispatchEvent] (node_modules/truffle/build/webpack:/~/yaeti/lib/EventTarget.js:107:1)
      at W3CWebSocket.onMessage (node_modules/truffle/build/webpack:/~/websocket/lib/W3CWebSocket.js:234:1)
      at WebSocketConnection.<anonymous> (node_modules/truffle/build/webpack:/~/websocket/lib/W3CWebSocket.js:205:1)
      at WebSocketConnection.processFrame (node_modules/truffle/build/webpack:/~/websocket/lib/WebSocketConnection.js:552:1)
      at node_modules/truffle/build/webpack:/~/websocket/lib/WebSocketConnection.js:321:34
      at processTicksAndRejections (internal/process/next_tick.js:74:9)
BlinkyStitt commented 5 years ago

If I try to truffle debug $tx, I get an error. Probably because the uniswap truffle artifacts only have the bytecode and nothing else in them.

Gathering transaction data...

TypeError: Cannot read property 'id' of undefined
    at newScope.variables.concat.filter.variable (/usr/src/app/node_modules/truffle/build/webpack:/packages/truffle-debugger/dist/debugger.js:1738:1)
    at Array.filter (<anonymous>)
    at map (/usr/src/app/node_modules/truffle/build/webpack:/packages/truffle-debugger/dist/debugger.js:1735:1)
    at Array.map (<anonymous>)
    at data.info.scopes._ (/usr/src/app/node_modules/truffle/build/webpack:/packages/truffle-debugger/dist/debugger.js:1720:125)
    at /usr/src/app/node_modules/truffle/build/webpack:/~/reselect/lib/index.js:76:1
    at /usr/src/app/node_modules/truffle/build/webpack:/~/reselect/lib/index.js:36:1
    at /usr/src/app/node_modules/truffle/build/webpack:/~/reselect/lib/index.js:90:1
    at Function._ (/usr/src/app/node_modules/truffle/build/webpack:/~/reselect/lib/index.js:36:1)
    at args (/usr/src/app/node_modules/truffle/build/webpack:/~/reselect-tree/index.js:209:1)
    at /usr/src/app/node_modules/truffle/build/webpack:/~/reselect/lib/index.js:86:1
    at Function._ (/usr/src/app/node_modules/truffle/build/webpack:/~/reselect/lib/index.js:36:1)
    at args (/usr/src/app/node_modules/truffle/build/webpack:/~/reselect-tree/index.js:209:1)
    at /usr/src/app/node_modules/truffle/build/webpack:/~/reselect/lib/index.js:86:1
    at /usr/src/app/node_modules/truffle/build/webpack:/~/reselect/lib/index.js:36:1
    at runSelectEffect (/usr/src/app/node_modules/truffle/build/webpack:/~/@redux-saga/core/dist/redux-saga-core.esm.js:723:1)
    at runEffect (/usr/src/app/node_modules/truffle/build/webpack:/~/@redux-saga/core/dist/redux-saga-core.esm.js:1191:1)
    at digestEffect (/usr/src/app/node_modules/truffle/build/webpack:/~/@redux-saga/core/dist/redux-saga-core.esm.js:1258:1)
    at next (/usr/src/app/node_modules/truffle/build/webpack:/~/@redux-saga/core/dist/redux-saga-core.esm.js:1148:1)
    at currCb (/usr/src/app/node_modules/truffle/build/webpack:/~/@redux-saga/core/dist/redux-saga-core.esm.js:1238:1)
    at checkEnd (/usr/src/app/node_modules/truffle/build/webpack:/~/@redux-saga/core/dist/chunk-585b854f.js:154:1)
    at chCbAtKey (/usr/src/app/node_modules/truffle/build/webpack:/~/@redux-saga/core/dist/chunk-585b854f.js:170:1)
    at Object.currCb [as cont] (/usr/src/app/node_modules/truffle/build/webpack:/~/@redux-saga/core/dist/redux-saga-core.esm.js:1238:1)
    at end (/usr/src/app/node_modules/truffle/build/webpack:/~/@redux-saga/core/dist/redux-saga-core.esm.js:1001:1)
    at Object.task.cont (/usr/src/app/node_modules/truffle/build/webpack:/~/@redux-saga/core/dist/redux-saga-core.esm.js:829:1)
    at next (/usr/src/app/node_modules/truffle/build/webpack:/~/@redux-saga/core/dist/redux-saga-core.esm.js:1157:1)
    at Object.currCb [as cont] (/usr/src/app/node_modules/truffle/build/webpack:/~/@redux-saga/core/dist/redux-saga-core.esm.js:1238:1)
    at end (/usr/src/app/node_modules/truffle/build/webpack:/~/@redux-saga/core/dist/redux-saga-core.esm.js:1001:1)
The above error occurred in task session.saga
Tasks cancelled due to error:
controller.saga
data.saga
evm.saga
solidity.saga
trace.saga
web3.saga
haydenadams commented 5 years ago

What happens if you set minLiquidity to 1?

Also if your eth balance and token balance are both 1000, maybe send less than 1000 (like 500 of each).

Also the abi is likely giving you issues b/c of vyper unit types that solidity doesnt have.

BlinkyStitt commented 5 years ago

Similar but not exactly the same error with minLiquidity of 1:

Error: Returned error: VM Exception while processing transaction: invalid JUMP at 377780708130f00ef852fcc086eded95c7eb0e896d0eb49c8c2fd4879bfdf303/014e15cbb300f9408e2886949053208fc5e07cd5:39

The eth and token balances far exceed 1000, so that isn’t the issue, but I will change the test to check for headroom.

I think I can get by using the abi of my solidity interface instead of the abi that is checked into the repo. My interface has worked for all the tests except addLiquidity so it’s at least mostly right. Am I doing the contract initialization correctly?

I’ve had to track down reverts in other code, but haven’t had to debut a JUMP before. What does the error even mean?

haydenadams commented 5 years ago

Your deploy order seems to be good.

The JUMP error is a Vyper bug. Basically Uniswap uses Vypers create_with_code_of() which was created before the REVERT opcode was added. It sets up a new contract with a delegatecall pattern to a library contract.

Unfortunately when REVERT was added, create_with_code_of() fell through the cracks so if a call fails on a proxy delegatecall contract created with this function, it throws by doing an invalid jump.

But yeah something is causing the addLiquidity function to fail.

Try calling tutorialTokenUniswapInstance.factoryAddress() and tutorialTokenUniswapInstance.tokenAddress() and make sure it outputs the expected addresses.

Do you have any tests for the tutorial token to make sure thats working?

BlinkyStitt commented 5 years ago

I do call factoryAddress and tokenAddress in the should have a pool for TutorialToken test and it passes.

BlinkyStitt commented 5 years ago

I have some tests on TutorialToken, but I’ll add some more. I’m using open zeppelin, so that stuff should all be up to standards.

On Feb 23, 2019, at 1:09 PM, Hayden Adams notifications@github.com wrote:

Your deploy order seems to be good.

The JUMP error is a Vyper bug. Basically Uniswap uses Vypers create_with_code_of() which was created before the REVERT opcode was added. It sets up a new contract with a delegatecall pattern to a library contract.

Unfortunately when REVERT was added, create_with_code_of() fell through the cracks so if a call fails on a proxy delegatecall contract created with this function, it throws by doing an invalid jump.

But yeah something is causing the addLiquidity function to fail.

Try calling tutorialTokenUniswapInstance.factoryAddress() and tutorialTokenUniswapInstance.tokenAddress() and make sure it outputs the expected addresses.

Do you have any tests for the tutorial token to make sure thats working?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

BlinkyStitt commented 5 years ago

This is my TutorialToken.sol. It is pretty simple so I doubt I've added any bugs here.

pragma solidity 0.5.4;

import "openzeppelin-solidity/contracts/token/ERC20/ERC20.sol";

contract TutorialToken is ERC20 {
    string public name = "TutorialToken";
    string public symbol = "TT";
    uint8 public decimals = 18;
    uint public INITIAL_SUPPLY = 1000000000000;

    constructor() public {
        _mint(msg.sender, INITIAL_SUPPLY);
    }
}
haydenadams commented 5 years ago

@WyseNynja I figured it out.

https://github.com/Uniswap/contracts-vyper/blob/master/contracts/uniswap_exchange.vy#L65

First liquidity provider needs to add at least 1000000000 wei to the contract.

BlinkyStitt commented 5 years ago

Thank you!! Will change that when I get home and test it out

On Feb 24, 2019, at 11:10 AM, Hayden Adams notifications@github.com wrote:

@WyseNynja I figured it out.

https://github.com/Uniswap/contracts-vyper/blob/master/contracts/uniswap_exchange.vy#L65

First liquidity provider needs to add at least 1000000000 wei to the contract.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

haydenadams commented 5 years ago

Fo sho lmk if there are still issues

BlinkyStitt commented 5 years ago

New test:

  it("... should add some TutorialToken to the Uniswap pool", async () => {
    await UniswapExchange.deployed();
    await UniswapFactory.deployed();
    const uniswapFactoryInstance = await IUniswapFactory.at(UniswapFactory.address);

    const tutorialTokenInstance = await TutorialToken.deployed();
    const tutorialTokenUniswapAddress = await uniswapFactoryInstance.getExchange(TutorialToken.address);

    await tutorialTokenInstance.approve(tutorialTokenUniswapAddress, 1500000000);

    const tutorialTokenUniswapInstance = await IUniswapExchange.at(tutorialTokenUniswapAddress);

    const balanceETH = web3.utils.toBN(await web3.eth.getBalance(accounts[0]));
    assert(balanceETH.gt(1500000000));

    const balanceTT = await tutorialTokenInstance.balanceOf(accounts[0]);
    assert.isAtLeast(balanceTT.toNumber(), 1500000000);

    const current_block = await web3.eth.getBlock(await web3.eth.getBlockNumber());

    await tutorialTokenUniswapInstance.addLiquidity(1000000000, 1000000000, current_block.timestamp + 300, {value: 1000000000});
  });

Sweet success:

  Contract: uniswap_factory
    ✓ ... should be initializable through the interface (356200 gas)
    ✓ ... should have a pool for TutorialToken (250727 gas)
    ✓ ... should add some TutorialToken to the Uniswap pool (155332 gas)
liamzebedee commented 5 years ago

@haydenadams any reason why 1000000000 wei was chosen? Out of pure curiosity :)

antoncoding commented 4 years ago

Thanks a lot @WyseNynja @haydenadams , this saved my day!