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

Validate keypair app and command #7617

Merged
merged 3 commits into from
Jan 22, 2021
Merged

Conversation

psteckler
Copy link
Member

@psteckler psteckler commented Jan 22, 2021

New validate_keypair app, and command advanced validate-keypair.

As with generate-keypair, the app and command use CODA_PRIVKEY_PASS, if set, and prompt for a password otherwise, and both take the privkey-path flag.

Validation consists of signing a dummy transaction and verifying the signature.

TODO: add a test of the keys using the VRF machinery (as commented in the code).

Update: won't add VRF bits, see comment below.

Example session:

$ coda advanced validate-keypair -privkey-path ~/keys/flimflam
Enter password: 
Again to confirm: 
Verified a transaction using specified keypair

@psteckler psteckler requested review from a team as code owners January 22, 2021 16:54
@psteckler psteckler force-pushed the feature/validate-keypairs branch from 38de5f8 to 5d618db Compare January 22, 2021 17:06
@psteckler psteckler changed the base branch from develop to compatible January 22, 2021 17:06
@psteckler psteckler requested a review from a team as a code owner January 22, 2021 18:25
Copy link
Contributor

@lk86 lk86 left a comment

Choose a reason for hiding this comment

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

Looks good from product + infra view, but still waiting on the VRF changes before we ship it

@bkase bkase added the ci-build-me Add this label to trigger a circle+buildkite build for this branch label Jan 22, 2021
match%map Secrets.Keypair.read ~privkey_path ~password with
| Ok keypair ->
validate_transaction keypair
(* TODO: add validation using VRF eval_and_test_public_key *)
Copy link
Member

Choose a reason for hiding this comment

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

The VRF is effectively a hash function built out of a signature. The only test you can do is to re-evaluate it and check that they're equal, which doesn't seem to give us anything of value. eval_and_check_public_key is for snarks, so the additional checks are not relevant for normal use.

Copy link
Member

Choose a reason for hiding this comment

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

In other words, you're saying there is no possible universe that a key could properly sign transactions but not evaluate VRFs? I guess if this is the case, then we don't need that extra check.

@psteckler psteckler added the ready-to-merge Adding this label will trigger mergify and trigger CI to run and merge the PR label Jan 22, 2021
@mrmr1993 mrmr1993 merged commit 75a8a39 into compatible Jan 22, 2021
@mrmr1993 mrmr1993 deleted the feature/validate-keypairs branch January 22, 2021 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-build-me Add this label to trigger a circle+buildkite build for this branch ready-to-merge Adding this label will trigger mergify and trigger CI to run and merge the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants