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

implement meta txn arg parsing, eip-712 msg encoding for meta txn and add a ecrecover test #3534

Closed
wants to merge 0 commits into from

Conversation

ailisp
Copy link
Member

@ailisp ailisp commented Oct 28, 2020

The test input is console logged from following js snippet:

const { recoverTypedSignature_v4 } = require("eth-sig-util");

let data = `{"types":{"EIP712Domain":[{"name":"name","type":"string"},{"name":"version","type":"string"},{"name":"chainId","type":"uint256"}],"Arguments":[{"name":"petId","type":"uint256"}],"NearTx":[{"name":"evmId","type":"string"},{"name":"nonce","type":"uint256"},{"name":"feeAmount","type":"uint256"},{"name":"feeAddress","type":"address"},{"name":"contractAddress","type":"address"},{"name":"contractMethod","type":"string"},{"name":"arguments","type":"Arguments"}]},"primaryType":"NearTx","domain":{"name":"NEAR","version":"1","chainId":1313161555},"message":{"evmId":"evm","nonce":0,"feeAmount":"6","feeAddress":"0x0000000000000000000000000000000000000000","contractAddress":"0xE5D4Dbb86d8bb8a001A613537C2e879902F8c2F4","contractMethod":"adopt(uint256)","arguments":{"petId":0}}}`;
let signature =
  "c710c068462547d3d3c452a4abc14fd91f152357c21e667ad6ac67130e76e9a1501491aa4e9d35846bff49d9c77e913217031fdc44f1dc36271a4b7d637763d01b";
const recoveredAddress = recoverTypedSignature_v4({
  data: JSON.parse(data), // this is just temporary, obvs
  sig: `0x${signature}`,
});

// patch eth-sig-utils in node_modules:
function recoverTypedSignature_v4(msgParams) {
    console.log('sig', msgParams.sig)
    var message = TypedDataUtils.sign(msgParams.data);
    console.log('msg', ethUtil.bufferToHex(message))
    var publicKey = recoverPublicKey(message, msgParams.sig);
    var sender = ethUtil.publicToAddress(publicKey);
    
    return ethUtil.bufferToHex(sender);
}

console.log("recoveredAddress", recoveredAddress);

Test Plan

Without the fix, the test test_ecrecover failed (doesn't give same result as metamask's ecrecover). After the fix, this test passed. Also after fix test_meta_call_sig_recover pass and gives same result in rust and in eth-sig-util. To see that, println from rust, get msg 0xfb453835cc8bc460affc6070c1859b77ed26957e5152bd2a1d9a2b83d460f149, then test this msg and sig in eth-sig-util, by setting

signature = '0x1cb6f28f29524cf3ae5ce49f364b5ad798af5dd8ec3563744dc62792735ce5e222285df1e91c416e430d0a38ea3b51d6677e337e1b0684d7618f5a00a26a2ee21c'
message = ethUtil.toBuffer('0xfb453835cc8bc460affc6070c1859b77ed26957e5152bd2a1d9a2b83d460f149') 

in function recoverTypedSignature_v4(msgParams) {
and run above snippet

Update:
The ecrecover in rust is actually good, just not in same format of eth-sig-util. With move sig[0] + 27 in , sig[64]<>sig[0] it's actually same. So the move +27 to encode_meta_call_function_args is just for test directly with eth-sig-util output, not mean ecrecover is wrong. The thing wrong is eip-712 sig is constructed manually (the construction was wrong, 2bb96b2 gives a correct manual contruct to make test passes & behaves exact as eth-sig-util), TODO: need to write a parser to parse eip-712, encode general meta call type parameters in this PR

@evgenykuzyakov evgenykuzyakov added the A-EVM Area: Native EVM implementation and support label Nov 5, 2020
@ailisp ailisp changed the title Fix: ecrecover implement meta txn arg parsing, eip-712 msg encoding for meta txn and add a ecrecover test Nov 6, 2020
@ailisp
Copy link
Member Author

ailisp commented Nov 8, 2020

@ilblackdragon prepare_meta_call_args fully works now, testing these cases in
fn test_meta_call_sig_and_recover() { and produce same sig (therefore also msg before sign) as this eth-sig-util script:
https://gist.github.com/ailisp/0f65bc1a3827b03141ae6601918a67a2

I noticed prepare_meta_call_args took encoded "function call args" and encode it to meta_txn msg. There lacks a encode_args function that:

encode_args("adopt(uint256 petId,PetObj petObject)PetObj(string petName,address owner)", PetObj{petName: "CapsLock", address: "0x0123456789..."})
=>
vec![
            u256_to_arr(&U256::from(9)).to_vec(),
            keccak(
                &vec![
                    encode_string("PetObj(string petName,address owner)"),
                    encode_string("CapsLock"),
                    encode_address(Address::from_slice(
                        &hex::decode("0123456789012345678901234567890123456789").unwrap(),
                    )),
                ]
                .concat(),
            )
            .as_bytes()
            .to_vec(),
        ]
        .concat(),

However, what type of the second argument we give to rust function encode_args (PetObj{petName: ...} above)? Seems i understand why it's given as a json string in eip-712 crate, didn't think of a good solution, maybe

HashMap<String, Encodable>
enum Encodable {
     Address
     String
     U256
     HashMap<String, Encodable>
}

?
Or we don't need to worry about the arg encoding here, it should be done in client side and in near-evm-runner we only need encode_meta_call_function_args?

@ailisp
Copy link
Member Author

ailisp commented Nov 9, 2020

@ilblackdragon For encoding args looks we did with use_contract generated encoder, such as:

soltest::functions::precompile_test::call();

However, this encoded form is not convenient to decode back to a format similar to:

PetObj(string petName,address owner)\0CapsLock\00x0123456789012345678901234567890123456789

so that:

  • Planned encode_args function can take this format as "args" and "adopt(uint256 petId,PetObj petObject)PetObj(string petName,address owner)" as "method_name", return eip-712 of this part as "args" for prepare_meta_call_args
  • encode_meta_call_function_args will take use_contract generated encode args as evm function arg.

So an easier way is both encode_meta_call_function_args and encode_args takes args similar to:

PetObj(string petName,address owner)\0CapsLock\00x0123456789012345678901234567890123456789

In metamask, this format is json. In our node, This probably should be borsh, replace Encodable in above comment.

@artob artob self-requested a review November 10, 2020 17:47
runtime/near-evm-runner/src/utils.rs Outdated Show resolved Hide resolved
runtime/near-evm-runner/src/utils.rs Outdated Show resolved Hide resolved
runtime/near-evm-runner/src/utils.rs Outdated Show resolved Hide resolved
@ailisp
Copy link
Member Author

ailisp commented Nov 19, 2020

@evgenykuzyakov ptal, do we merge this to evm-precompile or wait evm-precompile merge first?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-EVM Area: Native EVM implementation and support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants