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 signmessagewithprivkey JSON-RPC command (rpc server) #1585

Merged
merged 1 commit into from
Aug 31, 2020

Conversation

lindlof
Copy link
Contributor

@lindlof lindlof commented May 24, 2020

Reuse the Bitcoin message signature header const
also in verifymessage.

@lindlof lindlof changed the title Implement signmessagewithprivkey JSON-RPC command (rpc client & server) Implement signmessagewithprivkey JSON-RPC command (rpc server) May 25, 2020
@lindlof
Copy link
Contributor Author

lindlof commented May 26, 2020

Because of this Satoshi was finally able to sign his message the next day 😂

func handleSignMessageWithPrivKey(s *rpcServer, cmd interface{}, closeChan <-chan struct{}) (interface{}, error) {
c := cmd.(*btcjson.SignMessageWithPrivKeyCmd)

wif, err := btcutil.DecodeWIF(c.PrivKey)
Copy link
Collaborator

Choose a reason for hiding this comment

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

bitcoind rejects WIFs that do not belong to the current network. Or so it seems. Here's an example for you to reproduce using a mainnet WIF:

$ ./btcctl --regtest --rpcuser=aaaa --rpcpass=bbbb signmessagewithprivkey 5HueCGU8rMjxEXxiPuD5BDku4MkFqeZyd4dZ1jvhTVqvbTLvyTJ iamsatoshi
G+brEqKGQgPCSkPJrBau7x32eFhT3Y5YXdEXKG2bg2E2Tqi0lU54t+rYrEezLcSDLS4TM+/AO8a3A9ybAfvyMLY=

$ bitcoin-cli -regtest signmessagewithprivkey 5HueCGU8rMjxEXxiPuD5BDku4MkFqeZyd4dZ1jvhTVqvbTLvyTJ iamsatoshi
error code: -5
error message:
Invalid private key

The fix would be to simply check wif.IsForNet(s.cfg.ChainParams).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this was the case. Fixed by checking wif.IsForNet(s.cfg.ChainParams) as you suggested.

Reuse the Bitcoin message signature header const
also in verifymessage.
Copy link
Collaborator

@onyb onyb left a comment

Choose a reason for hiding this comment

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

Tested OK locally. ✔️

@jakesylvestre
Copy link
Collaborator

Looks good to me/tested locally against 0.20.0/0.19.0 (on bitcoind side). Since I'm not as familiar with btcec, would prefer to have someone else look over it as well

Copy link
Member

@jcvernaleo jcvernaleo left a comment

Choose a reason for hiding this comment

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

OK

@jcvernaleo jcvernaleo merged commit d2c0123 into btcsuite:master Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants