Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues with OpenZeppelin nonReentrant modifier on test #641

Closed
eboadom opened this issue Jun 9, 2020 · 1 comment
Closed

Issues with OpenZeppelin nonReentrant modifier on test #641

eboadom opened this issue Jun 9, 2020 · 1 comment
Assignees

Comments

@eboadom
Copy link

eboadom commented Jun 9, 2020

In a setup with Buidler + ethersjs + Waffle + typechain, I'm trying to run a test file with the following content:

import {evmRevert} from "../helpers/misc-utils";
import {
  TEST_SNAPSHOT_ID,
  APPROVAL_AMOUNT_LENDING_POOL_CORE,
} from "../helpers/constants";
import {AToken} from "../types/AToken";
import {MintableErc20} from "../types/MintableErc20";
import {LendingPool} from "../types/LendingPool";
import {LendingPoolCore} from "../types/LendingPoolCore";
import {
  getAaveProtocolTestHelpers,
  getMintableErc20,
  getAToken,
  convertToCurrencyDecimals,
  getEthersSigners,
  getLendingPoolCore,
  getLendingPool,
} from "../helpers/contracts-helpers";
import {Signer} from "ethers";

describe("AToken: Transfer", () => {
  let deployer: Signer;
  let users: Signer[];
  let _aDai: AToken;
  let _dai: MintableErc20;
  let _lendingPool: LendingPool;
  let _lendingPoolCore: LendingPoolCore;

  before(async () => {
    await evmRevert(TEST_SNAPSHOT_ID);

    const [_deployer, ..._users] = await getEthersSigners();
    deployer = _deployer;
    users = _users;

    _lendingPool = await getLendingPool();
    _lendingPoolCore = await getLendingPoolCore();

    const testHelpers = await getAaveProtocolTestHelpers();

    const aDaiAddress = (await testHelpers.getAllATokens()).find(
      (aToken) => aToken.symbol === "aDAI"
    )?.tokenAddress;

    const daiAddress = (await testHelpers.getAllReservesTokens()).find(
      (token) => token.symbol === "DAI"
    )?.tokenAddress;
    if (!aDaiAddress) {
      console.log(`atoken-modifiers.spec: aDAI not correctly initialized`);
      process.exit(1);
    }
    if (!daiAddress) {
      console.log(`atoken-modifiers.spec: DAI not correctly initialized`);
      process.exit(1);
    }

    _aDai = await getAToken(aDaiAddress);
    _dai = await getMintableErc20(daiAddress);
  });

  it("User 0 deposits 1000 DAI, transfers to user 1", async () => {
    await _dai
      .connect(users[0])
      .mint(await convertToCurrencyDecimals(_dai.address, "1000"));

    console.log(_lendingPoolCore.address);

    await _dai
      .connect(users[0])
      .approve(_lendingPoolCore.address, APPROVAL_AMOUNT_LENDING_POOL_CORE);

    //user 1 deposits 1000 DAI
    const amountDAItoDeposit = await convertToCurrencyDecimals(
      _dai.address,
      "1000"
    );

    await _lendingPool
      .connect(users[0])
      .deposit(_dai.address, amountDAItoDeposit, "0");
  });
});

The following error appears when calling

await _lendingPool
      .connect(users[0])
      .deposit(_dai.address, amountDAItoDeposit, "0");
Error: VM Exception while processing transaction: revert ReentrancyGuard: reentrant call
      at new TransactionExecutionError (node_modules/@nomiclabs/buidler/src/internal/buidler-evm/provider/errors.ts:93:16)
      at BuidlerNode._manageErrors (node_modules/@nomiclabs/buidler/src/internal/buidler-evm/provider/node.ts:1078:14)
      at BuidlerNode.estimateGas (node_modules/@nomiclabs/buidler/src/internal/buidler-evm/provider/node.ts:605:27)
      at runMicrotasks (<anonymous>)
      at processTicksAndRejections (internal/process/task_queues.js:97:5)
      at EthModule._estimateGasAction (node_modules/@nomiclabs/buidler/src/internal/buidler-evm/provider/modules/eth.ts:388:9)
      at BuidlerEVMProvider.send (node_modules/@nomiclabs/buidler/src/internal/buidler-evm/provider/provider.ts:82:14)
      at EthersProviderWrapper.send (node_modules/@nomiclabs/buidler-ethers/src/ethers-provider-wrapper.ts:13:20)
      at async Promise.all (index 1)
      at async Promise.all (index 0)

But this can't be related with being attempting a reentrancy on the LendingPool contract, as the call to deposit() is exactly the same as here https://github.com/aave/aave-protocol/blob/13f00958cfac4eaa916555276d243a8e04d40157/contracts/lendingpool/LendingPool.sol#L299 where there is no nested called to the LendingPool

Before the execution of this test, there is only a setup phase where all the necessary contracts are deployed and the evm snapshot is done. The only other test in the mocha suite doesn't involve the LendingPool contract.

@eboadom
Copy link
Author

eboadom commented Jun 9, 2020

It was not a problem with buidler, but with the version of the ReentrancyGuard contract we were using because of what was solved here OpenZeppelin/openzeppelin-contracts#2171

We were using "@openzeppelin/contracts": "3.0.1" on this new repository, on on that version, the ReentrancyGuard doesn't work correctly on our proxied LendingPool

@eboadom eboadom closed this as completed Jun 9, 2020
@fvictorio fvictorio self-assigned this Nov 27, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants