ECRecovery compiler warning

[linusnorton]

🎉 Description

ECRecovery generates a compiler warning with Solidity 0.4.24:

zeppelin-solidity/contracts/ECRecovery.sol:71:12: Warning: This function only accepts a single "bytes" argument. Please use "abi.encodePacked(...)" or a similar function to encode the data.
    return keccak256(
           ^ (Relevant source part starts here and spans across multiple lines).

• 🐛 This is a bug report.
• 📈 This is a feature request.

💻 Environment

Next, we need to know what your environment looks like.

• Which version of OpenZeppelin are you using?

1.10.0

• What network are you deploying to? Ganache? Ropsten?

Ganache

• How are you deploying your OpenZeppelin-backed contracts? truffle? Remix? Let us know!

Truffle

📝 Details

The code:

  function toEthSignedMessageHash(bytes32 hash)
    internal
    pure
    returns (bytes32)
  {
    // 32 is the length in bytes of hash,
    // enforced by the type signature above
    return keccak256(
      "\x19Ethereum Signed Message:\n32",
      hash
    );
  }

Should be:

  function toEthSignedMessageHash(bytes32 hash)
    internal
    pure
    returns (bytes32)
  {
    // 32 is the length in bytes of hash,
    // enforced by the type signature above
    return keccak256(abi.encodePacked(
      "\x19Ethereum Signed Message:\n32",
      hash
    ));
  }

I would be happy to provide a PR if that would be helpful.

[frangio]

Thanks for reporting @linusnorton! This has already been fixed in #951 and is released in v1.11.0.

[linusnorton]

Sorry @frangio I did a search but left :open on. I see 1.11.0 has been merged but not released in npm, is that intentional?

[frangio]

You're probably using the zeppelin-solidity package. We renamed the package to openzeppelin-solidity. You should see a deprecation notice suggesting to install the new one! Let me know if you can see it.

I've just published zeppelin-solidity@1.11.0 but the package remains deprecated and will stop being updated soon.

[linusnorton]

That was it, I can see it now. Thanks