It looks like @JoshOrndorff signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

I do recall our discussion, but can't quite see what the PR is trying to achieve here. Can you elaborate a bit more? WeightToFee's should internal implementation seems to be the same and it is only a matter of naming?

I think the logic of a simple coefficient multiplier is simple enough to be handled by just one struct, rather than one encapsulating the number and one encapsulating the multiplication. What do you think?

Although, maybe @thiolliere would be happier like this? you commented that you think having both Get<> and Convert<> implemented for one struct is confusing and you would rather have them separated. If you think this is better, I am happy to proceed.

I think the logic of a simple coefficient multiplier is simple enough to be handled by just one struct, rather than one encapsulating the number and one encapsulating the multiplication. What do you think?

Here's what the two types are representing

• TransactionWeightFree -- A wrapper type to encapsulate the constant coefficient. We do this for constants all over.
• LinearWeight -- A type to do linear conversion with some scale factor. Because we can't use an instance of LinearWeight, we must specify that coefficient as a type parameter.

Perhaps this could exemplified more simply with a more general example. Consider this alternative weight to fee converter.

/// Convert from weight to balance via a quadratic curve
/// The type parameters encapsulate the coefficients
pub struct QuadraticWeight<C0, C1, C2>(C0, C1, C2);

impl<C0, C1, C2> Convert<Weight, Balance> for QuadraticWeight<C0, C1, C2>
 	where C0: Get<Balance>, C1: Get<Balance>, C2: Get<Balance> {

	fn convert(w: Weight) -> Balance {
		let c0 = C0::get();
		let c1 = C1::get();
		let c2 = C2::get();
		let w = Balance::from(w);

		//TODO use saturating arithmetic
		c0 + c1 * w + c2 * w * w
	}
}

Then to actually use the quadratic converter in your runtime you would need to do something like this

parameter_types! {
	pub const TransactionBaseFee: Balance = 1 * CENTS;
	pub const TransactionByteFee: Balance = 10 * MILLICENTS;

	// Now there are even more structs because there are even more constants
	pub const WeightFeeConstant: Balance = 1_000;
	pub const WeightFeeLinear: Balance = 100;
	pub const WeightFeeQuadratic : Balance = 10;

	// for a sane configuration, this should always be less than `AvailableBlockRatio`.
	pub const TargetedFeeAdjustment: Perbill = Perbill::from_percent(25);
}

The above code separate the ultimate weight to fee calculation into four structs. One actual struct that represents quadratic calculation, and three that just wrap the constants. It wouldn't make sense to implement the entire conversion on any one of the three constant-wrapping types here.

I still like this idea, but I also had another idea that you may prefer.

New idea: Turn an instance of

parameter_types!{
  // --snip--
  pub const WeightToFee: LinearWeightFee = LinearWeightFee{ coefficient: 10_000 };
}

I've prototyped it 420e162 but I can't get it to compile. I'll wait for feedback before doing anything else.

Separating the coefficient into a separate struct only makes sense in an example like QuadraticWeight where you have multiple of them, because then you cannot distinguish between Get<C0>, Get<C1> etc. unless if you create a custom trait;

in node_runtime there is only one and I still think simplicity of not having multiple config structs laying around is more important. One struct can be Convert<_, _> + Get<_> and that works perfectly fine. The case of a user wanting to implement quadratic is more of an example, and not enough of a reason to be added to both substrate node and polakdot.

Even if not in node-runtime, we should put this example in the docs or maybe even in node-template.

Nonetheless, I will add some naming suggestions here to make it better, then we can merge.

I still like this idea, but I also had another idea that you may prefer.

New idea: Turn an instance of

parameter_types!{
  // --snip--
  pub const WeightToFee: LinearWeightFee = LinearWeightFee{ coefficient: 10_000 };
}

I've prototyped it 420e162 but I can't get it to compile. I'll wait for feedback before doing anything else.

I don't think you should pursue this. Using self in Convert will not work as it is called as a type function like T::AssociatedType::function(), hence will never have self.

Yes I think I like this, the parameter type is only used in the same file, and there is no like hidden implementation on top of it in another file.

Also maybe LinearWeight structure could be moved to srml/transaction-payment as an optionnal useful structure to use.

I've included the renames @kianenigma requested. I'm ready to merge this back into #3823 and if necessary finish any last pieces there.