Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Replace TransactionBaseFee with ExtrinsicBaseWeight #5761

Merged

Conversation

shawntabrizi
Copy link
Member

@shawntabrizi shawntabrizi commented Apr 23, 2020

This is a pretty simple PR which replaces the notion of TransactionBaseFee with ExtrinsicBaseWeight * WeightToFee.

We should confirm that Transaction Byte length should still be handled independent of the Substrate weight system, as it is now.

@shawntabrizi shawntabrizi added the A0-please_review Pull request needs code review. label Apr 23, 2020
@shawntabrizi shawntabrizi added this to the 2.0 milestone Apr 23, 2020
@ghost
Copy link

ghost commented Apr 23, 2020

@shawntabrizi, this pull request is being closed because it does not explicitly address an issue.

@ghost ghost closed this Apr 23, 2020
@sjeohp-zz sjeohp-zz reopened this Apr 23, 2020
Copy link
Member

@athei athei left a comment

Choose a reason for hiding this comment

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

I think you forgot to refund the base weight fee in post_dispatch.

frame/transaction-payment/src/lib.rs Show resolved Hide resolved
@shawntabrizi
Copy link
Member Author

We can ignore the Polkadot check here cause I am just looking to merge into another PR, where I will update Polkadot companion.

@kianenigma
Copy link
Contributor

kianenigma commented Apr 24, 2020

I am not sure about the naming:

  • TransactionByteFee
  • ExtrinsicBaseWeight

why is one prefixed transaction and extrinsic?

  • Also, if any difference, I think it should be the other way around. The length fee is applied to the opaque Vec<u8> which is just some external data and we only care about the length thereof. IMO this is called Extrinsic, hence ExtrinsicByteFee.
  • Once this blob is being treated as a function+functor (aka Call), as either a signed or unsigned, it is called transaction, hence TransactionBaseWeight.

Unless if the link below is outdated?

@shawntabrizi
Copy link
Member Author

@kianenigma lets take it to the main PR 👍

@shawntabrizi shawntabrizi merged commit 290d7b5 into shawntabrizi-additional-block-weights Apr 24, 2020
@shawntabrizi shawntabrizi deleted the shawntabrizi-base-fee branch April 24, 2020 09:04
ghost pushed a commit that referenced this pull request Apr 25, 2020
* Introduce `BlockExectionWeight` and `ExtrinsicBaseWeight`

* Add new traits everywhere

* Missed one update

* fix tests

* Update `check_weight` logic

* introduce `max_extrinsic_weight` function

* fix + add tests

* format nits

* remove println

* make test a bit more clear

* Remove minimum weight

* newlines left over from find/replace

* Fix test, improve clarity

* Fix executor tests

* Extrinsic base weight same as old `MINIMUM_WEIGHT`

* fix example test

* Expose constants

* Add test for full block with operational and normal

* Initiate test environment with `BlockExecutionWeight` weight

* format nit

* Update frame/system/src/lib.rs

Co-Authored-By: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Replace `TransactionBaseFee` with `ExtrinsicBaseWeight` (#5761)

* Replace `TransactionBaseFee` with `ExtrinsicBaseFee`

* Fix stuff

* Fix and make tests better

* Forgot to update this test

* Fix priority number in test

* Remove minimum weight from merge

* Fix weight in contracts

* remove `TransactionBaseFee` from contract tests

* Let `register_extra_weight_unchecked` go past `MaximumBlockWeight`

* address feedback

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
sorpaas pushed a commit that referenced this pull request Nov 20, 2020
* Introduce `BlockExectionWeight` and `ExtrinsicBaseWeight`

* Add new traits everywhere

* Missed one update

* fix tests

* Update `check_weight` logic

* introduce `max_extrinsic_weight` function

* fix + add tests

* format nits

* remove println

* make test a bit more clear

* Remove minimum weight

* newlines left over from find/replace

* Fix test, improve clarity

* Fix executor tests

* Extrinsic base weight same as old `MINIMUM_WEIGHT`

* fix example test

* Expose constants

* Add test for full block with operational and normal

* Initiate test environment with `BlockExecutionWeight` weight

* format nit

* Update frame/system/src/lib.rs

Co-Authored-By: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Replace `TransactionBaseFee` with `ExtrinsicBaseWeight` (#5761)

* Replace `TransactionBaseFee` with `ExtrinsicBaseFee`

* Fix stuff

* Fix and make tests better

* Forgot to update this test

* Fix priority number in test

* Remove minimum weight from merge

* Fix weight in contracts

* remove `TransactionBaseFee` from contract tests

* Let `register_extra_weight_unchecked` go past `MaximumBlockWeight`

* address feedback

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants