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

feat(pd): add app_version checks in local storage #4918

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 122 additions & 0 deletions crates/bin/pd/src/compat.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
//! Logic for determining protocol compatibility, as defined by the
//! workspace-wide APP_VERSION value. On startup, `pd` will inspect
//! the APP_VERSION set in the local non-verifiable storage, and warn
//! or fail about mismatches.
//!
//!
//! TODO: move this code into the `app` crate.
use anyhow;
use cnidarium::{StateDelta, StateRead, StateWrite, Storage};
// use penumbra_app::app::StateReadExt as _;
use penumbra_app::APP_VERSION;

const APP_VERSION_KEY: &str = "app_version";
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this with the other application state keys


/// Retrieve the APP_VERSION last written to local non-verifiable storage.
/// Returns an Option, because as late as v0.80.8, APP_VERSION was not
/// written to local state.
pub async fn get_state_version(storage: Storage) -> anyhow::Result<Option<u64>> {
let snapshot = storage.latest_snapshot();
let result = snapshot
.nonverifiable_get_raw(APP_VERSION_KEY.as_bytes())
.await?;

let local_app_version = match result {
Some(v) => {
let app_version: AppVersion = v.try_into()?;
Some(app_version.try_into()?)
}
None => None,
};
Ok(local_app_version)
}

/// Check whether the local state matches the current APP_VERSION.
/// If it's not set yet, set it to the current value.
/// If it's a surprising value, error.
pub async fn check_state_version(storage: Storage) -> anyhow::Result<u64> {
let local_version = get_state_version(storage.clone()).await?;
match local_version {
Some(v) => {
if v > APP_VERSION {
anyhow::bail!(
"state version {v} is newer than current app version {APP_VERSION}; you should upgrade pd",
)
} else if v < APP_VERSION {
anyhow::bail!(
"state version {v} is older than current app version {APP_VERSION}; you should run 'pd migrate'",
)
} else {
Ok(v)
}
}
// If not set, set it.
None => {
set_state_version(storage, APP_VERSION).await?;
Ok(APP_VERSION)
}
}
}

/// Write the given version to the local non-verifiable storage.
pub async fn set_state_version(storage: Storage, version: u64) -> anyhow::Result<()> {
let v = AppVersion(version);
let snapshot = storage.latest_snapshot();
let mut delta = StateDelta::new(snapshot);
delta.nonverifiable_put_raw(APP_VERSION_KEY.to_string().into(), v.try_into()?);
Ok(())
}

/// Wrapper struct representing the `APP_VERSION`, intended
/// for custom TryInto/TryFrom implementations.
#[derive(Debug)]
struct AppVersion(u64);
Copy link
Member

@erwanor erwanor Nov 13, 2024

Choose a reason for hiding this comment

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

If we write a proto to storage we can dispense with the wrapper type and custom serialization. In general, we should try to avoid doing custom serialization and using the _raw methods, preferring extension traits like StateWriteProto.

Here's an example of writing a primitive type to NV storage: https://github.com/penumbra-zone/penumbra/blob/main/crates/core/component/governance/src/component/view.rs#L978


use std::convert::TryFrom;

impl From<u64> for AppVersion {
fn from(i: u64) -> Self {
Self(i)
}
}

impl From<AppVersion> for u64 {
fn from(v: AppVersion) -> Self {
v.0
}
}

/// Ensure that bytes from be read out of non-verifiable [Storage]
/// and interpreted as an AppVersion.
impl TryFrom<Vec<u8>> for AppVersion {
type Error = anyhow::Error;

fn try_from(bytes: Vec<u8>) -> anyhow::Result<Self> {
if bytes.len() > 8 {
anyhow::bail!("jawn is bad");
}

let mut result: u64 = 0;
for (i, &byte) in bytes.iter().enumerate() {
result |= (byte as u64) << (i * 8);
}
Ok(Self(result))
}
}

/// Ensure that an AppVersion can be written to non-verifiable [Storage] as bytes.
impl TryFrom<AppVersion> for Vec<u8> {
type Error = anyhow::Error;

fn try_from(value: AppVersion) -> anyhow::Result<Self> {
let mut bytes = Vec::with_capacity(8);
let mut remaining = value.0;

while remaining > 0 || bytes.is_empty() {
bytes.push((remaining & 0xFF) as u8);
remaining >>= 8;
}

Ok(bytes)
}
}
1 change: 1 addition & 0 deletions crates/bin/pd/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
pub mod metrics;

pub mod cli;
pub mod compat;
pub mod migrate;
pub mod network;
pub mod zipserve;
Expand Down
6 changes: 5 additions & 1 deletion crates/bin/pd/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use cnidarium::Storage;
use metrics_exporter_prometheus::PrometheusBuilder;
use pd::{
cli::{NetworkCommand, Opt, RootCommand},
compat::get_state_version,
migrate::Migration::{Mainnet1, ReadyToStart},
network::{
config::{get_network_dir, parse_tm_address, url_has_necessary_parts},
Expand Down Expand Up @@ -97,11 +98,14 @@ async fn main() -> anyhow::Result<()> {
};
let rocksdb_home = pd_home.join("rocksdb");

let storage = Storage::load(rocksdb_home, SUBSTORE_PREFIXES.to_vec())
let storage = Storage::load(rocksdb_home.clone(), SUBSTORE_PREFIXES.to_vec())
.await
.context(
"Unable to initialize RocksDB storage - is there another `pd` process running?",
)?;
tracing::debug!(?rocksdb_home, "inspecting state version");
let v = get_state_version(storage.clone()).await?.unwrap();
tracing::warn!("found state version: {}", v);

tracing::info!(
?abci_bind,
Expand Down
Loading