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

Permit validator voting across an airgap #3985

Merged
merged 8 commits into from
Mar 12, 2024
Merged

Conversation

plaidfinch
Copy link
Collaborator

Describe your changes

This addresses the first changeset described in #3813, permitting validator definitions and votes to be signed separately and those signatures to be broadcast from a potentially-unrelated penumbra account.

This is a breaking change to the CLI for all workflows that use the validator vote subcommand, because it redefines that command to validator vote cast in order to make room for validator vote sign. It is purely a client-side change, and as such is not consensus-breaking.

Issue ticket number and link

#3813

Checklist before requesting a review

  • If this code contains consensus-breaking changes, I have added the "consensus-breaking" label.

Copy link
Member

@erwanor erwanor left a comment

Choose a reason for hiding this comment

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

LGTM!

@cratelyn cratelyn added C-enhancement Category: an enhancement to the codebase A-governance Area: Governance labels Mar 11, 2024
@conorsch
Copy link
Contributor

Nit: the docs at https://guide.penumbra.zone/main/pcli/governance.html#voting-as-a-validator should be updated to reflect this change.

Also, when we do land this, please squash-merge so that it's cherry-pickable. Important as we move toward stable testnets!

@plaidfinch plaidfinch merged commit ec9837a into main Mar 12, 2024
6 checks passed
@plaidfinch plaidfinch deleted the airgap-validator-custody branch March 12, 2024 21:35
plaidfinch added a commit that referenced this pull request Mar 12, 2024
author plaidfinch <finch@plaidfinch.net> 1710279332 -0400
committer finch <finch@plaidfinch.net> 1710280707 -0400

Permit validator voting across an airgap (#3985)

This addresses the first changeset described in #3813, permitting
validator definitions and votes to be signed separately and those
signatures to be broadcast from a potentially-unrelated penumbra
account.

This is a breaking change to the CLI for all workflows that use the
`validator vote` subcommand, because it redefines that command to
`validator vote cast` in order to make room for `validator vote sign`.
It is purely a client-side change, and as such is not
consensus-breaking.

- [X] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label.

Delete unused import

Make formatting a bit nicer, set validator vote sign to offline

Allow initializing a separate governance subkey

Add a validator command to emit the governance key

Use the governance subkey correctly when present, in SoftKMS mode

Use the governance key, if configured, in the validator template

Fix mistake where the governance key was overzealously used

It should only be used to sign *votes*, not *definitions*!

Fix bug loading wrong config file path in dkg deal for governance keys
plaidfinch added a commit that referenced this pull request Mar 12, 2024
author plaidfinch <finch@plaidfinch.net> 1710279332 -0400
committer finch <finch@plaidfinch.net> 1710280707 -0400

Permit validator voting across an airgap (#3985)

This addresses the first changeset described in #3813, permitting
validator definitions and votes to be signed separately and those
signatures to be broadcast from a potentially-unrelated penumbra
account.

This is a breaking change to the CLI for all workflows that use the
`validator vote` subcommand, because it redefines that command to
`validator vote cast` in order to make room for `validator vote sign`.
It is purely a client-side change, and as such is not
consensus-breaking.

- [X] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label.

Delete unused import

Make formatting a bit nicer, set validator vote sign to offline

Allow initializing a separate governance subkey

Add a validator command to emit the governance key

Use the governance subkey correctly when present, in SoftKMS mode

Use the governance key, if configured, in the validator template

Fix mistake where the governance key was overzealously used

It should only be used to sign *votes*, not *definitions*!

Fix bug loading wrong config file path in dkg deal for governance keys
plaidfinch added a commit that referenced this pull request Mar 13, 2024
author plaidfinch <finch@plaidfinch.net> 1710279332 -0400
committer finch <finch@plaidfinch.net> 1710280707 -0400

Permit validator voting across an airgap (#3985)

This addresses the first changeset described in #3813, permitting
validator definitions and votes to be signed separately and those
signatures to be broadcast from a potentially-unrelated penumbra
account.

This is a breaking change to the CLI for all workflows that use the
`validator vote` subcommand, because it redefines that command to
`validator vote cast` in order to make room for `validator vote sign`.
It is purely a client-side change, and as such is not
consensus-breaking.

- [X] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label.

Delete unused import

Make formatting a bit nicer, set validator vote sign to offline

Allow initializing a separate governance subkey

Add a validator command to emit the governance key

Use the governance subkey correctly when present, in SoftKMS mode

Use the governance key, if configured, in the validator template

Fix mistake where the governance key was overzealously used

It should only be used to sign *votes*, not *definitions*!

Fix bug loading wrong config file path in dkg deal for governance keys
plaidfinch added a commit that referenced this pull request Mar 15, 2024
## Describe your changes

These changes are meant to be stacked on top of #3985, which implements
the first set of three change sets defined in #3813.

The changes in this PR separate out a distinct governance subkey, which
can be initialized after main initialization in `pcli` by using `pcli
init validator-governance-subkey {soft-kms, threshold}`. This command
will edit in place the existing configuration file to add a section
describing the custody of a separate governance key, which may be
custodied differently than the main key. These changes support soft-kms
and threshold custody for governance key.

The generated distinct governance key will then, after initialization,
be used instead of the key implied by the spend key, when signing and
casting validator votes.

Subsequently, when generating a validator definition template, this key
is the one inserted into the template, and it can be fetched directly
using the `pcli validator governance-key` command.

Being merely client-side, this set of changes does not break consensus.

## Issue ticket number and link

#3813

## Checklist before requesting a review

- [X] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-governance Area: Governance C-enhancement Category: an enhancement to the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants