From 21ffe9f7dd9ec18e69c7f13b997256b04b3c54e2 Mon Sep 17 00:00:00 2001 From: Conor Schaefer Date: Tue, 4 Jun 2024 09:13:19 -0700 Subject: [PATCH] feat(fee): add FromStr, Display impls for FeeTier We recently enabled fees on Testnet 77 (#4306), and in the process broke a few things, such as Galileo. While working on adding FeeTier support to Galileo, Galileo's clap setup wanted Display and FromStr. Added those to satisfy the build, and then circled back to pcli to reuse the same impls, which allowed us to snip out some duplicative code. --- crates/bin/pcli/src/command/tx.rs | 53 ++++--------------- .../bin/pcli/src/command/tx/auction/dutch.rs | 8 +-- .../pcli/src/command/tx/liquidity_position.rs | 12 ++--- crates/bin/pcli/src/command/tx/proposal.rs | 6 +-- crates/core/component/fee/src/fee.rs | 25 +++++++++ 5 files changed, 47 insertions(+), 57 deletions(-) diff --git a/crates/bin/pcli/src/command/tx.rs b/crates/bin/pcli/src/command/tx.rs index 245cfd4af7..7657114771 100644 --- a/crates/bin/pcli/src/command/tx.rs +++ b/crates/bin/pcli/src/command/tx.rs @@ -28,6 +28,7 @@ use regex::Regex; use liquidity_position::PositionCmd; use penumbra_asset::{asset, asset::Metadata, Value, STAKING_TOKEN_ASSET_ID}; use penumbra_dex::{lp::position, swap_claim::SwapClaimPlan}; +use penumbra_fee::FeeTier; use penumbra_governance::{proposal::ProposalToml, proposal_state::State as ProposalState, Vote}; use penumbra_keys::{keys::AddressIndex, Address}; use penumbra_num::Amount; @@ -88,7 +89,7 @@ pub enum TxCmd { #[clap(long)] memo: 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, }, /// Deposit stake into a validator's delegation pool. @@ -103,7 +104,7 @@ pub enum TxCmd { #[clap(long, default_value = "0", display_order = 300)] source: u32, /// 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, }, /// Withdraw stake from a validator's delegation pool. @@ -115,14 +116,14 @@ pub enum TxCmd { #[clap(long, default_value = "0", display_order = 300)] source: u32, /// 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, }, /// Claim any undelegations that have finished unbonding. #[clap(display_order = 200)] UndelegateClaim { /// 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, }, /// Swap tokens of one denomination for another using the DEX. @@ -144,7 +145,7 @@ pub enum TxCmd { #[clap(long, default_value = "0", display_order = 300)] source: u32, /// 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, }, /// Vote on a governance proposal in your role as a delegator (see also: `pcli validator vote`). @@ -157,7 +158,7 @@ pub enum TxCmd { #[clap(subcommand)] vote: VoteCmd, /// 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, }, /// Submit or withdraw a governance proposal. @@ -172,7 +173,7 @@ pub enum TxCmd { #[clap(long, default_value = "0", display_order = 300)] source: u32, /// 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, }, /// Manage liquidity positions. @@ -223,47 +224,11 @@ pub enum TxCmd { #[clap(long, default_value = "0", display_order = 200)] source: u32, /// 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, }, } -// A fee tier enum suitable for use with clap. -#[derive(Copy, Clone, clap::ValueEnum, Debug)] -pub enum FeeTier { - Low, - Medium, - High, -} - -impl Default for FeeTier { - fn default() -> Self { - Self::Low - } -} - -// Convert from the internal fee tier enum to the clap-compatible enum. -impl From for FeeTier { - fn from(tier: penumbra_fee::FeeTier) -> Self { - match tier { - penumbra_fee::FeeTier::Low => Self::Low, - penumbra_fee::FeeTier::Medium => Self::Medium, - penumbra_fee::FeeTier::High => Self::High, - } - } -} - -// Convert from the the clap-compatible fee tier enum to the internal fee tier enum. -impl From for penumbra_fee::FeeTier { - fn from(tier: FeeTier) -> Self { - match tier { - FeeTier::Low => Self::Low, - FeeTier::Medium => Self::Medium, - FeeTier::High => Self::High, - } - } -} - /// Vote on a governance proposal. #[derive(Debug, Clone, Copy, clap::Subcommand)] pub enum VoteCmd { diff --git a/crates/bin/pcli/src/command/tx/auction/dutch.rs b/crates/bin/pcli/src/command/tx/auction/dutch.rs index 12c560a7ce..276d7c90c9 100644 --- a/crates/bin/pcli/src/command/tx/auction/dutch.rs +++ b/crates/bin/pcli/src/command/tx/auction/dutch.rs @@ -50,7 +50,7 @@ pub enum DutchCmd { #[clap(long, display_order = 700)] yes: bool, /// The selected fee tier to multiply the fee amount by. - #[clap(short, long, value_enum, default_value_t, display_order = 1000)] + #[clap(short, long, default_value_t, display_order = 1000)] fee_tier: FeeTier, #[clap(long, hide = true)] // Use to produce a debug file for numerical analysis. @@ -92,7 +92,7 @@ pub enum DutchCmd { #[clap(long, display_order = 800)] step_count: u64, /// The selected fee tier to multiply the fee amount by. - #[clap(short, long, value_enum, default_value_t, display_order = 1000)] + #[clap(short, long, default_value_t, display_order = 1000)] fee_tier: FeeTier, }, /// Terminate a Dutch auction. @@ -108,7 +108,7 @@ pub enum DutchCmd { #[clap(display_order = 200)] auction_id: Option, /// The selected fee tier to multiply the fee amount by. - #[clap(short, long, value_enum, default_value_t, display_order = 300)] + #[clap(short, long, default_value_t, display_order = 300)] fee_tier: FeeTier, }, /// Withdraw a Dutch auction, and claim its reserves. @@ -124,7 +124,7 @@ pub enum DutchCmd { #[clap(display_order = 200)] auction_id: Option, /// The selected fee tier to multiply the fee amount by. - #[clap(short, long, value_enum, default_value_t, display_order = 600)] + #[clap(short, long, default_value_t, display_order = 600)] fee_tier: FeeTier, }, } diff --git a/crates/bin/pcli/src/command/tx/liquidity_position.rs b/crates/bin/pcli/src/command/tx/liquidity_position.rs index 9cf2d2e4be..ef2daa52fc 100644 --- a/crates/bin/pcli/src/command/tx/liquidity_position.rs +++ b/crates/bin/pcli/src/command/tx/liquidity_position.rs @@ -26,7 +26,7 @@ pub enum PositionCmd { #[clap(long)] trading_pair: 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, }, /// Debits opened position NFTs and credits closed position NFTs. @@ -37,7 +37,7 @@ pub enum PositionCmd { /// The list of [`position::Id`] of the positions to close. position_ids: Vec, /// 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, }, /// Debits all closed position NFTs associated with a specific account and credits withdrawn position NFTs and the final reserves. @@ -49,7 +49,7 @@ pub enum PositionCmd { #[clap(long)] trading_pair: 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, }, /// Debits closed position NFTs and credits withdrawn position NFTs and the final reserves. @@ -60,7 +60,7 @@ pub enum PositionCmd { /// The list of [`position::Id`] of the positions to withdraw. position_ids: Vec, /// 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, }, @@ -102,7 +102,7 @@ pub enum OrderCmd { #[clap(long)] auto_close: bool, /// 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, }, Sell { @@ -119,7 +119,7 @@ pub enum OrderCmd { #[clap(long)] auto_close: bool, /// 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, }, } diff --git a/crates/bin/pcli/src/command/tx/proposal.rs b/crates/bin/pcli/src/command/tx/proposal.rs index 7a60172aa9..0a32d0c67e 100644 --- a/crates/bin/pcli/src/command/tx/proposal.rs +++ b/crates/bin/pcli/src/command/tx/proposal.rs @@ -30,7 +30,7 @@ pub enum ProposalCmd { #[clap(long, default_value = "")] deposit_amount: String, /// 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, }, /// Withdraw a governance proposal that you previously submitted. @@ -45,7 +45,7 @@ pub enum ProposalCmd { #[clap(long, default_value = "0")] source: u32, /// 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, }, /// Claim a governance proposal deposit for a proposal you submitted that has finished voting. @@ -60,7 +60,7 @@ pub enum ProposalCmd { #[clap(long, default_value = "0")] source: u32, /// 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, }, } diff --git a/crates/core/component/fee/src/fee.rs b/crates/core/component/fee/src/fee.rs index fb2f7f1152..72cac599c6 100644 --- a/crates/core/component/fee/src/fee.rs +++ b/crates/core/component/fee/src/fee.rs @@ -1,5 +1,7 @@ use anyhow::Context; use penumbra_proto::{penumbra::core::component::fee::v1 as pb, DomainType}; +use std::fmt; +use std::str::FromStr; use decaf377::Fr; use penumbra_asset::{asset, balance, Balance, Value, STAKING_TOKEN_ASSET_ID}; @@ -146,6 +148,29 @@ impl Default for FeeTier { } } +impl fmt::Display for FeeTier { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let s = match self { + FeeTier::Low => "low".to_owned(), + FeeTier::Medium => "medium".to_owned(), + FeeTier::High => "high".to_owned(), + }; + write!(f, "{}", s) + } +} + +impl FromStr for FeeTier { + type Err = anyhow::Error; + fn from_str(s: &str) -> Result { + match s { + "low" => Ok(FeeTier::Low), + "medium" => Ok(FeeTier::Medium), + "high" => Ok(FeeTier::High), + _ => anyhow::bail!(format!("cannot parse '{}' as FeeTier", s)), + } + } +} + impl DomainType for FeeTier { type Proto = pb::FeeTier; }