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

IdentityFee implementation used for fee calculation in messenger and transaction-payment pallets #2439

Open
mmostafas opened this issue Jan 23, 2024 · 2 comments
Labels
audit Audit results

Comments

@mmostafas
Copy link

Risk: Moderate severity

Issue

In pallets messenger and transaction-payment, the fee calculation for messages or transactions is delegated to the WeightToFee implementation which can be configured via runtime's configuration. This trait exposes the weight_to_fee() function which converts the weight (computation time) into a currency.

Currently, in Subspace IdentityFee is the only implementation used throughout the code base and specifically for messenger and transaction-payment pallets. Sine this implementation considers the exact weight for the fee which is does not take into account the current network’s economic conditions or congestion times.

impl<T> WeightToFee for IdentityFee<T>
  where
  T: BaseArithmetic + From<u32> + Copy + Unsigned,
{
  type Balance = T;

   fn weight_to_fee(weight: &Weight) -> Self::Balance {
    Self::Balance::saturated_from(weight.ref_time())
  }
}

Risk

This simplistic fee calculation can lead to weight fee underestimation and can be used by malicious network participants to spam the chain.

Mitigation

Implement the WeightToFee so that it dynamically adjusts the fee to reflect changes in the network’s requirements or economic conditions.

@vanhauser-thc vanhauser-thc added the audit Audit results label Jan 23, 2024
@vedhavyas
Copy link
Member

Messenger and Transaction payment is expected to use WeightToFee conversion provided through Runtime parameters. Unfortunately, we have not finalized the runtime parameters yet. We will point to the PR here once we push the final Runtime parameters

@NingLin-P
Copy link
Member

This reminds me about the evm tx fee calculation, which is gas_used * gas_price, the gas_price is adjusted according to the utilization of the last block, but because we set the evm max block weight to MAX, the utilization is always low thus the adjustment may not work:
https://github.com/subspace/frontier/blob/2b09a676cac8cca665c882a28bd13b2acef39395/frame/base-fee/src/lib.rs#L141-L146

cc @vedhavyas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit Audit results
Projects
None yet
Development

No branches or pull requests

4 participants