delegateBySig signature mismatch

Support Contracts
1

We are using OpenZeppelin's ERC20 and especiall ERC20Votes.

There, we are trying to use the delegateBySig function. Unfortunately, we are seeing some mismatch between the front-end signature and the ERC20 contract.

Using the following elements:

const domain = {
      "name": "Unlock Discount Token",
      "version": "1",
      "chainId": 1,
      "verifyingContract": "0x90DE74265a416e1393A450752175AED98fe11517"
    }

    const types = {
      'Delegation': [
        { name: 'delegatee', type: 'address' },
        { name: 'nonce', type: 'uint256' },
        { name: 'expiry', type: 'uint256' },
      ]
    }

    const message = {
      "delegatee": "0x9dED35Aef86F3c826Ff8fe9240f9e7a9Fb2094e5",
      "nonce": 0,
      "expiry": ethers.BigNumber.from("1634160375")
    }

We prompted a user to sign and they signature we got is this one:

    const v = 28;
    const r = "0x2d89602ddbb9baaf2f8cf7d839880ff3df556ebc1730adcc0cb7882a82b003f4";
    const s = "0x5c20eda96bdada01f35af58d682bfec930c0f44628560be92cd770938efcf3e7";

When using ethers verifyTypedData we get the address of the signer (0x245DC69FD58dd92C0B3298b71fA58CC299E79042)

ethers.utils.verifyTypedData( domain , types , message , {v,r,s} )

However, when the transaction is submitted, for a reason that we do not understand the contract recovers a completely different one: 0xc9f0970a72dc8a5832b48d2a0fb9c75043cdb474.

You can see the transaction there: https://etherscan.io/tx/0x6dc37b39c9ef967b85d25d47d3c2e21926f831e85e647ea2df91a60bf7fff99d#eventlog (check the logs for the DelegateChanged event).

Any help would be greatly appreciated.

2

For reference for anyone reading. We debugged this and found the cause, will share details later.

1 Like
3

Hi! I wanted to give an update to everyone watching this from home :slight_smile: !

tl;dr: It turns out that when we upgraded our ERC20 and added the ERC20Votes extensions, as well as a few other ones, we failed to initialize them.

Now, more details. When we introduced our governance token, back in November, we of course used the OpenZeppelin libraries. It was a very "basic" ERC20, deployed using the proxy pattern to make it upgradable.

/**
* @title The Unlock Discount Token
* This smart contract implements the Unlock Discount Token
*/
contract UnlockDiscountToken is
ERC20Mintable,
ERC20Detailed
{
  /**
   * @notice A one-time call to configure the token.
   * @param _minter A wallet with permissions to mint tokens and/or add other minters.
   */
  function initialize(address _minter) public initializer()
  {
    ERC20Mintable.initialize(_minter);
    ERC20Detailed.initialize('Unlock Discount Token', 'UDT', 18);
  }
}

This summer, as we started working on the DAO, we had to upgrade the contract, we realized that the upgrade would not be as straightforward because of some changes in the ERC20 library provided by OZ. The biggest "change" was the introduction of some __gap spaces that were not part of the initial contract storage. For that reason, we had to do some fine tuning in the contract's storage by flattening it into a single .sol file. Doing this was the easiest way to make things work, but also increased the complexity of the code significantly.

As a result of this, we completely missed that some of the new capabilities of our contract had to be initialized as part of the initialize function, and in particular, the EIP712, ERC20Permit and ERC20Votes extensions...

The initialization process actually sets very important elements, such as the hashes used to verify signatures for permit and delegateBySig!

To fix this, we had to perform another upgrade, because these initializers can only be called once. The upgrade basically consisted in adding a initialize2 function that could also be called only once, and that would call the internal init functions that we missed. See the code there.

Note: as we investigate this, we found an (independent) critical vulnerability in our contract that we patched at the same time.

1 Like
4

I'm facing the same issue. Can you share more details please?
TIA

5

@Keyser_Soze The comment by @julien51 explains what was the issue!

The initializer for ERC20Votes hadn't been invoked.