Best effort to decode nester error strings from reverts

[davecrighton]

Part of the changes for hyperledger/firefly#1717

The issue being addressed is that when solidity such as the following is used:

WillThrowError will = new WillThrowError();
           try will.aFunction() {
                _mint(to, 2000000); //if you reach this impossible statement you deserve 2 million tokens!
            }
            catch Error(string memory reason)  {
                revert(string.concat("[404]", "01c - caught error", reason));
            }

Where:

contract WillThrowError {
    error AnError(string message);
    function aFunction() public pure {
        revert AnError("I am an error");
    }
}

The resulting error message is not useful and appears garbled, for example:

FF10111: Error from ethereum connector: FF23021: EVM reverted: [404]01d - caught bytes:���������������������������������� ��������������������������������ޭ������������������������������

This PR moves evmconnect to use the new common revert processing defined in hyperledger/firefly-signer#98 which will attempt to decode nested errors to produce a more meaningful revert message such as:

[404]01d - caught bytes:AnError("I am an error")

This PR also introduces tests for using the new ff-signer functionality in-context.

[EnriqueL8]

first pass - @davecrighton can you sign off the commits for DCO please

[peterbroadhurst]

To aid a review @davecrighton - please could you provide in the subject:

• A Solidity example of the coding pattern that results in the problem
• A link to somewhere that helps us understand how standard this is
  • e.g. is it a link to the Solidity docs, or OpenZeppelin, or some forums on coding suggestions
• An description of what the algorithm is we're applying
  • I understand from your comment this only works for Error(string) "default" revert errors, but what are we inferring about the string placed in there and why

[peterbroadhurst]

Second request from me:

Can we make this a pkg utility function (honestly I think firefly-signer would be the ideal place, but we've started putting package utilities in firefly-evmconnect too a little recently).

This code feels important to standardize as DecodeRevertErrorBytes() and return a well structured struct return that is documented, that has a String() function.

[davecrighton]

A note on how standard this approach is:

We encountered this pattern in a complex enterprise solidity application where the contract author was making a significant number of calls to other contracts within a single contract execution and was using this as a way to provide context for the operation that was actually failing.

Having said that there are examples in the wild such as:

https://github.com/InjectiveLabs/solidity-contracts/blob/master/src/BankERC20.sol
https://github.com/zkemail/email-wallet/blob/main/packages/contracts/src/EmailWalletCore.sol
https://github.com/safe-fndn/safe-smart-account/blob/d7de455e0103bd3b6d1d6a4075d7c36d1869d677/contracts/common/StorageAccessible.sol

[davecrighton]

Second request from me:

Can we make this a pkg utility function (honestly I think firefly-signer would be the ideal place, but we've started putting package utilities in firefly-evmconnect too a little recently).

This code feels important to standardize as DecodeRevertErrorBytes() and return a well structured struct return that is documented, that has a String() function.

The standardization is now in this ff-signer PR hyperledger/firefly-signer#98

I have left this call site calling the UnwrapErrorStringCtx wrapper which is a thin wrapper around the DecodeRevertErrorCtx method which provides some nil handling behaviour that reduces boilerplate at the call site in this instance.

[davecrighton]

Note that for the moment I need a ff-signer psuedo version to build, once the ff-signer PR is merged I will drop these changes from the PR

[EnriqueL8]

Thanks @davecrighton