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

FMD Parameter Selection Breaking Changes #4137

Merged
merged 15 commits into from
May 21, 2024
Merged
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/bin/pd/src/migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
35 changes: 28 additions & 7 deletions crates/bin/pd/src/migrate/testnet76.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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:
Expand All @@ -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?;

Expand All @@ -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"),
Copy link
Member

Choose a reason for hiding this comment

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

@cronokirby fwiw, the convention is that you write why you expect the Result/Option to be Ok/Some

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()
Expand All @@ -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));

Expand All @@ -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");

Expand Down
Binary file modified crates/cnidarium/src/gen/proto_descriptor.bin.no_lfs
Binary file not shown.
3 changes: 2 additions & 1 deletion crates/core/app/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -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 }
Expand Down
15 changes: 14 additions & 1 deletion crates/core/app/src/action_handler/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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(())
}
}
Expand Down Expand Up @@ -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,
Expand Down
25 changes: 15 additions & 10 deletions crates/core/app/src/action_handler/transaction/stateful.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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 {
Expand Down
10 changes: 2 additions & 8 deletions crates/core/app/src/params/change.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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?;
Expand Down Expand Up @@ -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?;
Expand Down Expand Up @@ -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?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use {
self::common::BuilderExt,
anyhow::anyhow,
cnidarium::TempStorage,
decaf377_fmd::Precision,
penumbra_app::{
genesis::{self, AppState},
server::consensus::Consensus,
Expand Down Expand Up @@ -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?;

Expand Down
Loading
Loading