From cf2ab8c261dbcd2db75cfa3a660e8311c72072de Mon Sep 17 00:00:00 2001 From: Tal Derei Date: Tue, 4 Jun 2024 12:15:00 -0700 Subject: [PATCH 1/3] scratch --- crates/bin/pcli/src/command/validator.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/crates/bin/pcli/src/command/validator.rs b/crates/bin/pcli/src/command/validator.rs index 7a1a96178f..6e92a2ccdf 100644 --- a/crates/bin/pcli/src/command/validator.rs +++ b/crates/bin/pcli/src/command/validator.rs @@ -13,7 +13,7 @@ use serde_json::Value; use penumbra_governance::{ ValidatorVote, ValidatorVoteBody, ValidatorVoteReason, Vote, MAX_VALIDATOR_VOTE_REASON_LENGTH, }; -use penumbra_proto::DomainType; +use penumbra_proto::{view::v1::GasPricesRequest, DomainType}; use penumbra_stake::{ validator, validator::{Validator, ValidatorToml}, @@ -22,6 +22,8 @@ use penumbra_stake::{ use crate::App; +use super::tx::FeeTier; + #[derive(Debug, clap::Subcommand)] pub enum ValidatorCmd { /// Display the validator identity key derived from this wallet's spend seed. @@ -107,6 +109,9 @@ pub enum DefinitionCmd { /// definition may be generated using the `pcli validator definition sign` command. #[clap(long)] signature: Option, + /// The selected fee tier to multiply the fee amount by. + #[clap(short, long, value_enum, default_value_t)] + fee_tier: FeeTier, }, /// Sign a validator definition offline for submission elsewhere. Sign { @@ -160,6 +165,17 @@ impl ValidatorCmd { pub async fn exec(&self, app: &mut App) -> Result<()> { let fvk = app.config.full_viewing_key.clone(); + let gas_prices = app + .view + .as_mut() + .context("view service must be initialized")? + .gas_prices(GasPricesRequest {}) + .await? + .into_inner() + .gas_prices + .expect("gas prices must be available") + .try_into()?; + match self { ValidatorCmd::Identity { base64 } => { let ik = IdentityKey(fvk.spend_verification_key().clone().into()); @@ -229,6 +245,7 @@ impl ValidatorCmd { file, source, signature, + fee_tier, }) => { let new_validator = read_validator_toml(file)?; @@ -259,6 +276,8 @@ impl ValidatorCmd { let plan = Planner::new(OsRng) .validator_definition(vd) + .set_gas_prices(gas_prices) + .set_fee_tier((*fee_tier).into()) .plan(app.view(), source.into()) .await?; From 8f29a835063234d1b6b05a122cd06bee2ee021e2 Mon Sep 17 00:00:00 2001 From: Chris Czub Date: Tue, 4 Jun 2024 17:35:00 -0400 Subject: [PATCH 2/3] Switch gas prices to an Option on the planner and error if not set --- crates/bin/pcli/src/command/validator.rs | 39 +++++++++++++------ .../view_server_can_be_served_on_localhost.rs | 11 +++++- crates/view/src/planner.rs | 12 ++++-- crates/wallet/src/plan.rs | 3 ++ 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/crates/bin/pcli/src/command/validator.rs b/crates/bin/pcli/src/command/validator.rs index 6e92a2ccdf..4cf64e335d 100644 --- a/crates/bin/pcli/src/command/validator.rs +++ b/crates/bin/pcli/src/command/validator.rs @@ -72,6 +72,9 @@ pub enum VoteCmd { /// key, i.e. when using a separate governance key on another wallet. #[clap(long, global = true, display_order = 600)] validator: Option, + /// The selected fee tier to multiply the fee amount by. + #[clap(short, long, value_enum, default_value_t)] + fee_tier: FeeTier, }, /// Sign a vote on a proposal in your capacity as a validator, for submission elsewhere. Sign { @@ -165,17 +168,6 @@ impl ValidatorCmd { pub async fn exec(&self, app: &mut App) -> Result<()> { let fvk = app.config.full_viewing_key.clone(); - let gas_prices = app - .view - .as_mut() - .context("view service must be initialized")? - .gas_prices(GasPricesRequest {}) - .await? - .into_inner() - .gas_prices - .expect("gas prices must be available") - .try_into()?; - match self { ValidatorCmd::Identity { base64 } => { let ik = IdentityKey(fvk.spend_verification_key().clone().into()); @@ -247,6 +239,17 @@ impl ValidatorCmd { signature, fee_tier, }) => { + let gas_prices = app + .view + .as_mut() + .context("view service must be initialized")? + .gas_prices(GasPricesRequest {}) + .await? + .into_inner() + .gas_prices + .expect("gas prices must be available") + .try_into()?; + let new_validator = read_validator_toml(file)?; // Sign the validator definition with the wallet's spend key, or instead attach the @@ -340,7 +343,19 @@ impl ValidatorCmd { reason, signature, validator, + fee_tier, }) => { + let gas_prices = app + .view + .as_mut() + .context("view service must be initialized")? + .gas_prices(GasPricesRequest {}) + .await? + .into_inner() + .gas_prices + .expect("gas prices must be available") + .try_into()?; + let identity_key = validator .unwrap_or_else(|| IdentityKey(fvk.spend_verification_key().clone().into())); let governance_key = app.config.governance_key(); @@ -384,6 +399,8 @@ impl ValidatorCmd { // Construct a new transaction and include the validator definition. let plan = Planner::new(OsRng) + .set_gas_prices(gas_prices) + .set_fee_tier((*fee_tier).into()) .validator_vote(vote) .plan(app.view(), source.into()) .await?; diff --git a/crates/core/app/tests/view_server_can_be_served_on_localhost.rs b/crates/core/app/tests/view_server_can_be_served_on_localhost.rs index aa09892030..de5d40f825 100644 --- a/crates/core/app/tests/view_server_can_be_served_on_localhost.rs +++ b/crates/core/app/tests/view_server_can_be_served_on_localhost.rs @@ -13,7 +13,7 @@ use { penumbra_proto::{ view::v1::{ view_service_client::ViewServiceClient, view_service_server::ViewServiceServer, - StatusRequest, StatusResponse, + GasPricesRequest, StatusRequest, StatusResponse, }, DomainType, }, @@ -133,8 +133,17 @@ async fn view_server_can_be_served_on_localhost() -> anyhow::Result<()> { // Create a plan spending that note, using the `Planner`. let plan = { + let gas_prices = view_client + .gas_prices(GasPricesRequest {}) + .await? + .into_inner() + .gas_prices + .expect("gas prices must be available") + .try_into()?; + let mut planner = Planner::new(rand_core::OsRng); planner + .set_gas_prices(gas_prices) .spend(note.to_owned(), position) .output(note.value(), test_keys::ADDRESS_1.deref().clone()) .plan(&mut view_client, AddressIndex::default()) diff --git a/crates/view/src/planner.rs b/crates/view/src/planner.rs index 336f37f17c..059b3fc01c 100644 --- a/crates/view/src/planner.rs +++ b/crates/view/src/planner.rs @@ -56,7 +56,7 @@ pub struct Planner { /// The fee tier to apply to this transaction. fee_tier: FeeTier, /// The set of prices used for gas estimation. - gas_prices: GasPrices, + gas_prices: Option, /// The transaction parameters to use for the transaction. transaction_parameters: TransactionParameters, /// A user-specified change address, if any. @@ -105,7 +105,7 @@ impl Planner { /// Set the current gas prices for fee prediction. #[instrument(skip(self))] pub fn set_gas_prices(&mut self, gas_prices: GasPrices) -> &mut Self { - self.gas_prices = gas_prices; + self.gas_prices = Some(gas_prices); self } @@ -544,7 +544,9 @@ impl Planner { // Compute an initial fee estimate based on the actions we have so far. self.action_list.refresh_fee_and_change( &mut self.rng, - &self.gas_prices, + &self + .gas_prices + .context("planner instances must call set_gas_prices prior to planning")?, &self.fee_tier, &change_address, ); @@ -597,7 +599,9 @@ impl Planner { // Refresh the fee estimate and change outputs. self.action_list.refresh_fee_and_change( &mut self.rng, - &self.gas_prices, + &self + .gas_prices + .context("planner instances must call set_gas_prices prior to planning")?, &self.fee_tier, &change_address, ); diff --git a/crates/wallet/src/plan.rs b/crates/wallet/src/plan.rs index ff043a0694..f2ff8fad7e 100644 --- a/crates/wallet/src/plan.rs +++ b/crates/wallet/src/plan.rs @@ -91,6 +91,8 @@ where { const SWEEP_COUNT: usize = 8; + let gas_prices = view.gas_prices().await?; + let all_notes = view .notes(NotesRequest { ..Default::default() @@ -123,6 +125,7 @@ where // chunks, ignoring the biggest notes in the remainder. for group in records.chunks_exact(SWEEP_COUNT) { let mut planner = Planner::new(&mut rng); + planner.set_gas_prices(gas_prices); for record in group { planner.spend(record.note.clone(), record.position); From 406cf56280f8c1f1abfdaa47a046eb39e8e6ba15 Mon Sep 17 00:00:00 2001 From: Chris Czub Date: Tue, 4 Jun 2024 17:47:50 -0400 Subject: [PATCH 3/3] PR fixes --- crates/bin/pcli/src/command/validator.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bin/pcli/src/command/validator.rs b/crates/bin/pcli/src/command/validator.rs index 4cf64e335d..fd43d203ce 100644 --- a/crates/bin/pcli/src/command/validator.rs +++ b/crates/bin/pcli/src/command/validator.rs @@ -22,7 +22,7 @@ use penumbra_stake::{ use crate::App; -use super::tx::FeeTier; +use penumbra_fee::FeeTier; #[derive(Debug, clap::Subcommand)] pub enum ValidatorCmd { @@ -73,7 +73,7 @@ pub enum VoteCmd { #[clap(long, global = true, display_order = 600)] validator: Option, /// The selected fee tier to multiply the fee amount by. - #[clap(short, long, value_enum, default_value_t)] + #[clap(short, long, default_value_t)] fee_tier: FeeTier, }, /// Sign a vote on a proposal in your capacity as a validator, for submission elsewhere. @@ -113,7 +113,7 @@ pub enum DefinitionCmd { #[clap(long)] signature: Option, /// The selected fee tier to multiply the fee amount by. - #[clap(short, long, value_enum, default_value_t)] + #[clap(short, long, default_value_t)] fee_tier: FeeTier, }, /// Sign a validator definition offline for submission elsewhere.