eth.rb icon indicating copy to clipboard operation
eth.rb copied to clipboard

eth/abi: implement encode_packed/decode_packed

Open tomiwaAdey opened this issue 2 years ago • 9 comments

Given: I sign a message with a private key generated using eth.rb Then: I get a signature Expected: The signer of the message to be the signer verified on the smart contract

Ruby (Eth.rb) 
# Generate new key and wallet address
new_wallet = Eth::Key.new

# Array to be encoded 
token_ids = [1,2,3] 

 # Sign message
get_signature = Eth::Util.method(:prefix_hex) << new_wallet.method(:personal_sign) << Eth::Util
.method(:bin_to_prefixed_hex)  << Eth::Util.method(:keccak256)  << Eth::Abi.method(:encode)
 
get_signature.call(['uint256[]'], [token_ids])
# returns 0x656f70ae6747dc4de541fdaea84701a4b06feea22714cdae6b49ebd4e3c32d1b5b96dca9cf232add9ee0d0e35c951f3e940e93d26fcaf22a51413cd058402fb21c

Smart contract

// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity >=0.8.9;
import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
contract VerifySignature {
    using ECDSA for bytes32;

    function getMessageHash(uint256[] memory  ids) public pure returns (bytes32) {
        return keccak256(abi.encodePacked(ids));
    }


    function verify(bytes memory sig, uint256[] memory ids) external pure returns (bool) {
        bytes32 messagehash = keccak256(
            abi.encodePacked(ids)
        );
        return messagehash.toEthSignedMessageHash().recover(sig) == 0x2ceBaa61f571C38d41cFFc8605FB3C1ac4d7F6e7;
    }

    function getSigner(bytes memory sig, uint256[] memory ids) external pure returns (address) {
        bytes32 messagehash = getMessageHash(ids);
        return messagehash.toEthSignedMessageHash().recover(sig);
    }

}

getSigner returns a different address from the new_wallet.address

@q9f @kurotaky Is there a difference between how encodePacked in solidity works and encode in eth.rb?

Using v'0.5.3'

tomiwaAdey avatar May 20 '22 07:05 tomiwaAdey

I also tried with simple strings and I get the same result

tomiwaAdey avatar May 21 '22 08:05 tomiwaAdey

@tomiwaAdey Hi, I used abi.encode(ids) in Remix and the result of encode is the same.

Result of encode in eth.rb

returns 0x62e243217b24f0adeab63b697d9c38d64bd4cbf540c9915772ddc377b45b411c

[2] pry(main)> token_ids = [1,2,3]
=> [1, 2, 3]
 [23] pry(main)> Eth::Util.bin_to_prefixed_hex(Eth::Util.keccak256(Eth::Abi.encode(['uint256[]'], [token_ids])))
=> "0x62e243217b24f0adeab63b697d9c38d64bd4cbf540c9915772ddc377b45b411c"

Try it on Smart Contract with Remix (use abi.encodePacked() )

returns 0x6e0c627900b24bd432fe7b1f713f1b0744091a646a9fe4a65a18dfed21f2949c

Code

// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity >=0.8.9;
import "./ECDSA.sol";

contract VerifySignature {
    using ECDSA for bytes32;

    function getMessageHash(uint256[] memory ids) public pure returns (bytes32) {
        return keccak256(abi.encodePacked(ids));
    }

    function verify(bytes memory sig, uint256[] memory ids) external pure returns (bool) {
        bytes32 messagehash = keccak256(
            abi.encodePacked(ids)
        );
        return messagehash.toEthSignedMessageHash().recover(sig) == 0x2ceBaa61f571C38d41cFFc8605FB3C1ac4d7F6e7;
    }

    function getSigner(bytes memory sig, uint256[] memory ids) external pure returns (address) {
        bytes32 messagehash = getMessageHash(ids);
        return messagehash.toEthSignedMessageHash().recover(sig);
    }
}

Capture when executed

スクリーンショット 2022-05-22 20 09 59

Try it on Smart Contract with Remix (use abi.encode() )

returns 0x62e243217b24f0adeab63b697d9c38d64bd4cbf540c9915772ddc377b45b411c

Code

// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity >=0.8.9;
import "./ECDSA.sol";

contract VerifySignature {
    using ECDSA for bytes32;

    function getMessageHash(uint256[] memory ids) public pure returns (bytes32) {
        return keccak256(abi.encode(ids));
    }

    function verify(bytes memory sig, uint256[] memory ids) external pure returns (bool) {
        bytes32 messagehash = keccak256(
            abi.encode(ids)
        );
        return messagehash.toEthSignedMessageHash().recover(sig) == 0x2ceBaa61f571C38d41cFFc8605FB3C1ac4d7F6e7;
    }

    function getSigner(bytes memory sig, uint256[] memory ids) external pure returns (address) {
        bytes32 messagehash = getMessageHash(ids);
        return messagehash.toEthSignedMessageHash().recover(sig);
    }
}

Capture when executed

スクリーンショット 2022-05-22 20 16 01

kurotaky avatar May 22 '22 11:05 kurotaky

@kurotaky I appreciate you taking the time to look into this.

I have since dug deeper into it and encoding in solidity works the same as encoding here.

The only problem when it comes to signing the message hash with eth.rb and verifying using Openzeplin's ECDSA is that OZ uses encodePacked and I'm not exactly sure exactly why they are using encodePacked instead of encode.

These are the options I'm considering:

  1. adding a method to generate tightly packed encodings on eth.rb
  2. encoding the ids in the FE (frontend) using ethers' solidityPack before sending it to the BE (backend) and signing (personal_sign) with eth.rb
  3. Using a custom verifier instead of OZ's

I was leaning more towards option 3 but I'm a bit worried as to why OZ uses encodePacked considering their code has been audited multiple times. Now I'll probably go with option 2 and then when I've finished the implementation, I'll work on option 1 and submit a pr.

tomiwaAdey avatar May 22 '22 14:05 tomiwaAdey

Unfortunately, no luck with option 2 either. I'll move most of the logic here to use JS for now.

The implementation in JS using ethers:

const ids = [1,2,3]
const messageHash = ethers.utils.solidityKeccak256(["uint256[]"], [ids]);
const arrayifyMessage = ethers.utils.arrayify(messageHash);
const sig = await new ethers.Wallet(<PrivateKey>).signMessage(arrayifyMessage);

The sig return is verifiable with OZ ECDSA.

I've tried to send the messageHash to ruby.eth so I just sign the hash, but even that produces the wrong signature.

key = Eth::Key.new 
key.personal_sign messageHash

tomiwaAdey avatar May 22 '22 20:05 tomiwaAdey

@tomiwaAdey I am going to look at this issue now, what did you find out?

kurotaky avatar May 29 '22 06:05 kurotaky

Hey @kurotaky :)

I ended up using etherjs for signing just because I have a deadline coming up about a week from today.

EncodePacked Yes, encodePacked performs packed encoding where:

  • Dynamic types are encoded in place without length.
  • Static types will not be padded if they are shorter than 32 bytes
  • with the exception of arrays which doesn't have it's elements packed (not sure why) So this should be doable.

Another path is to verify the signature manually instead of using OZ ECDSA (the lib relies heavily on encodePacked). I implemented it like this:

function _splitSignature(bytes memory signature) private pure returns (
        bytes32 r, 
        bytes32 s, 
        uint8 v
        ) {
        require(signature.length == 65, "INVALID SIGNATURE LENGTH");
        assembly {
            r := mload(add(signature, 32))
            s := mload(add(signature, 64))
            v := byte(0, mload(add(signature, 96)))
        }
    }

    function _recoverSigner(
        bytes32 messageHash, 
        bytes memory signature) private pure returns (address) {
        (bytes32 r, bytes32 s, uint8 v) = _splitSignature(signature);
        return ecrecover(
           keccak256(
            abi.encode(
                "\x19Ethereum Signed Message:\n32", 
                messageHash
            )),
            v, 
            r, 
            s
        );
    }
    
    
Then I can verify
require(_recoverSigner(keccak256(abi.encode(id, address(this))), signature) == <signerAddress>, "WRONG SIGNATURE");

In terms of generating the signature I didn't dive as much as ethersjs was working out of the box. It might have something to do with the way I was constructing the message to be signed but I'm not sure and haven't looked much further. I'll look into it after I'm done with this project as I'll rather write these things in ruby than JS/ Typescript.

But this is a common pattern generally with writing dapps, sign a message offchain and verify onchain. We should make it as seamless as possible for developers to keep to the ruby ethos.

tomiwaAdey avatar May 29 '22 10:05 tomiwaAdey

Right, we might need encode_packed then.

q9f avatar May 30 '22 10:05 q9f

EncodePacked Yes, encodePacked performs packed encoding where:

  • Dynamic types are encoded in place without length.
  • Static types will not be padded if they are shorter than 32 bytes
  • with the exception of arrays which doesn't have their elements packed (not sure why) So this should be doable.

Do you mind pointing me toward the specification of this?

Edit: Apparently, it's in the solidity docs: https://docs.soliditylang.org/en/v0.8.17/abi-spec.html?highlight=encodepacked#non-standard-packed-mode

q9f avatar Jan 17 '23 15:01 q9f

Screenshot at 2023-01-18 16-33-53

q9f avatar Jan 18 '23 15:01 q9f