diff --git a/crates/bin/pd/src/migrate.rs b/crates/bin/pd/src/migrate.rs index 862a395e42..71c123911d 100644 --- a/crates/bin/pd/src/migrate.rs +++ b/crates/bin/pd/src/migrate.rs @@ -10,6 +10,7 @@ mod testnet72; mod testnet74; mod testnet76; mod testnet77; +mod testnet78; use anyhow::{ensure, Context}; use penumbra_governance::StateReadExt; @@ -47,6 +48,9 @@ pub enum Migration { /// Testnet-77 migration: /// - Reset the halt bit Testnet77, + /// Testnet-78 migration: + /// - Truncate various user-supplied `String` fields to a maximum length. + Testnet78, } impl Migration { @@ -72,30 +76,22 @@ impl Migration { ); tracing::info!("started migration"); + // If this is `ReadyToStart`, we need to reset the halt bit and return early. + if let Migration::ReadyToStart = self { + reset_halt_bit::migrate(storage, pd_home, genesis_start).await?; + return Ok(()); + } + match self { - Migration::ReadyToStart => { - reset_halt_bit::migrate(storage, pd_home, genesis_start).await?; - return Ok(()); - } Migration::SimpleMigration => { simple::migrate(storage, pd_home.clone(), genesis_start).await? } - Migration::Testnet72 => { - testnet72::migrate(storage, pd_home.clone(), genesis_start).await? - } - - Migration::Testnet74 => { - testnet74::migrate(storage, pd_home.clone(), genesis_start).await? + Migration::Testnet78 => { + testnet78::migrate(storage, pd_home.clone(), genesis_start).await? } - - Migration::Testnet76 => { - testnet76::migrate(storage, pd_home.clone(), genesis_start).await? - } - Migration::Testnet77 => { - testnet77::migrate(storage, pd_home.clone(), genesis_start).await? - } - }; + _ => unreachable!(), + } if let Some(comet_home) = comet_home { // TODO avoid this when refactoring to clean up migrations diff --git a/crates/bin/pd/src/migrate/reset_halt_bit.rs b/crates/bin/pd/src/migrate/reset_halt_bit.rs index 4e00605927..a2480cd019 100644 --- a/crates/bin/pd/src/migrate/reset_halt_bit.rs +++ b/crates/bin/pd/src/migrate/reset_halt_bit.rs @@ -16,5 +16,6 @@ pub async fn migrate( let _ = storage.commit_in_place(delta).await?; storage.release().await; tracing::info!("migration completed: halt bit is turned off, chain is ready to start"); + Ok(()) } diff --git a/crates/bin/pd/src/migrate/testnet72.rs b/crates/bin/pd/src/migrate/testnet72.rs index 36b426e970..66a9c4d8b7 100644 --- a/crates/bin/pd/src/migrate/testnet72.rs +++ b/crates/bin/pd/src/migrate/testnet72.rs @@ -1,5 +1,5 @@ //! Contains functions related to the migration script of Testnet72 - +#![allow(dead_code)] use anyhow; use cnidarium::{Snapshot, StateDelta, StateRead, StateWrite, Storage}; use futures::StreamExt as _; diff --git a/crates/bin/pd/src/migrate/testnet74.rs b/crates/bin/pd/src/migrate/testnet74.rs index b88fe4c51c..c99fd8f9f1 100644 --- a/crates/bin/pd/src/migrate/testnet74.rs +++ b/crates/bin/pd/src/migrate/testnet74.rs @@ -1,4 +1,5 @@ //! Contains functions related to the migration script of Testnet74 +#![allow(dead_code)] use anyhow; use cnidarium::{EscapedByteSlice, Snapshot, StateDelta, StateRead, StateWrite, Storage}; diff --git a/crates/bin/pd/src/migrate/testnet76.rs b/crates/bin/pd/src/migrate/testnet76.rs index f4664a23b2..03e383169c 100644 --- a/crates/bin/pd/src/migrate/testnet76.rs +++ b/crates/bin/pd/src/migrate/testnet76.rs @@ -1,4 +1,5 @@ //! Contains functions related to the migration script of Testnet74 +#![allow(dead_code)] use anyhow; use cnidarium::{Snapshot, StateDelta, Storage}; diff --git a/crates/bin/pd/src/migrate/testnet78.rs b/crates/bin/pd/src/migrate/testnet78.rs new file mode 100644 index 0000000000..401b81f35c --- /dev/null +++ b/crates/bin/pd/src/migrate/testnet78.rs @@ -0,0 +1,392 @@ +//! Contains functions related to the migration script of Testnet78. +use cnidarium::{Snapshot, StateDelta, Storage}; +use futures::TryStreamExt as _; +use jmt::RootHash; +use pbjson_types::Any; +use penumbra_app::app::StateReadExt as _; +use penumbra_governance::StateReadExt as _; +use penumbra_proto::{DomainType as _, StateReadProto as _, StateWriteProto as _}; +use penumbra_sct::component::clock::EpochRead as _; +use penumbra_stake::validator::Validator; +use std::path::PathBuf; +use tracing::instrument; + +use crate::testnet::generate::TestnetConfig; + +/// Run the full migration, given an export path and a start time for genesis. +/// +/// Menu: +/// - Truncate various user-supplied `String` fields to a maximum length. +/// * Validators: +/// - `name` (140 bytes) +/// - `website` (70 bytes) +/// - `description` (280 bytes) +/// * Governance Parameter Changes: +/// - `key` (64 bytes) +/// - `value` (2048 bytes) +/// - `component` (64 bytes) +/// * Governance Proposals: +/// - `title` (80 bytes) +/// - `description` (10,000 bytes) +/// * Governance Proposal Withdrawals: +/// - `reason` (1024 bytes) +/// * Governance IBC Client Freeze Proposals: +/// - `client_id` (128 bytes) +/// * Governance IBC Client Unfreeze Proposals: +/// - `client_id` (128 bytes) +/// * Governance Signaling Proposals: +/// - `commit hash` (255 bytes) +#[instrument] +pub async fn migrate( + storage: Storage, + pd_home: PathBuf, + genesis_start: Option, +) -> anyhow::Result<()> { + // Setup: + let initial_state = storage.latest_snapshot(); + let chain_id = initial_state.get_chain_id().await?; + let root_hash = initial_state + .root_hash() + .await + .expect("chain state has a root hash"); + let pre_upgrade_root_hash: RootHash = root_hash.into(); + let pre_upgrade_height = initial_state + .get_block_height() + .await + .expect("chain state has a block height"); + let post_upgrade_height = pre_upgrade_height.wrapping_add(1); + + // We initialize a `StateDelta` and start by reaching into the JMT for all entries matching the + // swap execution prefix. Then, we write each entry to the nv-storage. + let mut delta = StateDelta::new(initial_state); + let (migration_duration, post_upgrade_root_hash) = { + let start_time = std::time::SystemTime::now(); + // Adjust the length of `Validator` fields. + truncate_validator_fields(&mut delta).await?; + + // Adjust the length of governance proposal fields. + truncate_proposal_fields(&mut delta).await?; + + // Adjust the length of governance proposal outcome fields. + truncate_proposal_outcome_fields(&mut delta).await?; + + let post_upgrade_root_hash = storage.commit_in_place(delta).await?; + tracing::info!(?post_upgrade_root_hash, "post-migration root hash"); + + ( + start_time.elapsed().expect("start time not set"), + post_upgrade_root_hash, + ) + }; + storage.release().await; + + // The migration is complete, now we need to generate a genesis file. To do this, we need + // to lookup a validator view from the chain, and specify the post-upgrade app hash and + // initial height. + let app_state = penumbra_app::genesis::Content { + chain_id, + ..Default::default() + }; + let mut genesis = TestnetConfig::make_genesis(app_state.clone()).expect("can make genesis"); + genesis.app_hash = post_upgrade_root_hash + .0 + .to_vec() + .try_into() + .expect("infaillible conversion"); + genesis.initial_height = post_upgrade_height as i64; + genesis.genesis_time = genesis_start.unwrap_or_else(|| { + let now = tendermint::time::Time::now(); + tracing::info!(%now, "no genesis time provided, detecting a testing setup"); + now + }); + let checkpoint = post_upgrade_root_hash.0.to_vec(); + let genesis = TestnetConfig::make_checkpoint(genesis, Some(checkpoint)); + + let genesis_json = serde_json::to_string(&genesis).expect("can serialize genesis"); + tracing::info!("genesis: {}", genesis_json); + let genesis_path = pd_home.join("genesis.json"); + std::fs::write(genesis_path, genesis_json).expect("can write genesis"); + + let validator_state_path = pd_home.join("priv_validator_state.json"); + + let fresh_validator_state = crate::testnet::generate::TestnetValidator::initial_state(); + std::fs::write(validator_state_path, fresh_validator_state).expect("can write validator state"); + + tracing::info!( + pre_upgrade_height, + post_upgrade_height, + ?pre_upgrade_root_hash, + ?post_upgrade_root_hash, + duration = migration_duration.as_secs(), + "successful migration!" + ); + + Ok(()) +} + +/// * Validators: +/// - `name` (140 bytes) +/// - `website` (70 bytes) +/// - `description` (280 bytes) +async fn truncate_validator_fields(delta: &mut StateDelta) -> anyhow::Result<()> { + let key_prefix_validators = penumbra_stake::state_key::validators::definitions::prefix(); + let all_validators = delta + .prefix_proto::(&key_prefix_validators) + .map_ok(|(k, v)| (k, Validator::decode(v.value).expect("only validators"))) + .try_collect::>() + .await?; + + for (key, mut validator) in all_validators { + validator.name = truncate(&validator.name, 140).to_string(); + validator.website = truncate(&validator.website, 70).to_string(); + validator.description = truncate(&validator.description, 280).to_string(); + + delta.put(key, validator); + } + + Ok(()) +} + +/// * Governance Proposals: +/// - `title` (80 bytes) +/// - `description` (10,000 bytes) +/// * Governance Parameter Changes: +/// - `key` (64 bytes) +/// - `value` (2048 bytes) +/// - `component` (64 bytes) +/// * Governance IBC Client Freeze Proposals: +/// - `client_id` (128 bytes) +/// * Governance IBC Client Unfreeze Proposals: +/// - `client_id` (128 bytes) +/// * Governance Signaling Proposals: +/// - `commit hash` (255 bytes) +async fn truncate_proposal_fields(delta: &mut StateDelta) -> anyhow::Result<()> { + let next_proposal_id: u64 = delta.next_proposal_id().await?; + + // Range each proposal and truncate the fields. + for proposal_id in 0..next_proposal_id { + let proposal = delta.proposal_definition(proposal_id).await?; + + if proposal.is_none() { + break; + } + + let mut proposal = proposal.unwrap(); + + proposal.title = truncate(&proposal.title, 80).to_string(); + proposal.description = truncate(&proposal.description, 10_000).to_string(); + + // Depending on the proposal type, we may need to truncate additional fields. + match proposal.payload { + penumbra_governance::ProposalPayload::Signaling { commit } => { + proposal.payload = penumbra_governance::ProposalPayload::Signaling { + commit: commit.map(|commit| truncate(&commit, 255).to_string()), + }; + } + penumbra_governance::ProposalPayload::Emergency { halt_chain: _ } => {} + penumbra_governance::ProposalPayload::ParameterChange(mut param_change) => { + for (i, mut change) in param_change.changes.clone().into_iter().enumerate() { + let key = truncate(&change.key, 64).to_string(); + let value = truncate(&change.value, 2048).to_string(); + let component = truncate(&change.component, 64).to_string(); + + change.key = key; + change.value = value; + change.component = component; + + param_change.changes[i] = change; + } + + for (i, mut change) in param_change.preconditions.clone().into_iter().enumerate() { + let key = truncate(&change.key, 64).to_string(); + let value = truncate(&change.value, 2048).to_string(); + let component = truncate(&change.component, 64).to_string(); + + change.key = key; + change.value = value; + change.component = component; + + param_change.preconditions[i] = change; + } + + proposal.payload = + penumbra_governance::ProposalPayload::ParameterChange(param_change); + } + penumbra_governance::ProposalPayload::CommunityPoolSpend { + transaction_plan: _, + } => {} + penumbra_governance::ProposalPayload::UpgradePlan { height: _ } => {} + penumbra_governance::ProposalPayload::FreezeIbcClient { client_id } => { + proposal.payload = penumbra_governance::ProposalPayload::FreezeIbcClient { + client_id: truncate(&client_id, 128).to_string(), + }; + } + penumbra_governance::ProposalPayload::UnfreezeIbcClient { client_id } => { + proposal.payload = penumbra_governance::ProposalPayload::UnfreezeIbcClient { + client_id: truncate(&client_id, 128).to_string(), + }; + } + }; + + // Store the truncated proposal data + delta.put( + penumbra_governance::state_key::proposal_definition(proposal_id), + proposal.clone(), + ); + } + + Ok(()) +} + +/// * Governance Proposal Withdrawals: +/// - `reason` (1024 bytes) +async fn truncate_proposal_outcome_fields(delta: &mut StateDelta) -> anyhow::Result<()> { + let next_proposal_id: u64 = delta.next_proposal_id().await?; + + // Range each proposal outcome and truncate the fields. + for proposal_id in 0..next_proposal_id { + let proposal_state = delta.proposal_state(proposal_id).await?; + + if proposal_state.is_none() { + break; + } + + let mut proposal_state = proposal_state.unwrap(); + + match proposal_state { + penumbra_governance::proposal_state::State::Withdrawn { reason } => { + proposal_state = penumbra_governance::proposal_state::State::Withdrawn { + reason: truncate(&reason, 1024).to_string(), + }; + } + penumbra_governance::proposal_state::State::Voting => {} + penumbra_governance::proposal_state::State::Finished { ref outcome } => match outcome { + penumbra_governance::proposal_state::Outcome::Passed => {} + penumbra_governance::proposal_state::Outcome::Failed { withdrawn } => { + match withdrawn { + penumbra_governance::proposal_state::Withdrawn::No => {} + penumbra_governance::proposal_state::Withdrawn::WithReason { reason } => { + proposal_state = penumbra_governance::proposal_state::State::Finished { + outcome: penumbra_governance::proposal_state::Outcome::Failed { + withdrawn: + penumbra_governance::proposal_state::Withdrawn::WithReason { + reason: truncate(&reason, 1024).to_string(), + }, + }, + }; + } + } + } + penumbra_governance::proposal_state::Outcome::Slashed { withdrawn } => { + match withdrawn { + penumbra_governance::proposal_state::Withdrawn::No => {} + penumbra_governance::proposal_state::Withdrawn::WithReason { reason } => { + proposal_state = penumbra_governance::proposal_state::State::Finished { + outcome: penumbra_governance::proposal_state::Outcome::Slashed { + withdrawn: + penumbra_governance::proposal_state::Withdrawn::WithReason { + reason: truncate(&reason, 1024).to_string(), + }, + }, + }; + } + } + } + }, + penumbra_governance::proposal_state::State::Claimed { ref outcome } => match outcome { + penumbra_governance::proposal_state::Outcome::Passed => {} + penumbra_governance::proposal_state::Outcome::Failed { withdrawn } => { + match withdrawn { + penumbra_governance::proposal_state::Withdrawn::No => {} + penumbra_governance::proposal_state::Withdrawn::WithReason { reason } => { + proposal_state = penumbra_governance::proposal_state::State::Claimed { + outcome: penumbra_governance::proposal_state::Outcome::Failed { + withdrawn: + penumbra_governance::proposal_state::Withdrawn::WithReason { + reason: truncate(&reason, 1024).to_string(), + }, + }, + }; + } + } + } + penumbra_governance::proposal_state::Outcome::Slashed { withdrawn } => { + match withdrawn { + penumbra_governance::proposal_state::Withdrawn::No => {} + penumbra_governance::proposal_state::Withdrawn::WithReason { reason } => { + proposal_state = penumbra_governance::proposal_state::State::Claimed { + outcome: penumbra_governance::proposal_state::Outcome::Slashed { + withdrawn: + penumbra_governance::proposal_state::Withdrawn::WithReason { + reason: truncate(&reason, 1024).to_string(), + }, + }, + }; + } + } + } + }, + } + + // Store the truncated proposal state data + delta.put( + penumbra_governance::state_key::proposal_state(proposal_id), + proposal_state.clone(), + ); + } + Ok(()) +} + +// Since the limits are based on `String::len`, which returns +// the number of bytes, we need to truncate the UTF-8 strings at the +// correct byte boundaries. +// +// This can be simplified once https://github.com/rust-lang/rust/issues/93743 +// is stabilized. +#[inline] +pub fn floor_char_boundary(s: &str, index: usize) -> usize { + if index >= s.len() { + s.len() + } else { + let lower_bound = index.saturating_sub(3); + let new_index = s.as_bytes()[lower_bound..=index] + .iter() + .rposition(|b| is_utf8_char_boundary(*b)); + + // SAFETY: we know that the character boundary will be within four bytes + lower_bound + new_index.unwrap() + } +} + +#[inline] +pub(crate) const fn is_utf8_char_boundary(b: u8) -> bool { + // This is bit magic equivalent to: b < 128 || b >= 192 + (b as i8) >= -0x40 +} + +// Truncates a utf-8 string to the nearest character boundary, +// not exceeding max_bytes +fn truncate(s: &str, max_bytes: usize) -> &str { + let closest = floor_char_boundary(s, max_bytes); + + &s[..closest] +} + +mod tests { + + #[test] + fn truncation() { + use super::truncate; + let s = "Hello, world!"; + + assert_eq!(truncate(s, 5), "Hello"); + + let s = "โค๏ธ๐Ÿงก๐Ÿ’›๐Ÿ’š๐Ÿ’™๐Ÿ’œ"; + assert_eq!(s.len(), 26); + assert_eq!("โค".len(), 3); + + assert_eq!(truncate(s, 2), ""); + assert_eq!(truncate(s, 3), "โค"); + assert_eq!(truncate(s, 4), "โค"); + } +} diff --git a/crates/bin/pd/src/testnet/generate.rs b/crates/bin/pd/src/testnet/generate.rs index a03f33647a..cf1a1d5fae 100644 --- a/crates/bin/pd/src/testnet/generate.rs +++ b/crates/bin/pd/src/testnet/generate.rs @@ -572,6 +572,22 @@ impl Default for TestnetValidator { impl TryFrom<&TestnetValidator> for Validator { type Error = anyhow::Error; fn try_from(tv: &TestnetValidator) -> anyhow::Result { + // Validation: + // - Website has a max length of 70 bytes + if tv.website.len() > 70 { + anyhow::bail!("validator website field must be less than 70 bytes"); + } + + // - Name has a max length of 140 bytes + if tv.name.len() > 140 { + anyhow::bail!("validator name must be less than 140 bytes"); + } + + // - Description has a max length of 280 bytes + if tv.description.len() > 280 { + anyhow::bail!("validator description must be less than 280 bytes"); + } + Ok(Validator { // Currently there's no way to set validator keys beyond // manually editing the genesis.json. Otherwise they diff --git a/crates/core/component/governance/src/change.rs b/crates/core/component/governance/src/change.rs index 91f4f881c2..e2b877186a 100644 --- a/crates/core/component/governance/src/change.rs +++ b/crates/core/component/governance/src/change.rs @@ -20,6 +20,23 @@ impl DomainType for EncodedParameter { impl TryFrom for EncodedParameter { type Error = anyhow::Error; fn try_from(value: pb::EncodedParameter) -> Result { + // TODO: what are max key/value lengths here? + // Validation: + // - Key has max length of 64 chars + if value.key.len() > 64 { + anyhow::bail!("key length must be less than or equal to 64 characters"); + } + + // - Value has max length of 2048 chars + if value.value.len() > 2048 { + anyhow::bail!("value length must be less than or equal to 2048 characters"); + } + + // - Component has max length of 64 chars + if value.component.len() > 64 { + anyhow::bail!("component length must be less than or equal to 64 characters"); + } + Ok(EncodedParameter { component: value.component, key: value.key, diff --git a/crates/core/component/governance/src/proposal.rs b/crates/core/component/governance/src/proposal.rs index a69b6f35d6..1f7b3bb2bb 100644 --- a/crates/core/component/governance/src/proposal.rs +++ b/crates/core/component/governance/src/proposal.rs @@ -82,6 +82,17 @@ impl TryFrom for Proposal { type Error = anyhow::Error; fn try_from(inner: pb::Proposal) -> Result { + // Validation (matches limits from `impl AppActionHandler for ProposalSubmit`): + // - Title has a max length of 80 chars + if inner.title.len() > 80 { + anyhow::bail!("proposal title field must be less than 80 characters"); + } + + // - Description has a max length of 10_000 chars + if inner.description.len() > 10_000 { + anyhow::bail!("proposal description must be less than 10,000 characters"); + } + use pb::proposal::Payload; Ok(Proposal { id: inner.id, @@ -95,6 +106,11 @@ impl TryFrom for Proposal { commit: if signaling.commit.is_empty() { None } else { + // Commit hash has max length of 255 bytes: + if signaling.commit.len() > 255 { + anyhow::bail!("proposal commit hash must be less than 255 bytes"); + } + Some(signaling.commit) }, }, @@ -123,10 +139,20 @@ impl TryFrom for Proposal { Payload::UpgradePlan(upgrade_plan) => ProposalPayload::UpgradePlan { height: upgrade_plan.height, }, - Payload::FreezeIbcClient(freeze_ibc_client) => ProposalPayload::FreezeIbcClient { - client_id: freeze_ibc_client.client_id, - }, + Payload::FreezeIbcClient(freeze_ibc_client) => { + // Validation: client ID has a max length of 128 bytes + if freeze_ibc_client.client_id.len() > 128 { + anyhow::bail!("client ID must be less than 128 bytes"); + } + ProposalPayload::FreezeIbcClient { + client_id: freeze_ibc_client.client_id, + } + } Payload::UnfreezeIbcClient(unfreeze_ibc_client) => { + // Validation: client ID has a max length of 128 bytes + if unfreeze_ibc_client.client_id.len() > 128 { + anyhow::bail!("client ID must be less than 128 bytes"); + } ProposalPayload::UnfreezeIbcClient { client_id: unfreeze_ibc_client.client_id, } diff --git a/crates/core/component/governance/src/proposal_state.rs b/crates/core/component/governance/src/proposal_state.rs index 56af34b8c4..5525a2c29e 100644 --- a/crates/core/component/governance/src/proposal_state.rs +++ b/crates/core/component/governance/src/proposal_state.rs @@ -2,6 +2,8 @@ use serde::{Deserialize, Serialize}; use penumbra_proto::{penumbra::core::component::governance::v1 as pb, DomainType}; +use crate::MAX_VALIDATOR_VOTE_REASON_LENGTH; + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[serde(try_from = "pb::ProposalState", into = "pb::ProposalState")] pub enum State { @@ -292,6 +294,11 @@ impl TryFrom for Outcome { withdrawn, }) => Outcome::Failed { withdrawn: if let Some(pb::proposal_outcome::Withdrawn { reason }) = withdrawn { + // Max reason length is 1kb + if reason.len() > MAX_VALIDATOR_VOTE_REASON_LENGTH { + anyhow::bail!("withdrawn reason must be smaller than 1kb") + } + Withdrawn::WithReason { reason } } else { Withdrawn::No @@ -301,6 +308,10 @@ impl TryFrom for Outcome { withdrawn, }) => Outcome::Slashed { withdrawn: if let Some(pb::proposal_outcome::Withdrawn { reason }) = withdrawn { + // Max reason length is 1kb + if reason.len() > MAX_VALIDATOR_VOTE_REASON_LENGTH { + anyhow::bail!("withdrawn reason must be smaller than 1kb") + } Withdrawn::WithReason { reason } } else { Withdrawn::No @@ -360,14 +371,35 @@ impl TryFrom for Outcome<()> { } pb::proposal_outcome::Outcome::Failed(pb::proposal_outcome::Failed { withdrawn, - }) => Outcome::Failed { - withdrawn: >::from(withdrawn.map(|w| w.reason)).try_into()?, - }, + }) => { + // Max reason length is 1kb + if withdrawn.is_some() { + let reason = &withdrawn.as_ref().expect("reason is some").reason; + if reason.len() > MAX_VALIDATOR_VOTE_REASON_LENGTH { + anyhow::bail!("withdrawn reason must be smaller than 1kb"); + } + } + Outcome::Failed { + withdrawn: >::from(withdrawn.map(|w| w.reason)) + .try_into()?, + } + } pb::proposal_outcome::Outcome::Slashed(pb::proposal_outcome::Slashed { withdrawn, - }) => Outcome::Slashed { - withdrawn: >::from(withdrawn.map(|w| w.reason)).try_into()?, - }, + }) => { + // Max reason length is 1kb + if withdrawn.is_some() { + let reason = &withdrawn.as_ref().expect("reason is some").reason; + if reason.len() > MAX_VALIDATOR_VOTE_REASON_LENGTH { + anyhow::bail!("withdrawn reason must be smaller than 1kb"); + } + } + + Outcome::Slashed { + withdrawn: >::from(withdrawn.map(|w| w.reason)) + .try_into()?, + } + } }, ) } diff --git a/crates/core/component/governance/src/validator_vote/action.rs b/crates/core/component/governance/src/validator_vote/action.rs index 140ba1e319..d5e69db700 100644 --- a/crates/core/component/governance/src/validator_vote/action.rs +++ b/crates/core/component/governance/src/validator_vote/action.rs @@ -4,7 +4,7 @@ use penumbra_stake::{GovernanceKey, IdentityKey}; use penumbra_txhash::{EffectHash, EffectingData}; use serde::{Deserialize, Serialize}; -use crate::vote::Vote; +use crate::{vote::Vote, MAX_VALIDATOR_VOTE_REASON_LENGTH}; /// A vote by a validator. #[derive(Debug, Clone, Serialize, Deserialize)] @@ -138,6 +138,11 @@ impl TryFrom for ValidatorVoteReason { type Error = anyhow::Error; fn try_from(msg: pb::ValidatorVoteReason) -> Result { + // Check the length of the validator reason field. + if msg.reason.len() > MAX_VALIDATOR_VOTE_REASON_LENGTH { + anyhow::bail!("validator vote reason is too long"); + } + Ok(ValidatorVoteReason(msg.reason)) } } diff --git a/crates/core/component/stake/src/validator.rs b/crates/core/component/stake/src/validator.rs index 4117c180b6..858b182a44 100644 --- a/crates/core/component/stake/src/validator.rs +++ b/crates/core/component/stake/src/validator.rs @@ -133,6 +133,22 @@ impl TryFrom for Validator { type Error = anyhow::Error; fn try_from(v: ValidatorToml) -> anyhow::Result { + // Validation: + // - Website has a max length of 70 bytes + if v.website.len() > 70 { + anyhow::bail!("validator website field must be less than 70 bytes"); + } + + // - Name has a max length of 140 bytes + if v.name.len() > 140 { + anyhow::bail!("validator name must be less than 140 bytes"); + } + + // - Description has a max length of 280 bytes + if v.description.len() > 280 { + anyhow::bail!("validator description must be less than 280 bytes"); + } + Ok(Validator { identity_key: v.identity_key, governance_key: v.governance_key, @@ -224,6 +240,22 @@ impl From for pb::Validator { impl TryFrom for Validator { type Error = anyhow::Error; fn try_from(v: pb::Validator) -> Result { + // Validation: + // - Website has a max length of 70 bytes + if v.website.len() > 70 { + anyhow::bail!("validator website field must be less than 70 bytes"); + } + + // - Name has a max length of 140 bytes + if v.name.len() > 140 { + anyhow::bail!("validator name must be less than 140 bytes"); + } + + // - Description has a max length of 280 bytes + if v.description.len() > 280 { + anyhow::bail!("validator description must be less than 280 bytes"); + } + Ok(Validator { identity_key: v .identity_key diff --git a/crates/core/component/stake/src/validator/definition.rs b/crates/core/component/stake/src/validator/definition.rs index 34166d7154..f3cf5fc89d 100644 --- a/crates/core/component/stake/src/validator/definition.rs +++ b/crates/core/component/stake/src/validator/definition.rs @@ -29,11 +29,13 @@ impl From for pb::ValidatorDefinition { impl TryFrom for Definition { type Error = anyhow::Error; fn try_from(v: pb::ValidatorDefinition) -> Result { + let validator = v + .validator + .ok_or_else(|| anyhow::anyhow!("missing validator field in proto"))?; + // Validation: + // The validator fields are validated by the `try_into` call below: Ok(Definition { - validator: v - .validator - .ok_or_else(|| anyhow::anyhow!("missing validator field in proto"))? - .try_into()?, + validator: validator.try_into()?, auth_sig: v.auth_sig.as_slice().try_into()?, }) }