Contract that is OpenZeppelin2771ContractUpgradeable failing in test environment when used with OpenGSN due to a isTrustedForwarder failure

:computer: Environment

"@nomiclabs/hardhat-ethers": "^2.0.0",
"@nomiclabs/hardhat-waffle": "^2.0.0",
"@opengsn/gsn": "^2.1.0",
"@openzeppelin/hardhat-upgrades": "^1.6.0",
"@types/chai": "^4.2.14",
"@types/mocha": "^8.2.0",
"@types/node": "^14.14.21",
"chai": "^4.2.0",
"ethereum-waffle": "^3.0.0",
"ethers": "^5.0.0",
"hardhat": "^2.0.8",
"ts-node": "^9.1.1",
"typescript": "^4.1.3"

:memo:Details

My contract currently uses OpenGSN and initializes and deploys the contract with an ethers signer. Passing this contract object to deployProxy causes a timeout during testing. This problem does not occur when a web3Provider is not specified and the hardhat defaults are used instead. Unfortunately, I need to use a hardhat node running in a different terminal when testing OpenGSN, so I’m trying to get this working.

:1234: Code to reproduce

pragma solidity ^0.8.0;

import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";

contract Box is Initializable {
  uint256 private value;

  event ValueChanged(uint256 newValue);

  function store(uint256 newValue) public {
    value = newValue;
    emit ValueChanged(newValue);
  }

  function retrieve() public view returns (uint256) {
    return value;
  }
}

The following test causes the hang/timeout. You’ll need to run yarn hardhat node in another terminal window before executing the test:

const { expect } = require('chai');
const Web3HttpProvider = require("web3-providers-http");
import { ethers, upgrades } from "hardhat";
 
describe('Box (proxy)', function () {
  beforeEach(async function () {
    const web3provider = new Web3HttpProvider("http://localhost:8545");
    const deploymentProvider = new ethers.providers.Web3Provider(web3provider);

    const Box = await ethers.getContractFactory("Box", deploymentProvider.getSigner());
    const box = await upgrades.deployProxy(Box, [42], {initializer: 'store'});
  });
 
  // Test case
  it('retrieve returns a value previously initialized', async function () {
    expect((await box.retrieve()).toString()).to.equal('42');
  });
});

Hi varunsrin! Welcome to the forums.

Do you know where in the test it starts to hang?

When debugging I usually put several console.log(“hereXYZ”) statements so I can figure out where in my code it messes up.

Once you do that you can figure out which statement is giving you trouble. Let me know where you figure out the hang happens, and we can debug from there.

The test hangs on this line:

const box = await upgrades.deployProxy(Box, [42], {initializer: 'store'});

OK, after tweaking some more I’ve manged to get it working except when using it with OpenGSN. Updating the title to reflect this. Here is the minimum reproduction of the error state alongside some passing test cases.

pragma solidity ^0.8.0;

import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import {BaseRelayRecipient} from "./BaseRelayRecipient.sol";

contract Box is BaseRelayRecipient, Initializable {
  string public override versionRecipient;
  uint256 private value;

  function initialize(address _forwarder) public initializer {
    trustedForwarder = _forwarder;
    versionRecipient = "2.0.0";
  }

  // Stores a new value in the contract
  function store(uint256 newValue) public {
    value = newValue;
  }

  // Reads the last stored value
  function retrieve() public view returns (uint256) {
    return value;
  }
}

I also inlined BaseRelayRecipient and made a few tweaks to get it to work in Solidity 0.8.0. Namely, change the _msgSender return type to not include payable per: https://github.com/opengsn/gsn/issues/631

// SPDX-License-Identifier:MIT
// solhint-disable no-inline-assembly
pragma solidity ^0.8.0;

import "./IRelayRecipient.sol";

import "hardhat/console.sol";

/**
 * A base contract to be inherited by any contract that want to receive relayed transactions
 * A subclass must use "_msgSender()" instead of "msg.sender"
 */
abstract contract BaseRelayRecipient is IRelayRecipient {
  /*
   * Forwarder singleton we accept calls from
   */
  address public trustedForwarder;

  function isTrustedForwarder(address forwarder) public view override returns (bool) {
    return forwarder == trustedForwarder;
  }

  /**
   * return the sender of this call.
   * if the call came through our trusted forwarder, return the original sender.
   * otherwise, return `msg.sender`.
   * should be used in the contract anywhere instead of msg.sender
   */
  function _msgSender() internal view virtual override returns (address ret) {
    if (msg.data.length >= 20 && isTrustedForwarder(msg.sender)) {
      // At this point we know that the sender is a trusted forwarder,
      // so we trust that the last bytes of msg.data are the verified sender address.
      // extract sender address from the end of msg.data
      assembly {
        ret := shr(96, calldataload(sub(calldatasize(), 20)))
      }
    } else {
      return msg.sender;
    }
  }

  /**
   * return the msg.data of this call.
   * if the call came through our trusted forwarder, then the real sender was appended as the last 20 bytes
   * of the msg.data - so this method will strip those 20 bytes off.
   * otherwise, return `msg.data`
   * should be used in the contract instead of msg.data, where the difference matters (e.g. when explicitly
   * signing or hashing the
   */
  function _msgData() internal view virtual override returns (bytes memory ret) {
    if (msg.data.length >= 20 && isTrustedForwarder(msg.sender)) {
      return msg.data[0:msg.data.length - 20];
    } else {
      return msg.data;
    }
  }
}

Finally here are the test cases. The short version is that testing the contract directly or using either OpenGSN or OpenZeppelin’s Upgradeable works, but using them together fails:

import { RelayProvider } from "@opengsn/gsn";
import { GsnTestEnvironment } from "@opengsn/gsn/dist/GsnTestEnvironment";
import { Contract, Wallet } from "ethers";
import { ethers, upgrades } from "hardhat";
import { JsonRpcSigner } from "@ethersproject/providers/src.ts";
import { expect } from "chai";
const Web3HttpProvider = require("web3-providers-http");

/**
 * In this test case we do not use the OpenZeppelin upgradeable deployProxy method
 * to deploy the contract, and everything works as expected with OpenGSN.
 */

describe("Box test passes with OpenGSN", function () {
  let box1: Contract;

  let account: Wallet;
  let signer: JsonRpcSigner;
  let etherProvider: any;

  beforeEach(async function () {
    let env = await GsnTestEnvironment.startGsn("localhost");
    const { paymasterAddress, forwarderAddress } = env.contractsDeployment;

    const web3provider = new Web3HttpProvider("http://localhost:8545");
    account = new ethers.Wallet(Buffer.from("1".repeat(64), "hex"));

    const deploymentProvider = new ethers.providers.Web3Provider(web3provider);
    const Box = await ethers.getContractFactory("Box", deploymentProvider.getSigner());

    box1 = await Box.deploy();
    await box1.deployed();
    await box1.initialize(forwarderAddress);

    const config = { paymasterAddress: paymasterAddress, forwarderAddress: forwarderAddress };
    const gsnProvider = await RelayProvider.newProvider({ provider: web3provider, config }).init();
    gsnProvider.addAccount(account.privateKey);
    etherProvider = new ethers.providers.Web3Provider(gsnProvider);

    // This is a JsonRpcSigner exported by Ethers.
    signer = etherProvider.getSigner(account.address);
  });

  it("initializes and returns a value", async function () {
    expect((await box1.connect(signer).store(42)));
    expect((await box1.connect(signer).retrieve()).toString()).to.equal("42");
  });
});


/**
 * In this test case we do not use OpenGSN and everything works fine with OpenZeppelin's deployProxy
 */

describe("Box test passes with OpenZeppelin upgradeable", function () {
  let box2: Contract;

  beforeEach(async function () {
    const [deployer] = await ethers.getSigners();
    const Box = await ethers.getContractFactory("Box", deployer);
    box2 = await upgrades.deployProxy(Box, [await deployer.getAddress()]);
  });

  it("initializes and returns a value", async function () {
    // This fails if you attempt to connect with deployer.
    expect((await box2.store(42)));
    expect((await box2.retrieve()).toString()).to.equal("42");
  });
});


/**
 * In this test case we use OpenGSN and OpenZeppelin Contract Upgradeable. The contract is deployed with
 * the default hardhat provider while we spin up a new provider for the relayer. All write calls via the 
 * relayer fail with this message: 
 * 
 *      Error: Failed to relay call. Results: Relaying errors (1): http://127.0.0.1:53790 => 
 *      paymaster rejected in local view call to 'relayCall()' : isTrustedForwarder reverted
 */

describe("Box test failure #1 ", function () {
  let box3: Contract;

  let account: Wallet; 
  let signer: JsonRpcSigner;
  let etherProvider: any;

  beforeEach(async function () {
    let env = await GsnTestEnvironment.startGsn("localhost");
    const { paymasterAddress, forwarderAddress } = env.contractsDeployment;
    const web3provider = new Web3HttpProvider("http://localhost:8545");

    const Box = await ethers.getContractFactory("Box");
    box3 = await upgrades.deployProxy(Box, [forwarderAddress]);

    const config = { paymasterAddress: paymasterAddress, forwarderAddress: forwarderAddress };
    const gsnProvider = await RelayProvider.newProvider({ provider: web3provider, config }).init();

    account = new ethers.Wallet(Buffer.from("1".repeat(64), "hex"));
    gsnProvider.addAccount(account.privateKey);
    etherProvider = new ethers.providers.Web3Provider(gsnProvider);
    signer = etherProvider.getSigner(account.address);
  });

  it("initializes and returns a value", async function () {
    // Error: Failed to relay call happens here.
    expect((await box3.connect(signer).store(41)));
    expect((await box3.retrieve()).toString()).to.equal("42");
  });
});


/**
 * In this test case we use OpenGSN and OpenZeppelin Contract Upgradeable. The contract is deployed with the 
 * provider that we set up for OpenGSN, but the deployment fails without a reason. Shouldn't this fail
 * due to no funds?
 * 
 * "before each" hook for "initializes and returns a value":
 *   Error: Error: Transaction reverted without a reason
 */

describe("Box test failure #2 ", function () {
  let box4: Contract;

  let account: Wallet; 
  let signer: JsonRpcSigner;
  let etherProvider: any;

  beforeEach(async function () {
    let env = await GsnTestEnvironment.startGsn("localhost");
    const { paymasterAddress, forwarderAddress } = env.contractsDeployment;
    const web3provider = new Web3HttpProvider("http://localhost:8545");

    const deploymentProvider = new ethers.providers.Web3Provider(web3provider);
    const Box = await ethers.getContractFactory("Box", deploymentProvider.getSigner());
     // Error happens when deploying the contract.
    box4 = await upgrades.deployProxy(Box, [forwarderAddress]);

    const config = { paymasterAddress: paymasterAddress, forwarderAddress: forwarderAddress };
    const gsnProvider = await RelayProvider.newProvider({ provider: web3provider, config }).init();


    account = new ethers.Wallet(Buffer.from("1".repeat(64), "hex"));
    gsnProvider.addAccount(account.privateKey);
    // TypeError: Argument of type 'RelayProvider' is not assignable to
    //      parameter of type 'ExternalProvider | JsonRpcFetchFunc'.
    etherProvider = new ethers.providers.Web3Provider(gsnProvider);
    signer = etherProvider.getSigner(account.address);
  });

  it("initializes and returns a value", async function () {
    expect((await box4.connect(signer).store(41)));
    expect((await box4.retrieve()).toString()).to.equal("42");
  });
});

I can see that once you create a proxy, isTrustedForwarder() fails.
I suggest add a test that indeed you have the forwarder is initialized properly in the proxy:

expect(await box.trustedForwarder()).to.equal(forwarder);

Another alternative:
you can use OpenZeppelin’s implementation of BaseRelayRecipient, named ERC2771Context
This base contract is ABI-compatible with BaseRelayRecipient.
Just note that it has an immutable forwarder, so it has to be initialized at proxy template creation, not at deployment. I’m not sure how to use deployProxy() in this case.

Thanks for the pointers dror, they were very helpful.

  1. I upgraded the contract to ERC2771ContextUpgradeable which is an upgrade friendly version of the contract you linked above, which allows me to set the forwarder on deploy.

  2. I added an assert statement similar to the one you recommended

    expect(await box3.isTrustedForwarder(forwarder)).to.equal(true);

Outcome

The new assert passes indicating that the forwarder is set correctly, but the original test fails with the same message:

Error: Failed to relay call. Results:
Relaying errors (1):
http://127.0.0.1:57668 => paymaster rejected in local view call to 'relayCall()' : isTrustedForwarder reverted

Additional Exploration

It also appears that connecting the signer and making a read call results in the relay reverting the transaction. See the test suite at the bottom of this code sample for details on the failure:

pragma solidity ^0.8.0;

import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/metatx/ERC2771ContextUpgradeable.sol";

contract Box is Initializable, ERC2771ContextUpgradeable {
  uint256 private value;

  function initialize(address _forwarder) public initializer {
    _trustedForwarder = _forwarder;
  }

  // Stores a new value in the contract
  function store(uint256 newValue) public {
    value = newValue;
  }

  // Reads the last stored value
  function retrieve() public view returns (uint256) {
    return value;
  }
}

The first test passes and the second one fails

describe("Box test failure #1 ", function () {
  let box3: Contract;

  let account: Wallet; 
  let signer: JsonRpcSigner;
  let etherProvider: any;

  let forwarder: string | undefined;

  beforeEach(async function () {
    let env = await GsnTestEnvironment.startGsn("localhost");
    const { paymasterAddress, forwarderAddress } = env.contractsDeployment;
    const web3provider = new Web3HttpProvider("http://localhost:8545");

    const Box = await ethers.getContractFactory("Box");
    box3 = await upgrades.deployProxy(Box, [forwarderAddress]);

    const config = { paymasterAddress: paymasterAddress, forwarderAddress: forwarderAddress };
    const gsnProvider = await RelayProvider.newProvider({ provider: web3provider, config }).init();

    account = new ethers.Wallet(Buffer.from("1".repeat(64), "hex"));
    gsnProvider.addAccount(account.privateKey);
    etherProvider = new ethers.providers.Web3Provider(gsnProvider);
    signer = etherProvider.getSigner(account.address);

    forwarder = forwarderAddress
  });


  it("initializes and returns a value", async function () {
    // This passes.
    expect(await box3.isTrustedForwarder(forwarder)).to.equal(true);
    console.log("The forwarder address is:", forwarder);

    // This fails and stops the test suite.
    expect((await box3.connect(signer).store(41)));

    // If allowed to run, this will fail and stop the suite with Error: Error: Transaction reverted without a reason
    expect((await box3.connect(signer).retrieve()).toString()).to.equal("42");

    // If allowed to run, this test will pass. 
    expect((await box3.retrieve()).toString()).to.equal("0");
  });
});

However, it

I also tried deploying the contract to Rinkeby, and the contract works as intended. This leads me to believe that something in the local test configuration is the culprit.