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

fix!: integrate @substrate/[email protected] for partial fees #1017

Merged
merged 37 commits into from
Aug 31, 2022

Conversation

TarikGul
Copy link
Member

@TarikGul TarikGul commented Aug 12, 2022

Summary (BREAKING CHANGES)

This PR focuses on adding calc_partial_fee from @substrate/calc v0.3.0 so that fee calculations for both relay and parachains are completely accurate. It uses a combination of both payment_queryInfo and payment_queryFeeDetails in order to calculate the fee. If payment_queryFeeDetails is not present or available to use, it will default to payment_queryInfo. This PR also adds a field to the info of an extrinsic called kind which can either be preDispatch, or postDispatch. More Info below.

Removed

  • Breaking Change - Remove feeByEvent query param as it is no longer needed as Sidecar now has the accuracy it needs for fees. In the future if the event TransactionFeePaid is present it will take the fee from there.

Added

  • kind: 'preDispatch' | 'postDispatch' field in info for extrinsics.

    1. preDispatch: payment_queryInfo was used to get the partialFee given preDispatch information.
    2. postDispatch: calc_partial_fee was used to calculate the fee using post dispatch fee information.
  • calc_partial_fee: /blocks/* now uses this method to accurately calculate partialFee's for extrinsics.

Fixed

  • Pass in the previous blockHash for an extrinsic query for payment_queryInfo

Notes

  • Since there is no real way to evaluate if payment_queryFeeDetails is available in the runtime, and polkadot-js will throw an error regardless of catching it. Therefore we log a warning after the error is emitted to let users know the error is not important, than we cache the runtime to not call payment_queryFeeDetails again and instead recieve the partialFee from queryInfo.

Todo

  • e2e-tests
  • Ensure Docs are complete
  • Find out what can be done to correctly handle payment_queryFeeDetails for a runtime that doesn't support it.
      1. We cant catch the error from polkadot-js rpc call. It will log regardless of handling it.

@TarikGul TarikGul added B2 - Breaks API Breaking changes to the API I8 - Enhancement Additional feature request labels Aug 12, 2022
@TarikGul TarikGul requested a review from a team as a code owner August 12, 2022 22:17
docs/src/openapi-v1.yaml Outdated Show resolved Hide resolved
@niklasad1
Copy link
Member

As a side-not to this PR when paritytech/substrate#11648 is a thing I guess it would simplify things for us.

Copy link
Member

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks alright, I think the QueryFeeDetailsCache would benefit with some additional docs such that "this is a workaround" because it's not possible to fetch which runtime APIs are available.

@TarikGul TarikGul added the A0 - Please Review PR is ready for review label Aug 30, 2022
Copy link
Collaborator

@jsdw jsdw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@TarikGul TarikGul merged commit 92e3e1d into master Aug 31, 2022
@TarikGul TarikGul deleted the tarik-fix-partial-fee-calc branch August 31, 2022 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0 - Please Review PR is ready for review B2 - Breaks API Breaking changes to the API I8 - Enhancement Additional feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants