-
Notifications
You must be signed in to change notification settings - Fork 2.6k
More efficient identity and multiplier weight to fee #11226
More efficient identity and multiplier weight to fee #11226
Conversation
@@ -710,6 +710,10 @@ where | |||
degree: 1, | |||
}) | |||
} | |||
|
|||
fn calc(weight: &Weight) -> Self::Balance { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
substrate/frame/support/src/weights.rs Lines 668 to 693 in b13c73f
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 🤷 |
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 Seems the trait should simply be Then there should be a subtrait Does that make sense? Then you can simply have Identity be the true identity without any polynomial bs. |
I like the idea of |
I think you just need this:
Then just update the |
Seems in TransactionPayment Pallet, there is some exposed consts which assume that we use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
anyway, my comments aren't blocking this optimization. so lets get this in
bot merge |
@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 :) |
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. |
#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 toIdentityFee
to avoid going polynomial route since it is not necessary for those cases.