Revert messages are not bubbled in proxy constructors

[ppoliani]

I'm trying to write some tests to check if some of the user inputs are correct. More specifically I've got the following code in my smart contract function. The code is part of the initialize function.

contract ContractA is Initializable {
  function initialize(IERC20 token) public initializer {
      require(
      address(token) != address(0), 
      "Invalid erc20 address provided"
    );
  }
} 

I have written a test that checks if the correct message is thrown when a 0x address is passed. I used the helper

const deployContract = async  () => {
  const token = await ERC20.new('TokenA', 'TokenA')
  const opts = {
    unsafeAllowLinkedLibraries: true
   }

return await deployProxy(
    ContractA, 
    [token],
    opts
  )
}

const {constants, expectRevert} = require('@openzeppelin/test-helpers');

it('should revert', async () => {
    await expectRevert(
      deployContract(constants.ZERO_ADDRESS, validatorRegistry.address),
      'Invalid erc20 address provided',
    );
}) 

Now when I run this it revert, however the error I get doesn't include the message defined in the require() section. Instead I get the default message -revert

     Wrong kind of exception received
      + expected - actual

      -revert
      +Invalid erc20 address provided

Now if I deploy the contract user .new function and manually invoking the initialize function then I get the correct message.

const deployContract = async  () => {
  const token = await ERC20.new('TokenA', 'TokenA')
  const contract =  await ContractA.new()
  await contract.initialize(token)
}

I'm using:

• @openzeppelin/truffle-upgrades: 1.3.0
• @openzeppelin/test-helpers": "^0.5.9"
• "ganache-cli": "^6.12.1"
• "truffle": "^5.1.55"

[ppoliani]

Looking into the source code I stumdled across this

contract UpgradeabilityProxy is Proxy {
  constructor(address _logic, bytes memory _data) public payable {
     assert(IMPLEMENTATION_SLOT == bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1));
     _setImplementation(_logic);
      if(_data.length > 0) {
        (bool success,) = _logic.delegatecall(_data);
        require(success);
      }
  }  
...
}

Is it because of the (bool success,) = _logic.delegatecall(_data); which presumably doesn't bubble up exceptions?

[frangio]

Thanks for reporting! Yes, it seems that we're not bubbling the revert reason in the constructor! (It is bubbled in the fallback function.) Unfortunately I don't think there's a workaround for you to be able to write the tests you want.

I'm going to move this issue to our OpenZeppelin Contracts repo where proxies are hosted. We should fix it over there. We have to update the Upgrades Plugins to use those (see OpenZeppelin/openzeppelin-upgrades#164).

[frangio]

Fixed by #2454.