New Asset System - Enable fee payment for CampaignAssets

[sea212]

What does it do?

• Implements Unbalanced for asset-router (this is necessary for CreditOf which is enforced by pallet-asset-tx-payment and within the Balanced trait)
• Uses Assets instead of u32 as a transaction fee asset type
• Implements fees for CampaignAssets.
  • Fees are derived as native_fee * fee_multiplier * campaign_asset_supply / ztg_supply to scale properly in a sense that more campaign asset tokens do not lead to more possible transactions and that possible transactions are always 100 times less than what would be possible with ZTG
  • A fee multiplier is capped to max(fee_multiplier, BASE). This prevents infinite transactions as fee_multiplier * campaign_asset_supply / ztg_supply would just infinitely grow smaller.
  • Fees are burned completely, as campaign assets have a limited lifetime.

What important points should reviewers know?

Is there something left for follow-up PRs?

• Prepare merge into main
• Future:
  • Remove lazy migration logic once all old outcome tokens were redeemed
  • Enable CustomAssets as base assets - requires more thinking because the admin account (which can be any user) can destroy the assets while they are in use within a market

What alternative implementations were considered?

• Instead of implementing Unbalanced and using Assets for TxCharger, implementing TxCharger for Currencies and CampaignAssets. Does not work due to strict relationship of Config types in pallet-asset-tx-payment.

Are there relevant PRs or issues?

• Continuation of New Asset System - Integrate into XCM #1287

References

[Chralt98]

I have to admit that you cleaned up a lot of technical debt from me. So I love it, thanks for the learning experience!

Why don't you let the admin of the campaign asset define what the fee factor should look like? Ok, then users could spam transactions using the campaign asset. But if admins can create campaign assets out of thin air, then transactions could also be made on our chain for basically no cost, if the admin of the campaign asset is malicious. Just in general then: How does AssetHub solve this? I mean if users can create transactions, I assume fees are still paid in KSM or DOT then in order to prevent spamming transactions on-chain.

Fees are derived as native_fee * fee_multiplier * campaign_asset_supply / ztg_supply to scale properly in a sense that more campaign asset tokens do not lead to more possible transactions and that possible transactions are always 100 times less than what would be possible with ZTG

possible transactions are always 100 times less than what would be possible with ZTG

But the price of the underlying campaign asset is not known. So why this notion of having a relation to ZTG? Are the fees burned in ZTG or in campaign asset?

A fee multiplier is capped to max(fee_multiplier, BASE). This prevents infinite transactions as fee_multiplier * campaign_asset_supply / ztg_supply would just infinitely grow smaller.

I think with the current implementation of get_fee_factor_campaign_asset the fee factor can infinitely grow larger. campaign_asset_supply can be very large since you ensure ztg_supply / campaign_asset_supply < 100, which means if campaign_asset_supply grows to u128::MAX you would basically get u128::MAX / ztg_supply for the maximum fee factor, or am I wrong?

[Chralt98]

You might want to add a new error variant for identifying on the outer caller side, that the error occured here. You might call it FeeFactorArith.

[sea212]

FeeConversionArith perfectly describes what is going on from my perspective. What you are suggesting here implies that we would create an unique error for every place in the code where an error is potentially thrown, even if another error already perfectly describes what is happening. Not going down that path 😁

[Chralt98]

The reason I suggested this was that we once used the same error multiple times in the same function and therefore couldn't directly distinguish what has failed. In this case there are three possible places. But I think it's okay, since we can also distinguish where it was thrown ourselves to improve maintenance of errors on our side.

[Chralt98]

Shall FixedMulDiv be used here?

zeitgeist/primitives/src/math/fixed.rs

Lines 93 to 107 in 4d3519c

	/// Performs fixed point multiplication and division, calculating `self * multiplier / divisor`.
	pub trait FixedMulDiv
	where
	Self: Sized,
	{
	/// Calculates the fixed point product `self * multiplier / divisor` and rounds to the nearest
	/// representable fixed point number.
	fn bmul_bdiv(&self, multiplier: Self, divisor: Self) -> Result<Self, DispatchError>;
	/// Calculates the fixed point product `self * multiplier / divisor` and rounds down.
	fn bmul_bdiv_floor(&self, multiplier: Self, divisor: Self) -> Result<Self, DispatchError>;
	/// Calculates the fixed point product `self * multiplier / divisor` and rounds up.
	fn bmul_bdiv_ceil(&self, multiplier: Self, divisor: Self) -> Result<Self, DispatchError>;
	}

[Chralt98]

Assume the campaign_asset_supply is huge, then saturating_mul would lead to the biggest u128. Is the maximum campaign asset supply capped too?

[sea212]

The maximum campaign asset supply is not capped. I don't see any obvious issues with a supply of u128::MAX, as math seems to work anyways, fees are just reduced by up to CampaignAssetFeeMultiplier. If malicious behavior is detected council and technical committee can nuke the asset. Generally, it is probably advisable to assign admin accounts to either Zeitgeist or trusted accounts.
In regards to FixedMulDiv, I think bmul_bdiv is the wrong function here, because CampaignAssetFeeMultiplier has base 10 instead of BASE which bmul_bdiv requires.

[maltekliemann]

One way to improve this, if we're interested in that, is to check for overflow when multiplying and divide first if the check is positive. Not sure if we care about that though.

[sea212]

Why don't you let the admin of the campaign asset define what the fee factor should look like?

For the sake of simplicity (not having to maintain extra metadata), I decided to go with the current approach.

But the price of the underlying campaign asset is not known. So why this notion of having a relation to ZTG? Are the fees burned in ZTG or in campaign asset?

The fees are burned in campaign asset, but on the basis on the native fee, i.e. how much ZTG would have been paid. Essentially we need some way to determine how much campaign assets are burned, and I thought the given relationship is simple and effective because it scales with the total supply (users could have millions of campaign tokens, but pay thousands of fees per TX).

I think with the current implementation of get_fee_factor_campaign_asset the fee factor can infinitely grow larger. campaign_asset_supply can be very large since you ensure ztg_supply / campaign_asset_supply < 100, which means if campaign_asset_supply grows to u128::MAX you would basically get u128::MAX / ztg_supply for the maximum fee factor, or am I wrong?

You correctly defined the max value. Admin access to campaign assets should be given only to trusted accounts and limited by a specific amount of time. Council and TC might have to resolve during abusal.