UIP-6: App Version Safeguard

crates/bin/pd/src/cli.rs

@@ -133,6 +133,7 @@ pub enum RootCommand {
         #[clap(long, display_order = 200)]
         comet_home: Option<PathBuf>,
         /// If set, force a migration to occur even if the chain is not halted.
+        /// Will not override a detected mismatch in state versions.
         #[clap(long, display_order = 1000)]
         force: bool,
         /// If set, edit local state to permit the node to start, despite

crates/bin/pd/src/main.rs

@@ -19,6 +19,7 @@ use pd::{
         join::network_join,
     },
 };
+use penumbra_app::app_version::assert_latest_app_version;
 use penumbra_app::SUBSTORE_PREFIXES;
 use rand::Rng;
 use rand_core::OsRng;
@@ -102,6 +103,7 @@ async fn main() -> anyhow::Result<()> {
                 .context(
                     "Unable to initialize RocksDB storage - is there another `pd` process running?",
                 )?;
+            assert_latest_app_version(storage.clone()).await?;
 
             tracing::info!(
                 ?abci_bind,

crates/bin/pd/src/migrate/mainnet1.rs

@@ -7,6 +7,7 @@ use ibc_types::core::channel::{Packet, PortId};
 use ibc_types::transfer::acknowledgement::TokenTransferAcknowledgement;
 use jmt::RootHash;
 use penumbra_app::app::StateReadExt as _;
+use penumbra_app::app_version::migrate_app_version;
 use penumbra_governance::StateWriteExt;
 use penumbra_ibc::{component::ChannelStateWriteExt as _, IbcRelay};
 use penumbra_sct::component::clock::EpochManager;
@@ -111,6 +112,16 @@ pub async fn migrate(
     let (migration_duration, post_upgrade_root_hash) = {
         let start_time = std::time::SystemTime::now();
 
+        // Note, when this bit of code was added, the upgrade happened months ago,
+        // and the version safeguard mechanism was not in place. However,
+        // adding this will prevent someone running version 0.80.X with the
+        // safeguard patch from accidentally running the migraton again, since they
+        // will already have version 8 written into the state. But, if someone is syncing
+        // up from genesis, then version 0.79 will not have written anything into the safeguard,
+        // and this method will not complain. So, this addition provides a safeguard
+        // for existing nodes, while also not impeding syncing up a node from scratch.
+        migrate_app_version(&mut delta, 8).await?;
+
         // Reinsert all of the erroneously removed packets
         replace_lost_packets(&mut delta).await?;
 

crates/core/app/src/app/state_key.rs

@@ -1,3 +1,9 @@
+pub mod app_version {
+    pub fn safeguard() -> &'static str {
+        "application/version/safeguard"
+    }
+}
+
 pub mod genesis {
     pub fn app_state() -> &'static str {
         "application/genesis/app_state"

crates/core/app/src/app_version.rs

@@ -0,0 +1,10 @@
+/// Representation of the Penumbra application version. Notably, this is distinct
+/// from the crate version(s). This number should only ever be incremented.
+pub const APP_VERSION: u64 = 8;
+
+cfg_if::cfg_if! {
+    if #[cfg(feature="component")] {
+        mod component;
+        pub use component::{assert_latest_app_version, migrate_app_version};
+    }
+}

crates/core/app/src/app_version/component.rs

@@ -0,0 +1,154 @@
+use std::fmt::Write as _;
+
+use anyhow::{anyhow, Context};
+use cnidarium::{StateDelta, Storage};
+use penumbra_proto::{StateReadProto, StateWriteProto};
+
+use super::APP_VERSION;
+
+fn version_to_software_version(version: u64) -> &'static str {
+    match version {
+        1 => "v0.70.x",
+        2 => "v0.73.x",
+        3 => "v0.74.x",
+        4 => "v0.75.x",
+        5 => "v0.76.x",
+        6 => "v0.77.x",
+        7 => "v0.79.x",
+        8 => "v0.80.x",
+        _ => "unknown",
+    }
+}
+
+#[derive(Debug, Clone, Copy)]
+enum CheckContext {
+    Running,
+    Migration,
+}
+
+fn check_version(ctx: CheckContext, expected: u64, found: Option<u64>) -> anyhow::Result<()> {
+    let found = match (expected, found) {
+        (x, Some(y)) if x != y => y,
+        _ => return Ok(()),
+    };
+    match ctx {
+        CheckContext::Running => {
+            let expected_name = version_to_software_version(expected);
+            let found_name = version_to_software_version(found);
+            let mut error = String::new();
+            error.push_str("app version mismatch:\n");
+            write!(
+                &mut error,
+                "  expected {} (penumbra {})\n",
+                expected, expected_name
+            )?;
+            write!(&mut error, "  found {} (penumbra {})\n", found, found_name)?;
+            write!(
+                &mut error,
+                "make sure you're running penumbra {}",
+                expected_name
+            )?;
+            Err(anyhow!(error))
+        }
+        CheckContext::Migration => {
+            let expected_name = version_to_software_version(expected);
+            let found_name = version_to_software_version(found);
+            let mut error = String::new();
+            if found == APP_VERSION {
+                write!(
+                    &mut error,
+                    "state already migrated to version {}",
+                    APP_VERSION
+                )?;
+                anyhow::bail!(error);
+            }
+            error.push_str("app version mismatch:\n");
+            write!(
+                &mut error,
+                "  expected {} (penumbra {})\n",
+                expected, expected_name
+            )?;
+            write!(&mut error, "  found {} (penumbra {})\n", found, found_name)?;
+            write!(
+                &mut error,
+                "this migration should be run with penumbra {} instead",
+                version_to_software_version(expected + 1)
+            )?;
+            Err(anyhow!(error))
+        }
+    }
+}
+
+async fn read_app_version_safeguard<S: StateReadProto>(s: &S) -> anyhow::Result<Option<u64>> {
+    let out = s
+        .nonverifiable_get_proto(crate::app::state_key::app_version::safeguard().as_bytes())
+        .await
+        .context("while reading app version safeguard")?;
+    Ok(out)
+}
+
+// Neither async nor a result are needed, but only right now, so I'm putting these here
+// to reserve the right to change them later.
+async fn write_app_version_safeguard<S: StateWriteProto>(s: &mut S, x: u64) -> anyhow::Result<()> {
+    s.nonverifiable_put_proto(
+        crate::app::state_key::app_version::safeguard()
+            .as_bytes()
+            .to_vec(),
+        x,
+    );
+    Ok(())
+}
+
+/// Assert that the app version saved in the state is the correct one.
+///
+/// You should call this before starting a node.
+///
+/// This will succeed if no app version was found in local storage, or if the app version saved matches
+/// exactly.
+///
+/// This will also result in the current app version being stored, so that future
+/// calls to this function will be checked against this state.
+pub async fn assert_latest_app_version(s: Storage) -> anyhow::Result<()> {
+    // If the storage is not initialized, avoid touching it at all,
+    // to avoid complaints about it already being initialized before the first genesis.
+    if s.latest_version() == u64::MAX {
+        return Ok(());
+    }
+    let mut delta = StateDelta::new(s.latest_snapshot());
+    let found = read_app_version_safeguard(&delta).await?;
+    check_version(CheckContext::Running, APP_VERSION, found)?;
+    write_app_version_safeguard(&mut delta, APP_VERSION).await?;
+    s.commit(delta).await?;
+    Ok(())
+}
+
+/// Migrate the app version to a given number.
+///
+/// This will check that the app version is currently the previous version, if set at all.
+///
+/// This is the recommended way to change the app version, and should be called during a migration
+/// with breaking consensus logic.
+pub async fn migrate_app_version<S: StateWriteProto>(s: &mut S, to: u64) -> anyhow::Result<()> {
+    anyhow::ensure!(to > 1, "you can't migrate to the first penumbra version!");
+    let found = read_app_version_safeguard(s).await?;
+    check_version(CheckContext::Migration, to - 1, found)?;
+    write_app_version_safeguard(s, to).await?;
+    Ok(())
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    #[test]
+    /// Confirm there's a matching branch on the APP_VERSION to crate version lookup.
+    /// It's possible to overlook that update when bumping the APP_VERSION, so this test
+    /// ensures that if the APP_VERSION was changed, so was the match arm.
+    fn ensure_app_version_is_current_in_checks() -> anyhow::Result<()> {
+        let result = version_to_software_version(APP_VERSION);
+        assert!(
+            result != "unknown",
+            "APP_VERSION lacks a corresponding software version"
+        );
+        Ok(())
+    }
+}

crates/core/app/src/lib.rs

@@ -13,9 +13,8 @@ pub static SUBSTORE_PREFIXES: Lazy<Vec<String>> = Lazy::new(|| {
 /// The substore prefix used for storing historical CometBFT block data.
 pub static COMETBFT_SUBSTORE_PREFIX: &'static str = "cometbft-data";
 
-/// Representation of the Penumbra application version. Notably, this is distinct
-/// from the crate version(s). This number should only ever be incremented.
-pub const APP_VERSION: u64 = 8;
+pub mod app_version;
+pub use app_version::APP_VERSION;
 
 pub mod genesis;
 pub mod params;