Unable to Expect Revert on Upgrades

[Metaxona]

Please add an option on upgrades where we can specify whether we expect a revert or not

Case: I was testing an upgrade where the upgrader has no authorization and it is not working since forge-std seemingly does not allow 2 expectRevert on top of each other

might be possible to do options like

expectRevert
errorSelector
errorBytes <encodeWithSelector>

which would allow users to specify a custom error in case they use a custom upgrade authorization logic and error

forge 0.2.0 (7bef9ca 2024-06-26T00:18:22.002685333Z)

[ericglau]

Would a try-catch pattern work for you instead? See this testcase for an example https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades/blob/4cd15fc50b141c77d8cc9ff8efb44d00e841a299/test/Upgrades.t.sol#L161-L169 where the try calls the upgrade via a contract instance that exists only in the tests, such as https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades/blob/4cd15fc50b141c77d8cc9ff8efb44d00e841a299/test/Upgrades.t.sol#L265-L273

[Metaxona]

Would a try-catch pattern work for you instead? See this testcase for an example https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades/blob/4cd15fc50b141c77d8cc9ff8efb44d00e841a299/test/Upgrades.t.sol#L161-L169 where the try calls the upgrade via a contract instance that exists only in the tests, such as https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades/blob/4cd15fc50b141c77d8cc9ff8efb44d00e841a299/test/Upgrades.t.sol#L265-L273

ohhhh didn't know solidity has a try catch, maybe i just forgot, will try if that will work. thanks

[ericglau]

Closing as this should be solvable with the above approach. If not, feel free to reopen.

[Metaxona]

Closing as this should be solvable with the above approach. If not, feel free to reopen.

apparently using try catch is not working since the function is internal and try catch does not work on those kind of functions, it can only be an external function or contract creation

http://docs.soliditylang.org/en/v0.8.20/grammar.html#a4.SolidityParser.tryStatement

http://docs.soliditylang.org/en/v0.8.26/grammar.html#a4.SolidityParser.tryStatement

any other type of function like an internal one that upgrades or deploy uses won't work

[ericglau]

@Metaxona You can wrap the internal function from an external one in a helper contract in your tests. For example:

contract UpgradeHelper {
    function upgradeProxy(address proxy, string memory contractName, bytes memory data) public {
        Upgrades.upgradeProxy(proxy, contractName, data);
    }
}

Then in your test:

    UpgradeHelper helper = new UpgradeHelper();
    try helper.upgradeProxy(proxy, "MyToken.sol", "") {
      fail();
    } catch (bytes memory err) {
      // check the error here
    } 

[Metaxona]

@Metaxona You can wrap the internal function from an external one in a helper contract in your tests. For example:

contract UpgradeHelper {
    function upgradeProxy(address proxy, string memory contractName, bytes memory data) public {
        Upgrades.upgradeProxy(proxy, contractName, data);
    }
}

Then in your test:

    UpgradeHelper helper = new UpgradeHelper();
    try helper.upgradeProxy(proxy, "MyToken.sol", "") {
      fail();
    } catch (bytes memory err) {
      // check the error here
    } 

hmmm definitely slipped my mind, thanks will try it later and hopefully it works