More efficient identity and multiplier weight to fee

[nazar-pc]

#10785 left me with impression that ConstantMultiplier implementation is too heavy for what is needed in that particular case, so I decided to apply small optimization to it and to IdentityFee to avoid going polynomial route since it is not necessary for those cases.

[kianenigma]

I find this API a bit strange, that now polynomial still needs to be implemented but it is not really used, right? Nonetheless, looks like a reasonable micro optimization.

[nazar-pc]

Sorry, I don't get it. Where is calc being used in this PR?

calc() is a method of WeightToFeePolynomial trait. Method is called for every transaction in order to convert weight to fee. without these optimizations this would have been used instead:

substrate/frame/support/src/weights.rs

Lines 668 to 693 in b13c73f

	/// Calculates the fee from the passed `weight` according to the `polynomial`.
	///
	/// This should not be overriden in most circumstances. Calculation is done in the
	/// `Balance` type and never overflows. All evaluation is saturating.
	fn calc(weight: &Weight) -> Self::Balance {
	Self::polynomial()
	.iter()
	.fold(Self::Balance::saturated_from(0u32), |mut acc, args| {
	let w = Self::Balance::saturated_from(*weight).saturating_pow(args.degree.into());
	// The sum could get negative. Therefore we only sum with the accumulator.
	// The Perbill Mul implementation is non overflowing.
	let frac = args.coeff_frac * w;
	let integer = args.coeff_integer.saturating_mul(w);
	if args.negative {
	acc = acc.saturating_sub(frac);
	acc = acc.saturating_sub(integer);
	} else {
	acc = acc.saturating_add(frac);
	acc = acc.saturating_add(integer);
	}
	acc
	})
	}

I didn't benchmark it, but I also don't see the point of doing any of that if we can just do nothing or one multiplication. API is awkward, I agree, but I didn't design it 🤷

[shawntabrizi]

Sorry @nazar-pc I deleted my comment cause I didnt read any code, but now I did.

It is strange to me that there is a WeightToFeePolynomial trait at all.

Seems the trait should simply be WeightToFee with a single function fn calc that should be implemented, and this is what should be required in the pallet.

Then there should be a subtrait WeightToFeePolynomial, or just a struct, which implements WeightToFee but in the case of a polynomial.

Does that make sense? Then you can simply have Identity be the true identity without any polynomial bs.

[nazar-pc]

I like the idea of WeightToFee. I can make an alternative PR when I have time to do so if that'd be preferred.

[shawntabrizi]

I think you just need this:

/// A trait that describes a general weight to fee calculation.
pub trait WeightToFee {
	/// The type that is returned as result from polynomial evaluation.
	type Balance: BaseArithmetic + From<u32> + Copy + Unsigned;

	fn calc(weight: &Weight) -> Self::Balance;
}

pub struct WeightToFeePolynomialWrapper<W>(sp_std::marker::PhantomData::<W>);

impl<W: WeightToFeePolynomial> WeightToFee for WeightToFeePolynomialWrapper<W> {
	type Balance = W::Balance;

	fn calc(weight: &Weight) -> Self::Balance {
		W::calc(weight)
	}
}

Then just update the TransactionPayment Pallet

[shawntabrizi]

Seems in TransactionPayment Pallet, there is some exposed consts which assume that we use polynomial. @kianenigma is this needed?

[shawntabrizi]

anyway, my comments aren't blocking this optimization. so lets get this in

[shawntabrizi]

bot merge

[shawntabrizi]

@nazar-pc this one is a bit too baby to get a tip, but happy to send a tip if you do the changes mentioned in the conversation above :)

[kianenigma]

Seems in TransactionPayment Pallet, there is some exposed consts which assume that we use polynomial. @kianenigma is this needed?

They are only for UI, and I agree that we should break them and not assume the existence of a polynomial anymore.

All UIs should use the payment RPC for fee estimation.