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

rpc: Correct JSON-RPC and gRPC verifymessage. #2150

Merged
merged 1 commit into from May 5, 2022

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented May 5, 2022

This corrects the main logic that verifies messages to create a p2pkh address from the recovered public key instead of a p2pk since p2pkh is what is produced when signing and therefore what is required when verifying (as can be verified in dcrd as well).

This also updates the call sites that decode the addresses to properly reject p2pk addresses for the same reason.

@davecgh
Copy link
Member Author

davecgh commented May 5, 2022

Here is a test case on testnet:

Before:

$ dcrctl --testnet --wallet verifymessage TsYYKeoH94fkyZr4Jkvk9WcQkW8LRjxx9yL "Hx5i6DXPWeWbSb37PUq57MWNLtx4Js4dztOjepIebZSiaC3Twfx5VDgx1OIGWYVBfIe+iDT056cS4ZWHbl28Z4o=" test
false

After:

$ dcrctl --testnet --wallet verifymessage TsYYKeoH94fkyZr4Jkvk9WcQkW8LRjxx9yL "Hx5i6DXPWeWbSb37PUq57MWNLtx4Js4dztOjepIebZSiaC3Twfx5VDgx1OIGWYVBfIe+iDT056cS4ZWHbl28Z4o=" test
true

wallet/wallet.go Outdated Show resolved Hide resolved
Copy link
Member

@alexlyp alexlyp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, just a comment spello to fix

This corrects the main logic that verifies messages to create a p2pkh
address from the recovered public key instead of a p2pk since p2pkh is
what is produced when signing and therefore what is required when
verifying (as can be verified in dcrd as well).

This also updates the call sites that decode the addresses to properly
reject p2pk addresses for the same reason.
@davecgh davecgh force-pushed the rpc_correct_verify_message branch from f8e1e8a to a547182 Compare May 5, 2022 15:07
@jrick jrick merged commit a547182 into decred:master May 5, 2022
@davecgh davecgh deleted the rpc_correct_verify_message branch May 5, 2022 15:15
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

Successfully merging this pull request may close these issues.

None yet

3 participants