Problem with ECDSA signatures

[sullof]

In my smart contract a function can be executed only if signed by an oracle (which validates the data before generating the transaction). The solidity code uses OpenZeppelin's ECDSA.sol and looks like:

function isSignedByOracle(
        bytes32 _hash,
        bytes memory _signature
    ) public view
    returns (bool)
    {
        return oracle == ECDSA.recover(_hash, _signature);
    }

The hash is calculated like in this example:

require(
            isSignedByOracle(keccak256(abi.encode(
                getChainId(),
                msg.sender,
                _appId,
                _id,
                _timestamp
            )), _signature),
            "Invalid signature"
        );

I had hard time to find a way to generate a working signature.
I ended up with this function:

function ECDSASign(ethers, hexPrivateKey, types, values) {
    const abiEncoded = ethers.utils.defaultAbiCoder.encode(types, values)
    const hash = ethers.utils.keccak256(abiEncoded)
    const signingKey = new ethers.utils.SigningKey(hexPrivateKey)
    const signedDigest = signingKey.signDigest(hash)
    return ethers.utils.joinSignature(signedDigest)
  }

I call it like

ECDSASign(
        ethers,
        '0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d',
        ['uint256', 'address', 'uint256', 'uint256', 'uint256'],
        [chainId, address, appId, id, timestamp]
    )

I use hardhat for test and deployment, and use the same process to deploy the smart contracts during tests and actual deployment to networks.

Problem is that during tests everything works fine, but, when I try to do initialize a mapping during the deployment to localhost:8545 (served by hardhat), the recovered address is not the expected one. I am getting crazy to understand why.

Any idea?

[sullof]

I solved it.

I modified the smart contract adding a public function that returns the keccak256 of the abi-encoded params.
This way, the ECDSASign changes as:

  function ECDSASign(ethers, hexPrivateKey, hash) {
    const signingKey = new ethers.utils.SigningKey(hexPrivateKey)
    const signedDigest = signingKey.signDigest(hash)
    return ethers.utils.joinSignature(signedDigest)
  }

I get the hash calling the function in the smart contract as

    hash = await contract.getHash(address, appId, id, timestamp)

instead that using

    const abiEncoded = ethers.utils.defaultAbiCoder.encode(types, values)
    const hash = ethers.utils.keccak256(abiEncoded)

as before.

It is very weird that ethers formats the data in a slightly different way during test and deployment. It looks like a bug.

[ricmoo]

Ethers has no way to determine the difference between production or testing (and no different logic), so it shouldn't behave any differently.

Can you include the content of your getHash method? Could you be computing the hash differently without realizing it?

Also note, the above code uses SigningKey and signs digests directly, which is incredibly dangerous and this type of signature recovery should not be done in production. It is also incompatible with almost every client (e.g. MetaMask, Geth, etc). You should look into using an EIP-191 signed message, using the signer.signMessage which is widely supported by all clients and prevents accidental theft of funds. :)

[sullof]

Thanks for the response.

In deploy.js I was calling the same helper I was calling during the tests. That is what makes this difference so weird. If I have time, I will extrapolate the code from my project and create a repo for it, so you can easily check it, if you like.

About the signature, I tried before to use signer.signMessage but it was recovering all the time the wrong address. So, after many attempts, I used that low level approach that works. It will be used by a server side oracle only, so it should be safe. But I will anyway try again to use EIP-191.

[sullof]

@ricmoo, I took a look at the EIP-191 standard. The approach is actually very similar. Mine also adds the chain ID — because Tweedentity is multi-chain. I could use the EIP-191 anyway. I will try to figure out if, in my case, it makes sense since the only signer is an off-line, nodejs oracle that authorizes or not accounts creations.

You were asking before about the getHash method. It was this

    const abiEncoded = ethers.utils.defaultAbiCoder.encode(types, values)
    const hash = ethers.utils.keccak256(abiEncoded)

[sullof]

So, I changed the hasher this way to support eip191

    function encodeForSignature(
        address _address,
        uint _appId,
        uint _id,
        uint _timestamp
    ) public view
    returns (bytes32)
    {
        return keccak256(abi.encodePacked(
                "\x19\x00",
                _address,
                getChainId(),
                _appId,
                _id,
                _timestamp
            ));
    }

I don't think that it is safer, in my specific case, but if it works anyway... why not 😁
Thanks again for the suggestion.