Skip to content

Commit

Permalink
fee: support v0 multi-asset fees (protocol side only)
Browse files Browse the repository at this point in the history
This commit implements a v0 of multi-asset fees (on the protocol side).
Multi-asset fees are important because they allows users who send tokens into
Penumbra over IBC to start using the chain immediately.

It does this by extending the `GasPrices` struct to include the asset ID of the
fee token the prices are for.  This allows natural handling of multi-asset fees
by just having multiple sets of `GasPrices`.

In the past, we've laid the groundwork for this (by allowing other assets in
the `Fee` structure, even if it defaults to the staking token) but haven't
prioritized it, due to concerns about some of the details of the
implementation, particularly:

A: How does the chain know that the prices of non-native fee tokens are correct?
B: What happens to the non-native fee tokens?
C: What are the privacy implications for users paying with non-native fee tokens?

This design punts on (A) by only allowing non-native fee tokens with prices
explicitly specified using governance. In the future, Penumbra could own DEX as
a price oracle, but this would be better to do after it's live and we have real
data about it, rather than worrying about edge cases now (e.g., if someone can
temporarily manipulate a price for their own trades, that's not a big deal in
itself, but hooking it to the fee mechanism used for resource usage could make
it one).

The approach to (C) is that these explicitly specified prices should be
substantially higher (10x) than those of the native token.  This means that
regardless of price variability, it should remain the case that the native fee
token is cheaper, so that the majority of users will use the same fee token and
their transactions will not be distinguishable.  On the other hand, letting it
be _possible_ to use non native fee tokens means that users who send IBC tokens
into Penumbra can access the protocol, and even if one of their first acts is
to swap some de minimis amount of the native token, they can do that within the
system.

This implementation currently does not properly handle (B), in that it silently
burns all fees, regardless of token.  It is not appropriate to burn any tokens
other than the native token, whose supply is managed by the chain. Instead, the
transaction-level fee check should move into `check_and_execute`, and call a
method on the fee component that records the fee amount in the object store. At
the end of the block, all non-native fees should be swapped into the native
token and then burned.  Emitted events should record:

- an `EventFee` for each transaction, with the transaction's gas cost, the gas
  prices used to compute the base fee, the base fee, and the actual fee;
- an `EventFeeBurn` at the end of the block, with the total amount of native
  fees burned as well as, for each alt fee token used to pay fees in that
  block, how much of the alt token was paid and how much of the native token it
  was traded for.

Client support is still mostly missing, though I have manually tested this on a
local devnet, by

1. Creating the devnet
2. Executing a parameter change proposal to set `dexParams.fixedAltGasPrices` to `"[{\"assetId\":{\"altBaseDenom\":\"ugm\"},\"blockSpacePrice\":\"80000\",\"compactBlockSpacePrice\":\"80000\",\"verificationPrice\":\"80000\",\"executionPrice\":\"80000\"}]"`
3. Hardcoding at the top of `TxCmd::exec` a change to the `gas_prices` to add the `gm` asset ID and multiplying all the prices by 10.

To properly support clients, we need to

- Record the `gas_prices` data for alt fee tokens that is newly included in the
  `CompactBlock` in the view server's storage (probably in a new `gas_prices`
  table that maps a fee token asset ID to the latest price);

- Use that data to populate the view server's gas prices RPC response (instead
  of the current `Vec::new` stub that reports any alt fee tokens as
  non-existing);

- Change the `pcli tx` command so that the existing `--fee-tier` and a new
  `--fee-token` parameters move to `pcli tx`, and apply to all subcommands
  (i.e., `pcli tx --fee-tier ... --fee-token gm send ...`) so that `TxCmd::exec`
  can pull the right `GasPrices` out of the RPC response.
  • Loading branch information
hdevalence committed May 6, 2024
1 parent 7854a5f commit a0baeda
Show file tree
Hide file tree
Showing 25 changed files with 346 additions and 115 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
81 changes: 50 additions & 31 deletions crates/core/app/src/action_handler/transaction/stateful.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,37 +72,6 @@ pub async fn expiry_height_is_valid<S: StateRead>(state: S, expiry_height: u64)
Ok(())
}

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());
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();

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
);

// We split the check to provide granular error messages.
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
);

Ok(())
}

pub async fn fmd_parameters_valid<S: StateRead>(state: S, transaction: &Transaction) -> Result<()> {
let previous_fmd_parameters = state
.get_previous_fmd_parameters()
Expand Down Expand Up @@ -171,3 +140,53 @@ pub async fn claimed_anchor_is_valid<S: StateRead>(
) -> Result<()> {
state.check_claimed_anchor(transaction.anchor).await
}

pub async fn fee_greater_than_base_fee<S: StateRead>(
state: S,
transaction: &Transaction,
) -> Result<()> {
// 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 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!(
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,
);

let transaction_base_fee = current_gas_prices.fee(&transaction.gas_cost());

ensure!(
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(())
}
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 a0baeda

Please sign in to comment.