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

unusable message signing (ethereum/accounts/:name/sign and ethereum/accounts/:name/verify) #74

Open
mortimr opened this issue Mar 2, 2020 · 9 comments

Comments

@mortimr
Copy link
Contributor

mortimr commented Mar 2, 2020

Detailed Description

I'm currently trying to sign an EIP712 payload with vault-ethereum. I'm using a lib that is properly signing and verifying payloads that can also be verified by the EVM (https://github.com/ticket721/e712).

Right now I am unable to properly verify the signatures generated by vault-ethereum. When using e712 or eth-sig-util, I end up with a failed verification.

Also the verify route does not seem to work on vault-ethereum.

Why is there no encoding argument like on the sign-tx route ?

Context

Signing messages (and more precisely encoded EIP712 payloads) is primordial as this EIP is becoming the standard for signing. I still cannot identify where the issue is coming from (maybe the format of the data is not good, adding an 'hex' encoding option might be the solution as I'm currently send some binary unreadable strings hoping it gets properly evaluated on vault-ethereum's side).

Possible Implementation

Maybe it can work by reusing what you done in the sign-tx encoding section. Maybe it's coming from something else (differing keccak256 implems, differing signing algorithms). But I'm pretty confident that the tools cited above are properly working as the signatures can get properly verified on-chain.

Your Environment

@cypherhat
Copy link
Member

I will try to take a look at this later this evening.

@mortimr
Copy link
Contributor Author

mortimr commented Mar 5, 2020

Hi @cypherhat, any news on the issue ? Let me know if I can help you with anything

@cypherhat
Copy link
Member

I apologize - I have 2 full time commitments... I will definitely try this weekend to evaluate your issue. If you have an implementation suggestion, please suggest. Or submit a PR.

@cypherhat
Copy link
Member

I think I should provide a native EIP712 signer method. As I mentioned, I have a heavy workload - I am gonna add this as a feature request. I will make every effort to address this ASAP. Contributions are welcome.

@mortimr
Copy link
Contributor Author

mortimr commented Mar 9, 2020

Hi @cypherhat, I currently have a big task to finish. I’ll look into the issue aswell in ~2-3 days. I think the issue can be solved quite easily by debugging the current “sign” and “verify” method and make sur of their interoperability with other tools. First thing I’ll try is to accept hex encoded arguments for the “sign” method in order to have an encoding format to properly relay binary as hex. I’ll try to debug and compare the results of the keccak256 and ecdsa with other tools and will come back to you then :)

@mortimr
Copy link
Contributor Author

mortimr commented Mar 12, 2020

Hi @cypherhat, the issue came from the keccak256 implementation I was using. The sign route seems to work. But the verify route on the accounts is not working at all. I'll let you look into it.

@mortimr
Copy link
Contributor Author

mortimr commented Mar 12, 2020

Sorry, my bad, the issue came from the hash you are doing without giving the ability to encode. I fixed it locally and will raise a PR with an extra argument encoding on the sign route.

@cypherhat
Copy link
Member

Thanks man!

Remember to wash your hands before you submit the PR.

@MonarthS
Copy link

@mortimr & @cypherhat any updates on this, as I'm facing same issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants