Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eth/abi: implement encode_packed/decode_packed #103

Open
tomiwaAdey opened this issue May 20, 2022 · 9 comments · May be fixed by #211
Open

eth/abi: implement encode_packed/decode_packed #103

tomiwaAdey opened this issue May 20, 2022 · 9 comments · May be fixed by #211
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@tomiwaAdey
Copy link

tomiwaAdey commented May 20, 2022

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
Copy link
Author

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

@kurotaky
Copy link
Collaborator

kurotaky commented May 22, 2022

@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

@tomiwaAdey
Copy link
Author

@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
Copy link
Author

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

@kurotaky
Copy link
Collaborator

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

@tomiwaAdey
Copy link
Author

tomiwaAdey commented May 29, 2022

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.

@q9f
Copy link
Owner

q9f commented May 30, 2022

Right, we might need encode_packed then.

@q9f q9f changed the title Smart Contract unable to verify signed message eth/abi: implement encode_packed/decode_packed May 30, 2022
@q9f q9f added enhancement New feature or request help wanted Extra attention is needed labels May 30, 2022
@q9f
Copy link
Owner

q9f commented Jan 17, 2023

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
Copy link
Owner

q9f commented Jan 18, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants