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

Sign & verify message with name #609

Merged
merged 8 commits into from
Jun 9, 2021

Conversation

Falci
Copy link
Member

@Falci Falci commented Jun 6, 2021

#606 item 2: Create new RPCs signmessagewithname and verifymessagewithname

@coveralls
Copy link

coveralls commented Jun 6, 2021

Pull Request Test Coverage Report for Build 922544001

  • 30 of 35 (85.71%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.07%) to 60.142%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/wallet/rpc.js 15 17 88.24%
lib/node/rpc.js 15 18 83.33%
Files with Coverage Reduction New Missed Lines %
lib/protocol/consensus.js 1 82.79%
Totals Coverage Status
Change from base Build 863907438: 0.07%
Covered Lines: 19893
Relevant Lines: 30831

💛 - Coveralls

Copy link
Member

@pinheadmz pinheadmz left a comment

Choose a reason for hiding this comment

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

Great job on this dude! Very cool and fun to play with on command line as well:

--> hsw-rpc signmessagewithname test-txt "Falci is handsome."
CUjgakQfpOG9aTa4DzKaX9heuYZW2D26tZ9pb6VEgco5pLNuU8NljSQn2DtAFGkpEiEugNYNh94W7JZAjKuWkQ==
--> hsd-rpc verifymessagewithname test-txt CUjgakQfpOG9aTa4DzKaX9heuYZW2D26tZ9pb6VEgco5pLNuU8NljSQn2DtAFGkpEiEugNYNh94W7JZAjKuWkQ== "Falci is handsome."
true
--> hsd-rpc verifymessagewithname test-txt CUjgakQfpOG9aTa4DzKaX9heuYZW2D26tZ9pb6VEgco5pLNuU8NljSQn2DtAFGkpEiEugNYNh94W7JZAjKuWkQ== "Falci is not handsome."

false

lib/node/rpc.js Outdated Show resolved Hide resolved
lib/wallet/rpc.js Outdated Show resolved Hide resolved
test/wallet-rpc-test.js Show resolved Hide resolved
test/wallet-rpc-test.js Outdated Show resolved Hide resolved
@pinheadmz
Copy link
Member

Oh one more thing: please add these new RPC methods to the CHANGELOG!

@pinheadmz
Copy link
Member

As discussed, both sign and verify should check if name is null:

if (!name || !rules.verifyName(name))
      throw new RPCError(errs.TYPE_ERROR, 'Invalid name.');

test/wallet-rpc-test.js Outdated Show resolved Hide resolved
test/wallet-rpc-test.js Outdated Show resolved Hide resolved
lib/node/rpc.js Outdated
}

const address = coin.address.toString(this.network);
return this.verifyMessage([address, sig, str], help);
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you need to pass help here. If help were true, we'd throw an error right away, and we don't want to return help from a different RPC call anyway. Applies to both functions.

Comment on lines 1604 to 1605
if (!wallet.master.key)
throw new RPCError(errs.WALLET_UNLOCK_NEEDED, 'Wallet is locked.');
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need this here, it will be checked in signMessage() and we don't need to decrypt the wallet until then anyway.

Copy link
Member

@pinheadmz pinheadmz left a comment

Choose a reason for hiding this comment

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

ACK f1e096f

Show Signature
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK f1e096ff1faa7b9ee56415f37e5fe55286ec3a55
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmDBDOAACgkQ5+KYS2KJ
yTox3xAAqMLyvBNxI0MCIy8W2JwLFAOM5FsbC6kf7GQ9zO7rgYPjUWtIBIg8ECW0
vYd/kPlXU0I86hDYEGnUtwW3P6uX+SERhR6AVPW+mbxPJlT2MZ7LfzAMXAXLxfTV
umm6fzWsHXUj9H59FZX3eFkGpZ6fcofZck4/5NMsaiHBffZLbeP/uQN8sFC1NTEw
c1orML4rrstolIX9km9AZoOkecZXl0eRmEg9HMPlSpNnZHFokW8TFgpDvlqPQC0g
2zfPvTHtISSyULwEsAuwq4464ad9ZZLpppFx3lED22SW1d5IxqwjHH4iSqC9L4zp
f45CoDsbbOoocbCAyMwuPRg7QaiDeLYxdR1ijIc3AlKmHHSUPzOM1xU6e3vt/pxt
5/ZtwAKHwzcfSh/l9aUXvDXuEt0L1v+ra5lfUzr6MKtICdCrNsoJxHIXq4uBI1JP
Td03E5xH5b80xEZt3YwupliN/2ShSRYc+8RieZSLLB+9GhpWwMSM/pE2IUT1vWnX
tyhvdL//ledI/eRHwdoeZ9aYQsiRJz/5H9xXoP1q3RfQ+rYWUWf2yXtqDRJ6Gc1I
dPcnpdkK6hfDTQn/41FRnQFWQ9ueZFHMXZWtRPP07sfSd5alrYCqWeP6Q7IG588E
N+nYe/ZWvn6xWksJ2iW5stUNyK9hdVoF8SLlhqVHXATcmAXo3DQ=
=dTW7
-----END PGP SIGNATURE-----

pinheadmz's public key is on keybase

@pinheadmz pinheadmz merged commit 7c00529 into handshake-org:master Jun 9, 2021
Copy link

@0racl3z 0racl3z left a comment

Choose a reason for hiding this comment

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

thx!

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

4 participants