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

Use fee_per_second from asset metadata in XCM config #393

Closed
ebma opened this issue Jan 10, 2024 · 7 comments · Fixed by #410 or #432
Closed

Use fee_per_second from asset metadata in XCM config #393

ebma opened this issue Jan 10, 2024 · 7 comments · Fixed by #410 or #432
Assignees

Comments

@ebma
Copy link
Member

ebma commented Jan 10, 2024

With #392 we have a fee_per_second attribute available for each asset. We want to use this value to replace the RelativeValues defined eg here.

An example of an implementation that uses the fee_per_second attribute defined in the asset registry to derive XCM fees can be found here.

@annatekl
Copy link

@gianfra-t
Copy link
Contributor

gianfra-t commented Feb 5, 2024

Hey @pendulum-chain/devs, I have doubts into which route to take to implement this, so far I see two roads:

We could use the fee_per_second value in order to calculate the relative value, but maintain the use of the WeightToFee implementation to get the fee amount in terms of the Native token.
This way, we would not really be using the fee_per_second in the sense of it's name, but rather an indicator of the relative value of the token when compared to the native one. Also, we would need to make fee_per_second = 1 for the Native.

The other alternative is to replace completely the use of WeightToFee and implement something like this in our charge_weight_in_fungibles method of ChargeWeightInFungibles, where WEIGHT_PER_SECOND is a constant from Frame.

I don't see any other way to combine these two solutions because the actual baselines to calculate the fees are different. In terms of preference, I prefer the first option since we would still have control of the WeightToFee value, but as negatives we have that it can be confusing the use of the fee_per_second value. WDYT?

Also, both solutions would allow to control the absolute value of the fees for each token, just in different ways.

@TorstenStueber
Copy link
Member

@gianfra-t At the end both solutions are equivalent if the parameters are defined appropriately, correct?

Now that we have the fee_per_second parameter in the asset registry, I think it is fair to use it straightforward and use the second solution. I would have taken that as the most obvious way to incorporate the fee_per_second parameter anyway, since weight is just defined as a unit of processing time on a standard machine.

We could change our WeightToFee implementation to also use a fee_per_second parameter we define for the native token. But that would always be one more database lookup for calculating transaction fees, so I think it is okay the way it is. We should just ensure that the fee_per_second for the native asset coincides with the multiplier we use in WeightToFee.

@ebma
Copy link
Member Author

ebma commented Feb 6, 2024

I'd also prefer the second option. I think, using the fee_per_second parameter like this should make it easier to calculate with a script like this compared to treating it like a relative value.

Is there a good reason not to implement it similar to what Centrifuge did, ie. using a struct that implements the orml_traits::FixedConversionRateProvider and then using it as part of a FixedRateAssetRegistryTrader in the Trader of our XCM config? Do we need to keep ChargeWeightInFungibles around or can we just skip it?

By the way, @gianfra-t please don't forget to assign yourself to tickets and move them to 'in development' once you start working on them.

@TorstenStueber
Copy link
Member

Looks good to me to use the AssetRegistryTrader trait as the Trader similarly to what Centrifuge does.

@gianfra-t
Copy link
Contributor

Thanks for your inputs! Let's go with the "second option" and adjust fee_per_second accordingly.

As for your question @ebma I think we can also use that other trader, because it behaves in the same way as FirsrAssetTrader , so we will probably end up with a very similar configuration as the link you provided.

@ebma
Copy link
Member Author

ebma commented Mar 14, 2024

Opening this again because we reverted the changes in #431.

@ebma ebma reopened this Mar 14, 2024
@ebma ebma closed this as completed in #432 Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants