FMD Parameter Selection Breaking Changes (#4137)

The first part of #1087. This restructures the way FMD parameters are handled so that we can choose different algorithms to update the parameters. A migration has been written so that we move from the old format, where we had a hard-coded parameter, to a new format, where we have an algorithm as a chain parameter, and then update the current FMD parameter over time based on that algorithm. The protobufs are also structured so that we can easily add new algorithms over time, without making a breaking state change. By default, the algorithm just chooses a constant parameter, so dynamic FMD params are not part of this PR. --------- Signed-off-by: Lúcás Meier <[email protected]> Co-authored-by: Erwan Or <[email protected]>
penumbra-zone · May 21, 2024 · b5d13e9 · b5d13e9
1 parent 567e0eb
commit b5d13e9
Show file tree

Hide file tree

Showing 40 changed files with 799 additions and 120 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/bin/pd/src/migrate.rs b/crates/bin/pd/src/migrate.rs
@@ -41,6 +41,7 @@ pub enum Migration {
     Testnet74,
     /// Testnet-76 migration:
     /// - Heal the auction component's VCB tally.
+    /// - Update FMD parameters to new protobuf structure.
     Testnet76,
 }
 

diff --git a/crates/bin/pd/src/migrate/testnet76.rs b/crates/bin/pd/src/migrate/testnet76.rs
@@ -6,10 +6,13 @@ use futures::TryStreamExt;
 use jmt::RootHash;
 use pbjson_types::Any;
 use penumbra_app::app::StateReadExt as _;
+use penumbra_app::SUBSTORE_PREFIXES;
 use penumbra_asset::Balance;
 use penumbra_auction::auction::dutch::DutchAuction;
 use penumbra_proto::{DomainType, StateReadProto, StateWriteProto};
 use penumbra_sct::component::clock::{EpochManager, EpochRead};
+use penumbra_shielded_pool::params::ShieldedPoolParameters;
+use penumbra_shielded_pool::{component::StateWriteExt as _, fmd::Parameters as FmdParameters};
 use std::path::PathBuf;
 use tracing::instrument;
 
@@ -60,6 +63,17 @@ async fn heal_auction_vcb(delta: &mut StateDelta<Snapshot>) -> anyhow::Result<()
     Ok(())
 }
 
+async fn write_shielded_pool_params(delta: &mut StateDelta<Snapshot>) -> anyhow::Result<()> {
+    delta.put_shielded_pool_params(ShieldedPoolParameters::default());
+    Ok(())
+}
+
+async fn write_fmd_params(delta: &mut StateDelta<Snapshot>) -> anyhow::Result<()> {
+    delta.put_previous_fmd_parameters(FmdParameters::default());
+    delta.put_current_fmd_parameters(FmdParameters::default());
+    Ok(())
+}
+
 /// Run the full migration, given an export path and a start time for genesis.
 ///
 /// Menu:
@@ -71,22 +85,25 @@ pub async fn migrate(
     genesis_start: Option<tendermint::time::Time>,
 ) -> anyhow::Result<()> {
     // Setup:
-    let snapshot = storage.latest_snapshot();
-    let chain_id = snapshot.get_chain_id().await?;
-    let root_hash = snapshot.root_hash().await.expect("can get root hash");
+    let export_state = storage.latest_snapshot();
+    let root_hash = export_state.root_hash().await.expect("can get root hash");
     let pre_upgrade_root_hash: RootHash = root_hash.into();
-    let pre_upgrade_height = snapshot
+    let pre_upgrade_height = export_state
         .get_block_height()
         .await
         .expect("can get 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(snapshot);
+    let mut delta = StateDelta::new(export_state);
     let (migration_duration, post_upgrade_root_hash) = {
         let start_time = std::time::SystemTime::now();
 
+        // Set shield pool params to the new default
+        write_shielded_pool_params(&mut delta).await?;
+        // Initialize fmd params
+        write_fmd_params(&mut delta).await?;
         // Reconstruct a VCB balance for the auction component.
         heal_auction_vcb(&mut delta).await?;
 
@@ -95,16 +112,20 @@ pub async fn migrate(
         tracing::info!(?post_upgrade_root_hash, "post-upgrade root hash");
 
         (
-            start_time.elapsed().expect("start time is set"),
+            start_time.elapsed().expect("start time not set"),
             post_upgrade_root_hash,
         )
     };
 
     storage.release().await;
+    let rocksdb_dir = pd_home.join("rocksdb");
+    let storage = Storage::load(rocksdb_dir, SUBSTORE_PREFIXES.to_vec()).await?;
+    let migrated_state = storage.latest_snapshot();
 
     // 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 chain_id = migrated_state.get_chain_id().await?;
     let app_state = penumbra_app::genesis::Content {
         chain_id,
         ..Default::default()
@@ -121,7 +142,6 @@ pub async fn migrate(
         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));
 
@@ -131,6 +151,7 @@ pub async fn migrate(
     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");
 

diff --git a/crates/cnidarium/src/gen/proto_descriptor.bin.no_lfs b/crates/cnidarium/src/gen/proto_descriptor.bin.no_lfs
diff --git a/crates/core/app/Cargo.toml b/crates/core/app/Cargo.toml
@@ -54,6 +54,7 @@ penumbra-shielded-pool           = { workspace = true, features = ["component"],
 penumbra-stake                   = { workspace = true, default-features = true }
 penumbra-tct                     = { workspace = true, default-features = true }
 penumbra-tendermint-proxy        = { path = "../../util/tendermint-proxy" }
+penumbra-test-subscriber         = { workspace = true }
 penumbra-tower-trace             = {  path = "../../util/tower-trace"  }
 penumbra-transaction             = { workspace = true, features = ["parallel"], default-features = true }
 penumbra-txhash                  = { workspace = true, default-features = true }
@@ -82,10 +83,10 @@ tracing                          = { workspace = true }
 url                              = { workspace = true }
 
 [dev-dependencies]
+decaf377-fmd             = { workspace = true, default-features = true }
 ed25519-consensus        = { workspace = true }
 penumbra-mock-consensus  = { workspace = true }
 penumbra-mock-client     = { workspace = true }
-penumbra-test-subscriber = { workspace = true }
 rand                     = { workspace = true }
 rand_core                = { workspace = true }
 rand_chacha              = { workspace = true }

diff --git a/crates/core/app/src/action_handler/transaction.rs b/crates/core/app/src/action_handler/transaction.rs
@@ -4,6 +4,7 @@ use anyhow::Result;
 use async_trait::async_trait;
 use cnidarium::{StateRead, StateWrite};
 use penumbra_sct::{component::source::SourceContext, CommitmentSource};
+use penumbra_shielded_pool::component::ClueManager;
 use penumbra_transaction::Transaction;
 use tokio::task::JoinSet;
 use tracing::{instrument, Instrument};
@@ -105,6 +106,18 @@ impl AppActionHandler for Transaction {
         // Delete the note source, in case someone else tries to read it.
         state.put_current_source(None);
 
+        // Record all the clues in this transaction
+        // To avoid recomputing a hash.
+        let id = self.id();
+        for clue in self
+            .transaction_body
+            .detection_data
+            .iter()
+            .flat_map(|x| x.fmd_clues.iter())
+        {
+            state.record_clue(clue.clone(), id.clone()).await?;
+        }
+
         Ok(())
     }
 }
@@ -172,7 +185,7 @@ mod tests {
                 clue_plans: vec![CluePlan::new(
                     &mut OsRng,
                     test_keys::ADDRESS_1.deref().clone(),
-                    1,
+                    1.try_into().unwrap(),
                 )],
             }),
             memo: None,

diff --git a/crates/core/app/src/action_handler/transaction/stateful.rs b/crates/core/app/src/action_handler/transaction/stateful.rs
@@ -10,8 +10,6 @@ use penumbra_transaction::{Transaction, TransactionParameters};
 
 use crate::app::StateReadExt;
 
-const FMD_GRACE_PERIOD_BLOCKS: u64 = 10;
-
 pub async fn tx_parameters_historical_check<S: StateRead>(
     state: S,
     transaction: &Transaction,
@@ -73,6 +71,11 @@ pub async fn expiry_height_is_valid<S: StateRead>(state: S, expiry_height: u64)
 }
 
 pub async fn fmd_parameters_valid<S: StateRead>(state: S, transaction: &Transaction) -> Result<()> {
+    let meta_params = state
+        .get_shielded_pool_params()
+        .await
+        .expect("chain params request must succeed")
+        .fmd_meta_params;
     let previous_fmd_parameters = state
         .get_previous_fmd_parameters()
         .await
@@ -84,6 +87,7 @@ pub async fn fmd_parameters_valid<S: StateRead>(state: S, transaction: &Transact
     let height = state.get_block_height().await?;
     fmd_precision_within_grace_period(
         transaction,
+        meta_params,
         previous_fmd_parameters,
         current_fmd_parameters,
         height,
@@ -93,14 +97,15 @@ pub async fn fmd_parameters_valid<S: StateRead>(state: S, transaction: &Transact
 #[tracing::instrument(
     skip_all,
     fields(
-        current_fmd.precision_bits = current_fmd_parameters.precision_bits,
-        previous_fmd.precision_bits = previous_fmd_parameters.precision_bits,
+        current_fmd.precision_bits = current_fmd_parameters.precision.bits(),
+        previous_fmd.precision_bits = previous_fmd_parameters.precision.bits(),
         previous_fmd.as_of_block_height = previous_fmd_parameters.as_of_block_height,
         block_height,
     )
 )]
 pub fn fmd_precision_within_grace_period(
     tx: &Transaction,
+    meta_params: fmd::MetaParameters,
     previous_fmd_parameters: fmd::Parameters,
     current_fmd_parameters: fmd::Parameters,
     block_height: u64,
@@ -112,12 +117,12 @@ pub fn fmd_precision_within_grace_period(
         .fmd_clues
     {
         // Clue must be using the current `fmd::Parameters`, or be within
-        // `FMD_GRACE_PERIOD_BLOCKS` of the previous `fmd::Parameters`.
-        let clue_precision = clue.precision_bits();
-        let using_current_precision = clue_precision == current_fmd_parameters.precision_bits;
-        let using_previous_precision = clue_precision == previous_fmd_parameters.precision_bits;
-        let within_grace_period =
-            block_height < previous_fmd_parameters.as_of_block_height + FMD_GRACE_PERIOD_BLOCKS;
+        // `fmd_grace_period_blocks` of the previous `fmd::Parameters`.
+        let clue_precision = clue.precision()?;
+        let using_current_precision = clue_precision == current_fmd_parameters.precision;
+        let using_previous_precision = clue_precision == previous_fmd_parameters.precision;
+        let within_grace_period = block_height
+            < previous_fmd_parameters.as_of_block_height + meta_params.fmd_grace_period_blocks;
         if using_current_precision || (using_previous_precision && within_grace_period) {
             continue;
         } else {

diff --git a/crates/core/app/src/params/change.rs b/crates/core/app/src/params/change.rs
@@ -90,10 +90,7 @@ impl AppParameters {
                     outbound_ics20_transfers_enabled: _,
                 },
             sct_params: SctParameters { epoch_duration },
-            shielded_pool_params:
-                ShieldedPoolParameters {
-                    fixed_fmd_params: _,
-                },
+            shielded_pool_params: ShieldedPoolParameters { fmd_meta_params: _ },
             stake_params:
                 StakeParameters {
                     active_validator_limit,
@@ -188,10 +185,7 @@ impl AppParameters {
                     outbound_ics20_transfers_enabled,
                 },
             sct_params: SctParameters { epoch_duration },
-            shielded_pool_params:
-                ShieldedPoolParameters {
-                    fixed_fmd_params: _,
-                },
+            shielded_pool_params: ShieldedPoolParameters { fmd_meta_params: _ },
             stake_params:
                 StakeParameters {
                     active_validator_limit,

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
@@ -2,6 +2,7 @@ use {
     self::common::{BuilderExt, TestNodeExt, ValidatorDataReadExt},
     anyhow::anyhow,
     cnidarium::TempStorage,
+    decaf377_fmd::Precision,
     decaf377_rdsa::{SigningKey, SpendAuth, VerificationKey},
     penumbra_app::{
         genesis::{self, AppState},
@@ -157,7 +158,7 @@ async fn app_can_define_and_delegate_to_a_validator() -> anyhow::Result<()> {
                 ..Default::default()
             },
         };
-        plan.populate_detection_data(rand_core::OsRng, 0);
+        plan.populate_detection_data(rand_core::OsRng, Precision::default());
         plan
     };
     let tx = client.witness_auth_build(&plan).await?;
@@ -270,7 +271,7 @@ async fn app_can_define_and_delegate_to_a_validator() -> anyhow::Result<()> {
                 ..Default::default()
             },
         };
-        plan.populate_detection_data(rand_core::OsRng, 0);
+        plan.populate_detection_data(rand_core::OsRng, Precision::default());
         plan
     };
     let tx = client.witness_auth_build(&plan).await?;
@@ -432,7 +433,7 @@ async fn app_can_define_and_delegate_to_a_validator() -> anyhow::Result<()> {
                 ..Default::default()
             },
         };
-        plan.populate_detection_data(rand_core::OsRng, 0);
+        plan.populate_detection_data(rand_core::OsRng, Precision::default());
         plan
     };
     let tx = client.witness_auth_build(&plan).await?;

diff --git a/crates/core/app/tests/app_can_deposit_into_community_pool.rs b/crates/core/app/tests/app_can_deposit_into_community_pool.rs
@@ -81,7 +81,7 @@ async fn app_can_deposit_into_community_pool() -> anyhow::Result<()> {
             },
         }
     };
-    plan.populate_detection_data(OsRng, 0);
+    plan.populate_detection_data(OsRng, Default::default());
     let tx = client.witness_auth_build(&plan).await?;
 
     // Execute the transaction, applying it to the chain state.

diff --git a/crates/core/app/tests/app_can_disable_community_pool_spends.rs b/crates/core/app/tests/app_can_disable_community_pool_spends.rs
@@ -184,7 +184,7 @@ async fn app_can_disable_community_pool_spends() -> anyhow::Result<()> {
             },
         }
     };
-    plan.populate_detection_data(OsRng, 0);
+    plan.populate_detection_data(OsRng, Default::default());
     let tx = client.witness_auth_build(&plan).await?;
 
     // Execute the transaction, applying it to the chain state.
@@ -253,7 +253,7 @@ async fn app_can_disable_community_pool_spends() -> anyhow::Result<()> {
             },
         }
     };
-    plan.populate_detection_data(OsRng, 0);
+    plan.populate_detection_data(OsRng, Default::default());
     let tx = client.witness_auth_build(&plan).await?;
 
     // Execute the transaction, applying it to the chain state.
@@ -288,7 +288,7 @@ async fn app_can_disable_community_pool_spends() -> anyhow::Result<()> {
             },
         }
     };
-    plan.populate_detection_data(OsRng, 0);
+    plan.populate_detection_data(OsRng, Default::default());
     let tx = client.witness_auth_build(&plan).await?;
 
     // Execute the transaction, applying it to the chain state.

diff --git a/crates/core/app/tests/app_can_propose_community_pool_spends.rs b/crates/core/app/tests/app_can_propose_community_pool_spends.rs
@@ -178,7 +178,7 @@ async fn app_can_propose_community_pool_spends() -> anyhow::Result<()> {
             },
         }
     };
-    plan.populate_detection_data(OsRng, 0);
+    plan.populate_detection_data(OsRng, Default::default());
     let tx = client.witness_auth_build(&plan).await?;
 
     // Execute the transaction, applying it to the chain state.
@@ -247,7 +247,7 @@ async fn app_can_propose_community_pool_spends() -> anyhow::Result<()> {
             },
         }
     };
-    plan.populate_detection_data(OsRng, 0);
+    plan.populate_detection_data(OsRng, Default::default());
     let tx = client.witness_auth_build(&plan).await?;
 
     // Execute the transaction, applying it to the chain state.
@@ -282,7 +282,7 @@ async fn app_can_propose_community_pool_spends() -> anyhow::Result<()> {
             },
         }
     };
-    plan.populate_detection_data(OsRng, 0);
+    plan.populate_detection_data(OsRng, Default::default());
     let tx = client.witness_auth_build(&plan).await?;
 
     // Execute the transaction, applying it to the chain state.

diff --git a/crates/core/app/tests/app_can_spend_notes_and_detect_outputs.rs b/crates/core/app/tests/app_can_spend_notes_and_detect_outputs.rs
@@ -2,6 +2,7 @@ use {
     self::common::BuilderExt,
     anyhow::anyhow,
     cnidarium::TempStorage,
+    decaf377_fmd::Precision,
     penumbra_app::{
         genesis::{self, AppState},
         server::consensus::Consensus,
@@ -87,7 +88,7 @@ async fn app_can_spend_notes_and_detect_outputs() -> anyhow::Result<()> {
             ..Default::default()
         },
     };
-    plan.populate_detection_data(OsRng, 0);
+    plan.populate_detection_data(OsRng, Precision::default());
 
     let tx = client.witness_auth_build(&plan).await?;