Skip to content

Commit

Permalink
wip: multi-asset fees
Browse files Browse the repository at this point in the history
still to do:

- record data from CB in view server storage (needs new `gas_prices` table)
- add that data into view server rpc responses (instead of Vec::new)
- some kind of pcli flag that selects the fee token and picks the right GasPrices out of rpc rsp
  • Loading branch information
hdevalence committed May 5, 2024
1 parent 7896aae commit 7717b9e
Show file tree
Hide file tree
Showing 25 changed files with 331 additions and 100 deletions.
29 changes: 13 additions & 16 deletions crates/bin/pcli/src/command/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ use auction::AuctionCmd;
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::Fee;
use penumbra_governance::{proposal::ProposalToml, proposal_state::State as ProposalState, Vote};
use penumbra_keys::{keys::AddressIndex, Address};
use penumbra_num::Amount;
Expand Down Expand Up @@ -321,6 +320,9 @@ impl TxCmd {
}

pub async fn exec(&self, app: &mut App) -> Result<()> {
// TODO: use a command line flag to determine the fee token,
// and pull the appropriate GasPrices out of this rpc response,
// the rest should follow
let gas_prices = app
.view
.as_mut()
Expand Down Expand Up @@ -423,6 +425,7 @@ impl TxCmd {
} => {
let input = input.parse::<Value>()?;
let into = asset::REGISTRY.parse_unit(into.as_str()).base();
let fee_tier: FeeTier = (*fee_tier).into();

let fvk = app.config.full_viewing_key.clone();

Expand All @@ -434,20 +437,14 @@ impl TxCmd {
let mut planner = Planner::new(OsRng);
planner
.set_gas_prices(gas_prices.clone())
.set_fee_tier((*fee_tier).into());
// The swap claim requires a pre-paid fee, however gas costs might change in the meantime.
// This shouldn't be an issue, since the planner will account for the difference and add additional
// spends alongside the swap claim transaction as necessary.
//
// Regardless, we apply a gas adjustment factor of 2.0 up-front to reduce the likelihood of
// requiring an additional spend at the time of claim.
//
// Since the swap claim fee needs to be passed in to the planner to build the swap (it is
// part of the `SwapPlaintext`), we can't use the planner to estimate the fee and need to
// call the helper method directly.
let estimated_claim_fee = Fee::from_staking_token_amount(
Amount::from(2u32) * gas_prices.fee(&swap_claim_gas_cost()),
);
.set_fee_tier(fee_tier.into());

// We don't expect much of a drift in gas prices in a few blocks, and the fee tier
// adjustments should be enough to cover it.
let estimated_claim_fee = gas_prices
.fee(&swap_claim_gas_cost())
.apply_tier(fee_tier.into());

planner.swap(input, into.id(), estimated_claim_fee, claim_address)?;

let plan = planner
Expand Down Expand Up @@ -503,7 +500,7 @@ impl TxCmd {
let mut planner = Planner::new(OsRng);
planner
.set_gas_prices(gas_prices)
.set_fee_tier((*fee_tier).into());
.set_fee_tier(fee_tier.into());
let plan = planner
.swap_claim(SwapClaimPlan {
swap_plaintext,
Expand Down
22 changes: 1 addition & 21 deletions crates/bin/pcli/src/network.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
use anyhow::Context;
use decaf377_rdsa::{Signature, SpendAuth};
use futures::{FutureExt, TryStreamExt};
use penumbra_fee::GasPrices;
use penumbra_governance::ValidatorVoteBody;
use penumbra_proto::{
custody::v1::{AuthorizeValidatorDefinitionRequest, AuthorizeValidatorVoteRequest},
util::tendermint_proxy::v1::tendermint_proxy_service_client::TendermintProxyServiceClient,
view::v1::broadcast_transaction_response::Status as BroadcastStatus,
view::v1::GasPricesRequest,
DomainType,
};
use penumbra_stake::validator::Validator;
use penumbra_transaction::{gas::GasCost, txhash::TransactionId, Transaction, TransactionPlan};
use penumbra_transaction::{txhash::TransactionId, Transaction, TransactionPlan};
use penumbra_view::ViewClient;
use std::future::Future;
use tonic::transport::{Channel, ClientTlsConfig};
Expand All @@ -24,25 +22,7 @@ impl App {
&mut self,
plan: TransactionPlan,
) -> anyhow::Result<TransactionId> {
let gas_prices: GasPrices = self
.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 transaction = self.build_transaction(plan).await?;
let gas_cost = transaction.gas_cost();
let fee = gas_prices.fee(&gas_cost);
assert!(
transaction.transaction_parameters().fee.amount() >= fee,
"paid fee {} must be greater than minimum fee {}",
transaction.transaction_parameters().fee.amount(),
fee
);
self.submit_transaction(transaction).await
}

Expand Down
18 changes: 18 additions & 0 deletions crates/bin/pd/src/testnet/generate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
use crate::testnet::config::{get_testnet_dir, TestnetTendermintConfig, ValidatorKeys};
use anyhow::{Context, Result};
use penumbra_app::params::AppParameters;
use penumbra_asset::{asset, STAKING_TOKEN_ASSET_ID};
use penumbra_fee::genesis::Content as FeeContent;
use penumbra_governance::genesis::Content as GovernanceContent;
use penumbra_keys::{keys::SpendKey, Address};
Expand Down Expand Up @@ -223,7 +224,24 @@ impl TestnetConfig {
compact_block_space_price: gas_price_simple,
verification_price: gas_price_simple,
execution_price: gas_price_simple,
asset_id: *STAKING_TOKEN_ASSET_ID,
},
fixed_alt_gas_prices: vec![
penumbra_fee::GasPrices {
block_space_price: 10 * gas_price_simple,
compact_block_space_price: 10 * gas_price_simple,
verification_price: 10 * gas_price_simple,
execution_price: 10 * gas_price_simple,
asset_id: asset::REGISTRY.parse_unit("gm").id(),
},
penumbra_fee::GasPrices {
block_space_price: 10 * gas_price_simple,
compact_block_space_price: 10 * gas_price_simple,
verification_price: 10 * gas_price_simple,
execution_price: 10 * gas_price_simple,
asset_id: asset::REGISTRY.parse_unit("gn").id(),
},
],
},
},
governance_content: GovernanceContent {
Expand Down
51 changes: 35 additions & 16 deletions crates/core/app/src/action_handler/transaction/stateful.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,28 +83,47 @@ pub async fn fee_greater_than_base_fee<S: StateRead>(
state: S,
transaction: &Transaction,
) -> Result<()> {
let current_gas_prices = state
.get_gas_prices()
.await
.expect("gas prices must be present in state");

let transaction_base_price = current_gas_prices.fee(&transaction.gas_cost());
// Check whether the user is requesting to pay fees in the native token
// or in an alternative fee token.
let user_supplied_fee = transaction.transaction_body().transaction_parameters.fee;
let user_supplied_fee_amount = user_supplied_fee.amount();
let user_supplied_fee_asset_id = user_supplied_fee.asset_id();

let current_gas_prices =
if user_supplied_fee.asset_id() == *penumbra_asset::STAKING_TOKEN_ASSET_ID {
state
.get_gas_prices()
.await
.expect("gas prices must be present in state")
} else {
let alt_gas_prices = state
.get_alt_gas_prices()
.await
.expect("alt gas prices must be present in state");
alt_gas_prices
.into_iter()
.find(|prices| prices.asset_id == user_supplied_fee.asset_id())
.ok_or_else(|| {
anyhow::anyhow!(
"fee token {} not recognized by the chain",
user_supplied_fee.asset_id()
)
})?
};

// Double check that the gas price assets match.
ensure!(
user_supplied_fee_amount >= transaction_base_price,
"fee must be greater than or equal to the transaction base price (supplied: {}, base: {})",
user_supplied_fee_amount,
transaction_base_price
current_gas_prices.asset_id == user_supplied_fee.asset_id(),
"unexpected mismatch between fee and queried gas prices (expected: {}, found: {})",
user_supplied_fee.asset_id(),
current_gas_prices.asset_id,
);

// We split the check to provide granular error messages.
let transaction_base_fee = current_gas_prices.fee(&transaction.gas_cost());

ensure!(
user_supplied_fee_asset_id == *penumbra_asset::STAKING_TOKEN_ASSET_ID,
"fee must be paid in staking tokens (found: {})",
user_supplied_fee_asset_id
user_supplied_fee.amount() >= transaction_base_fee.amount(),
"fee must be greater than or equal to the transaction base price (supplied: {}, base: {})",
user_supplied_fee.amount(),
transaction_base_fee.amount(),
);

Ok(())
Expand Down
16 changes: 10 additions & 6 deletions crates/core/app/src/params/change.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,11 @@ impl AppParameters {
DistributionsParameters {
staking_issuance_per_block: _,
},
fee_params: FeeParameters {
fixed_gas_prices: _,
},
fee_params:
FeeParameters {
fixed_gas_prices: _,
fixed_alt_gas_prices: _,
},
funding_params: FundingParameters {},
governance_params:
GovernanceParameters {
Expand Down Expand Up @@ -165,9 +167,11 @@ impl AppParameters {
DistributionsParameters {
staking_issuance_per_block: _,
},
fee_params: FeeParameters {
fixed_gas_prices: _,
},
fee_params:
FeeParameters {
fixed_gas_prices: _,
fixed_alt_gas_prices: _,
},
funding_params: FundingParameters {},
governance_params:
GovernanceParameters {
Expand Down
19 changes: 10 additions & 9 deletions crates/core/asset/src/asset/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,14 +416,15 @@ pub static REGISTRY: Lazy<Registry> = Lazy::new(|| {
denom: format!("mvoted_on_{data}"),
},
])
}) as for<'r> fn(&'r str) -> _)
.add_asset(
"^auctionnft_(?P<data>[a-z_0-9]+_pauctid1[a-zA-HJ-NP-Z0-9]+)$",
&[ /* no display units - nft, unit 1 */ ],
(|data: &str| {
assert!(!data.is_empty());
denom_metadata::Inner::new(format!("auctionnft_{data}"), vec![])
}) as for<'r> fn(&'r str) -> _,
)
}) as for<'r> fn(&'r str) -> _
)
.add_asset(
"^auctionnft_(?P<data>[a-z_0-9]+_pauctid1[a-zA-HJ-NP-Z0-9]+)$",
&[ /* no display units - nft, unit 1 */ ],
(|data: &str| {
assert!(!data.is_empty());
denom_metadata::Inner::new(format!("auctionnft_{data}"), vec![])
}) as for<'r> fn(&'r str) -> _,
)
.build()
});
12 changes: 11 additions & 1 deletion crates/core/component/compact-block/src/compact_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ pub struct CompactBlock {
pub swap_outputs: BTreeMap<TradingPair, BatchSwapOutputData>,
/// Set if the app parameters have been updated. Notifies the client that it should re-sync from the fullnode RPC.
pub app_parameters_updated: bool,
/// Updated gas prices, if they have changed.
/// Updated gas prices for the native token, if they have changed.
pub gas_prices: Option<GasPrices>,
/// Updated gas prices for alternative fee tokens, if they have changed.
pub alt_gas_prices: Vec<GasPrices>,
// The epoch index
pub epoch_index: u64,
// **IMPORTANT NOTE FOR FUTURE HUMANS**: if you want to add new fields to the `CompactBlock`,
Expand All @@ -59,6 +61,7 @@ impl Default for CompactBlock {
swap_outputs: BTreeMap::new(),
app_parameters_updated: false,
gas_prices: None,
alt_gas_prices: Vec::new(),
epoch_index: 0,
}
}
Expand All @@ -73,6 +76,7 @@ impl CompactBlock {
|| self.proposal_started // need to process proposal start
|| self.app_parameters_updated // need to save latest app parameters
|| self.gas_prices.is_some() // need to save latest gas prices
|| !self.alt_gas_prices.is_empty() // need to save latest alt gas prices
}
}

Expand All @@ -98,6 +102,7 @@ impl From<CompactBlock> for pb::CompactBlock {
swap_outputs: cb.swap_outputs.into_values().map(Into::into).collect(),
app_parameters_updated: cb.app_parameters_updated,
gas_prices: cb.gas_prices.map(Into::into),
alt_gas_prices: cb.alt_gas_prices.into_iter().map(Into::into).collect(),
epoch_index: cb.epoch_index,
}
}
Expand Down Expand Up @@ -136,6 +141,11 @@ impl TryFrom<pb::CompactBlock> for CompactBlock {
proposal_started: value.proposal_started,
app_parameters_updated: value.app_parameters_updated,
gas_prices: value.gas_prices.map(TryInto::try_into).transpose()?,
alt_gas_prices: value
.alt_gas_prices
.into_iter()
.map(GasPrices::try_from)
.collect::<Result<Vec<GasPrices>>>()?,
epoch_index: value.epoch_index,
})
}
Expand Down
16 changes: 11 additions & 5 deletions crates/core/component/compact-block/src/component/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,19 @@ trait Inner: StateWrite {

// Check to see if the gas prices have changed, and include them in the compact block
// if they have (this is signaled by `penumbra_fee::StateWriteExt::put_gas_prices`):
let gas_prices = if self.gas_prices_changed() || height == 0 {
Some(
self.get_gas_prices()
let (gas_prices, alt_gas_prices) = if self.gas_prices_changed() || height == 0 {
(
Some(
self.get_gas_prices()
.await
.context("could not get gas prices")?,
),
self.get_alt_gas_prices()
.await
.context("could not get gas prices")?,
.context("could not get alt gas prices")?,
)
} else {
None
(None, Vec::new())
};

let fmd_parameters = if height == 0 {
Expand Down Expand Up @@ -133,6 +138,7 @@ trait Inner: StateWrite {
fmd_parameters,
app_parameters_updated,
gas_prices,
alt_gas_prices,
epoch_index,
};

Expand Down
2 changes: 0 additions & 2 deletions crates/core/component/fee/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ impl Component for Fee {
match app_state {
Some(genesis) => {
state.put_fee_params(genesis.fee_params.clone());
// Put the initial gas prices
state.put_gas_prices(genesis.fee_params.fixed_gas_prices);
}
None => { /* perform upgrade specific check */ }
}
Expand Down
1 change: 1 addition & 0 deletions crates/core/component/fee/src/component/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ impl QueryService for Server {

Ok(tonic::Response::new(pb::CurrentGasPricesResponse {
gas_prices: Some(gas_prices.into()),
alt_gas_prices: Vec::new(),
}))
}
}
Loading

0 comments on commit 7717b9e

Please sign in to comment.