From de135e807d4e883fb4fb5e0757748e38cd2d8719 Mon Sep 17 00:00:00 2001 From: katelyn martin Date: Wed, 3 Apr 2024 20:25:58 -0400 Subject: [PATCH] stake: `VerificationKeyBytes` in `IdentityKey` (#4152) ## issue ticket number and link fixes #2304. ## checklist before requesting a review - [x] If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: > only changes internal representation --------- Co-authored-by: Henry de Valence --- crates/bin/pcli/src/command/validator.rs | 12 ++++++------ crates/bin/pcli/tests/proof.rs | 2 +- crates/bin/pd/src/testnet/generate.rs | 4 ++-- .../app_can_define_and_delegate_to_a_validator.rs | 4 ++-- ..._tracks_uptime_for_validators_only_once_active.rs | 4 ++-- .../core/app/tests/common/test_node_builder_ext.rs | 2 +- .../component/action_handler/validator_definition.rs | 7 +++---- .../component/stake/src/component/stake/tests.rs | 5 +++-- crates/core/component/stake/src/delegation_token.rs | 5 +++-- crates/core/component/stake/src/identity_key.rs | 8 ++++---- crates/core/component/stake/src/rate.rs | 4 ++-- crates/core/component/stake/src/state_key.rs | 8 ++++---- crates/core/component/stake/src/unbonding_token.rs | 5 +++-- .../component/stake/src/undelegate_claim/proof.rs | 3 ++- 14 files changed, 38 insertions(+), 35 deletions(-) diff --git a/crates/bin/pcli/src/command/validator.rs b/crates/bin/pcli/src/command/validator.rs index d1c0264457..6f9e28108c 100644 --- a/crates/bin/pcli/src/command/validator.rs +++ b/crates/bin/pcli/src/command/validator.rs @@ -158,11 +158,11 @@ impl ValidatorCmd { match self { ValidatorCmd::Identity { base64 } => { - let ik = IdentityKey(fvk.spend_verification_key().clone()); + let ik = IdentityKey(fvk.spend_verification_key().clone().into()); if *base64 { use base64::{display::Base64Display, engine::general_purpose::STANDARD}; - println!("{}", Base64Display::new(&ik.0.to_bytes(), &STANDARD)); + println!("{}", Base64Display::new(ik.0.as_ref(), &STANDARD)); } else { println!("{ik}"); } @@ -276,7 +276,7 @@ impl ValidatorCmd { reason, signature_file, }) => { - let identity_key = IdentityKey(fvk.spend_verification_key().clone()); + let identity_key = IdentityKey(fvk.spend_verification_key().clone().into()); let governance_key = app.config.governance_key(); let (proposal, vote): (u64, Vote) = (*vote).into(); @@ -323,7 +323,7 @@ impl ValidatorCmd { reason, signature, }) => { - let identity_key = IdentityKey(fvk.spend_verification_key().clone()); + let identity_key = IdentityKey(fvk.spend_verification_key().clone().into()); let governance_key = app.config.governance_key(); let (proposal, vote): (u64, Vote) = (*vote).into(); @@ -385,7 +385,7 @@ impl ValidatorCmd { tendermint_validator_keyfile, }) => { let (address, _dtk) = fvk.incoming().payment_address(0u32.into()); - let identity_key = IdentityKey(fvk.spend_verification_key().clone()); + let identity_key = IdentityKey(fvk.spend_verification_key().clone().into()); // By default, the template sets the governance key to the same verification key as // the identity key, but a validator can change this if they want to use different // key material. @@ -473,7 +473,7 @@ impl ValidatorCmd { } } ValidatorCmd::Definition(DefinitionCmd::Fetch { file }) => { - let identity_key = IdentityKey(fvk.spend_verification_key().clone()); + let identity_key = IdentityKey(fvk.spend_verification_key().clone().into()); super::query::ValidatorCmd::Definition { file: file.clone(), identity_key: identity_key.to_string(), diff --git a/crates/bin/pcli/tests/proof.rs b/crates/bin/pcli/tests/proof.rs index a3eacf3860..adaa09277e 100644 --- a/crates/bin/pcli/tests/proof.rs +++ b/crates/bin/pcli/tests/proof.rs @@ -425,7 +425,7 @@ fn undelegate_claim_parameters_vs_current_undelegate_claim_circuit() { let balance_blinding = Fr::from(1u8); let value1_amount = 1u64; let penalty_amount = 1u64; - let validator_identity = IdentityKey((&sk).into()); + let validator_identity = IdentityKey(VerificationKey::from(&sk).into()); let unbonding_amount = Amount::from(value1_amount); let start_height = 1; diff --git a/crates/bin/pd/src/testnet/generate.rs b/crates/bin/pd/src/testnet/generate.rs index f063c02afd..d8585200f1 100644 --- a/crates/bin/pd/src/testnet/generate.rs +++ b/crates/bin/pd/src/testnet/generate.rs @@ -457,7 +457,7 @@ impl TestnetValidator { let ivk = fvk.incoming(); let (dest, _dtk_d) = ivk.payment_address(0u32.into()); - let identity_key: IdentityKey = IdentityKey(fvk.spend_verification_key().clone()); + let identity_key: IdentityKey = IdentityKey(fvk.spend_verification_key().clone().into()); let delegation_denom = DelegationToken::from(&identity_key).denom(); Ok(Allocation { address: dest, @@ -540,7 +540,7 @@ impl TryFrom<&TestnetValidator> for Validator { // Currently there's no way to set validator keys beyond // manually editing the genesis.json. Otherwise they // will be randomly generated keys. - identity_key: IdentityKey(tv.keys.validator_id_vk), + identity_key: IdentityKey(tv.keys.validator_id_vk.into()), governance_key: GovernanceKey(tv.keys.validator_id_vk), consensus_key: tv.keys.validator_cons_pk, name: tv.name.clone(), diff --git a/crates/core/app/tests/app_can_define_and_delegate_to_a_validator.rs b/crates/core/app/tests/app_can_define_and_delegate_to_a_validator.rs index 25f6cdb4a9..4d59dfab20 100644 --- a/crates/core/app/tests/app_can_define_and_delegate_to_a_validator.rs +++ b/crates/core/app/tests/app_can_define_and_delegate_to_a_validator.rs @@ -4,7 +4,7 @@ use { self::common::BuilderExt, anyhow::{anyhow, Context}, cnidarium::TempStorage, - decaf377_rdsa::{SigningKey, SpendAuth}, + decaf377_rdsa::{SigningKey, SpendAuth, VerificationKey}, penumbra_app::{genesis::AppState, server::consensus::Consensus}, penumbra_keys::test_keys, penumbra_mock_client::MockClient, @@ -115,7 +115,7 @@ async fn app_can_define_and_delegate_to_a_validator() -> anyhow::Result<()> { // To define a validator, we need to define two keypairs: an identity key // for the Penumbra application and a consensus key for cometbft. let new_validator_id_sk = SigningKey::::new(OsRng); - let new_validator_id = IdentityKey(new_validator_id_sk.into()); + let new_validator_id = IdentityKey(VerificationKey::from(&new_validator_id_sk).into()); let new_validator_consensus_sk = ed25519_consensus::SigningKey::new(OsRng); let new_validator_consensus = new_validator_consensus_sk.verification_key(); diff --git a/crates/core/app/tests/app_tracks_uptime_for_validators_only_once_active.rs b/crates/core/app/tests/app_tracks_uptime_for_validators_only_once_active.rs index 70b2bdc041..de687e553c 100644 --- a/crates/core/app/tests/app_tracks_uptime_for_validators_only_once_active.rs +++ b/crates/core/app/tests/app_tracks_uptime_for_validators_only_once_active.rs @@ -3,7 +3,7 @@ mod common; use { self::common::BuilderExt, cnidarium::TempStorage, - decaf377_rdsa::{SigningKey, SpendAuth}, + decaf377_rdsa::{SigningKey, SpendAuth, VerificationKey}, penumbra_app::{genesis::AppState, server::consensus::Consensus}, penumbra_keys::test_keys, penumbra_mock_client::MockClient, @@ -90,7 +90,7 @@ async fn app_tracks_uptime_for_validators_only_once_active() -> anyhow::Result<( // To define a validator, we need to define two keypairs: an identity key // for the Penumbra application and a consensus key for cometbft. let new_validator_id_sk = SigningKey::::new(OsRng); - let new_validator_id = IdentityKey(new_validator_id_sk.into()); + let new_validator_id = IdentityKey(VerificationKey::from(&new_validator_id_sk).into()); let new_validator_consensus_sk = ed25519_consensus::SigningKey::new(OsRng); let new_validator_consensus = new_validator_consensus_sk.verification_key(); diff --git a/crates/core/app/tests/common/test_node_builder_ext.rs b/crates/core/app/tests/common/test_node_builder_ext.rs index 6362e6343d..0e586dfdcd 100644 --- a/crates/core/app/tests/common/test_node_builder_ext.rs +++ b/crates/core/app/tests/common/test_node_builder_ext.rs @@ -91,7 +91,7 @@ fn generate_penumbra_validator( .incoming() .payment_address(0u32.into()); - let ik = penumbra_stake::IdentityKey(validator_id_vk); + let ik = penumbra_stake::IdentityKey(validator_id_vk.into()); let delegation_denom = DelegationToken::from(ik).denom(); let allocation = Allocation { diff --git a/crates/core/component/stake/src/component/action_handler/validator_definition.rs b/crates/core/component/stake/src/component/action_handler/validator_definition.rs index 192caf76fc..4eb38eb5f1 100644 --- a/crates/core/component/stake/src/component/action_handler/validator_definition.rs +++ b/crates/core/component/stake/src/component/action_handler/validator_definition.rs @@ -9,6 +9,7 @@ use crate::{ use anyhow::{ensure, Context, Result}; use async_trait::async_trait; use cnidarium::StateWrite; +use decaf377_rdsa::VerificationKey; use penumbra_proto::DomainType; #[async_trait] @@ -36,10 +37,8 @@ impl ActionHandler for validator::Definition { // Then, we check the signature: let definition_bytes = self.validator.encode_to_vec(); - self.validator - .identity_key - .0 - .verify(&definition_bytes, &self.auth_sig) + VerificationKey::try_from(self.validator.identity_key.0) + .and_then(|vk| vk.verify(&definition_bytes, &self.auth_sig)) .context("validator definition signature failed to verify")?; let total_funding_bps = self diff --git a/crates/core/component/stake/src/component/stake/tests.rs b/crates/core/component/stake/src/component/stake/tests.rs index 6087318d43..5aa3a5d519 100644 --- a/crates/core/component/stake/src/component/stake/tests.rs +++ b/crates/core/component/stake/src/component/stake/tests.rs @@ -1,6 +1,6 @@ use anyhow::ensure; use cnidarium::{StateDelta, TempStorage}; -use decaf377_rdsa::SigningKey; +use decaf377_rdsa::{SigningKey, SpendAuth, VerificationKey}; use rand_core::OsRng; use tendermint::PublicKey; @@ -19,7 +19,8 @@ async fn test_persistent_identity_by_ck() -> anyhow::Result<()> { let mut state = StateDelta::new(storage.latest_snapshot()); let rng = OsRng; - let persistent_identity = IdentityKey(SigningKey::new(rng).into()); + let vk = VerificationKey::from(SigningKey::::new(OsRng)); + let persistent_identity = IdentityKey(vk.into()); let old_ck_raw = ed25519_consensus::SigningKey::new(rng) .verification_key() diff --git a/crates/core/component/stake/src/delegation_token.rs b/crates/core/component/stake/src/delegation_token.rs index c0bc4509de..9a92358736 100644 --- a/crates/core/component/stake/src/delegation_token.rs +++ b/crates/core/component/stake/src/delegation_token.rs @@ -123,7 +123,7 @@ impl std::hash::Hash for DelegationToken { #[cfg(test)] mod tests { - use decaf377_rdsa::{SigningKey, SpendAuth}; + use decaf377_rdsa::{SigningKey, SpendAuth, VerificationKey}; use super::*; @@ -131,7 +131,8 @@ mod tests { fn delegation_token_denomination_round_trip() { use rand_core::OsRng; - let ik = IdentityKey(SigningKey::::new(OsRng).into()); + let vk = VerificationKey::from(SigningKey::::new(OsRng)); + let ik = IdentityKey(vk.into()); let token = DelegationToken::new(ik); diff --git a/crates/core/component/stake/src/identity_key.rs b/crates/core/component/stake/src/identity_key.rs index 96d7ece5af..3ffc9e19b9 100644 --- a/crates/core/component/stake/src/identity_key.rs +++ b/crates/core/component/stake/src/identity_key.rs @@ -7,7 +7,7 @@ use penumbra_proto::{ }; use serde::{Deserialize, Serialize}; -use decaf377_rdsa::{SpendAuth, VerificationKey}; +use decaf377_rdsa::{SpendAuth, VerificationKeyBytes}; /// The root of a validator's identity. /// @@ -20,7 +20,7 @@ use decaf377_rdsa::{SpendAuth, VerificationKey}; /// designed for custodying funds to protect their identity. #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] #[serde(try_from = "pb::IdentityKey", into = "pb::IdentityKey")] -pub struct IdentityKey(pub VerificationKey); +pub struct IdentityKey(pub VerificationKeyBytes); // IMPORTANT: Changing this implementation is state-breaking. impl std::str::FromStr for IdentityKey { @@ -37,7 +37,7 @@ impl std::str::FromStr for IdentityKey { impl std::fmt::Display for IdentityKey { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_str(&bech32str::encode( - &self.0.to_bytes(), + self.0.as_ref(), BECH32_PREFIX, bech32str::Bech32m, )) @@ -57,7 +57,7 @@ impl DomainType for IdentityKey { impl From for pb::IdentityKey { fn from(ik: IdentityKey) -> Self { pb::IdentityKey { - ik: ik.0.to_bytes().to_vec(), + ik: ik.0.as_ref().to_vec(), } } } diff --git a/crates/core/component/stake/src/rate.rs b/crates/core/component/stake/src/rate.rs index bc8b07fa04..435b24c5e0 100644 --- a/crates/core/component/stake/src/rate.rs +++ b/crates/core/component/stake/src/rate.rs @@ -400,8 +400,8 @@ mod tests { #[test] fn slash_rate_by_penalty() { - let sk = rdsa::SigningKey::new(OsRng); - let ik = IdentityKey((&sk).into()); + let vk = rdsa::VerificationKey::from(rdsa::SigningKey::new(OsRng)); + let ik = IdentityKey(vk.into()); let rate_data = RateData { identity_key: ik, diff --git a/crates/core/component/stake/src/state_key.rs b/crates/core/component/stake/src/state_key.rs index 4a75380569..50a87e680c 100644 --- a/crates/core/component/stake/src/state_key.rs +++ b/crates/core/component/stake/src/state_key.rs @@ -168,8 +168,8 @@ mod tests { #[test] fn penalty_in_epoch_padding() { - let sk = rdsa::SigningKey::new(OsRng); - let ik = IdentityKey((&sk).into()); + let vk = rdsa::VerificationKey::from(rdsa::SigningKey::new(OsRng)); + let ik = IdentityKey(vk.into()); assert_eq!( penalty::for_id_in_epoch(&ik, 791), @@ -180,8 +180,8 @@ mod tests { #[test] fn penalty_in_epoch_sorting() { - let sk = rdsa::SigningKey::new(OsRng); - let ik = IdentityKey((&sk).into()); + let vk = rdsa::VerificationKey::from(rdsa::SigningKey::new(OsRng)); + let ik = IdentityKey(vk.into()); let k791 = penalty::for_id_in_epoch(&ik, 791); let k792 = penalty::for_id_in_epoch(&ik, 792); diff --git a/crates/core/component/stake/src/unbonding_token.rs b/crates/core/component/stake/src/unbonding_token.rs index d3344f7f39..c6921d9e33 100644 --- a/crates/core/component/stake/src/unbonding_token.rs +++ b/crates/core/component/stake/src/unbonding_token.rs @@ -136,7 +136,7 @@ impl std::hash::Hash for UnbondingToken { #[cfg(test)] mod tests { - use decaf377_rdsa::{SigningKey, SpendAuth}; + use decaf377_rdsa::{SigningKey, VerificationKey}; use super::*; @@ -144,7 +144,8 @@ mod tests { fn unbonding_token_denomination_round_trip() { use rand_core::OsRng; - let ik = IdentityKey(SigningKey::::new(OsRng).into()); + let vk = VerificationKey::from(SigningKey::new(OsRng)); + let ik = IdentityKey(vk.into()); let start = 782; let token = UnbondingToken::new(ik, start); diff --git a/crates/core/component/stake/src/undelegate_claim/proof.rs b/crates/core/component/stake/src/undelegate_claim/proof.rs index bf1c6e4f8c..607da700ae 100644 --- a/crates/core/component/stake/src/undelegate_claim/proof.rs +++ b/crates/core/component/stake/src/undelegate_claim/proof.rs @@ -106,6 +106,7 @@ mod tests { use penumbra_proof_params::generate_prepared_test_parameters; use proptest::prelude::*; use rand_core::OsRng; + use rdsa::VerificationKey; use crate::{IdentityKey, Penalty, UnbondingToken}; use penumbra_shielded_pool::ConvertCircuit; @@ -124,7 +125,7 @@ mod tests { let (pk, vk) = generate_prepared_test_parameters::(&mut rng); let sk = rdsa::SigningKey::new_from_field(validator_randomness); - let validator_identity = IdentityKey((&sk).into()); + let validator_identity = IdentityKey(VerificationKey::from(&sk).into()); let unbonding_amount = Amount::from(value1_amount); let start_epoch_index = 1;