Moving `pallet-asset-tx-payment` from cumulus to substrate #10127

georgesdib · 2021-10-30T21:24:07Z

Moving pallet-asset-tx-payment into substrate given there is nothing cumulus specific in it.

@apopiak @nukemandan
related cumulus PR building on this one: paritytech/cumulus#712

Polkadot address: 131dPecTmpTC1p1ofemufqFBJo9vNbV7dkgN7vWwKnaSMkC4

apopiak · 2021-11-03T09:39:14Z

Thank you for your contribution! I took the liberty of moving the pallet into transaction-payment because it is an extension of that pallet.

…x-payment-from-cumulus

apopiak · 2021-11-03T09:44:09Z

@shawntabrizi Do we want to add this pallet to the node runtime? Will break transaction format, but would probably be good to make sure that it keeps working with any changes.

bkchr · 2021-11-03T11:26:05Z

The Substrate node is just for testing, for nothing else. So, it should be fine to add this.

apopiak · 2021-11-03T11:32:12Z

@georgesdib
Could you please add the new pallet to the node runtime

I had to change the Balance type to u128. Also harmonised that pallet's version

georgesdib · 2021-11-03T23:02:20Z

@georgesdib
Could you please add the new pallet to the node runtime

Done, however I had to do quite a few changes, and some (like changing Balance from u64 to u128) could be controversial.

…orgesdib/substrate into asset-tx-payment-from-cumulus

apopiak

The SignedExtension is replaced, not added.

bin/node/cli/src/service.rs

bin/node/runtime/src/lib.rs

bin/node/test-runner-example/src/lib.rs

apopiak · 2021-11-04T09:49:15Z

@jacogr I imagine this PR would have an impact on how Polkadot-js handles the default Substrate node.

jacogr · 2021-11-04T13:48:10Z

No impact, the metadata defined which extensions should be used. (As long as it knows the type and in this case it does... assuming no changes to the format of the extension...)

from `0.3.1` to `0.3.2`

georgesdib · 2021-11-11T15:36:07Z

How is this related to this PR?

It's not. I have reverted the impl-serde accidental upgrade.

georgesdib · 2021-11-11T17:18:08Z

All checks are green, @kianenigma @tomusdrw look good to merge?

into asset-tx-payment-from-cumulus

frame/transaction-payment/asset-tx-payment/src/payment.rs

kianenigma

Looks good to me!

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

Reverting changes which broke cargo fmt

georgesdib · 2021-11-18T19:12:49Z

@apopiak we have all approvals, shall we merge it?

apopiak · 2021-11-19T09:29:02Z

bot merge

bkchr · 2021-11-19T19:51:57Z

/tip small

substrate-tip-bot · 2021-11-19T19:51:58Z

Please fix the following problems before calling the tip bot again:

Contributor did not properly post their Polkadot or Kusama address. Make sure the pull request has: "{network} address: {address}".

bkchr · 2021-11-19T19:54:01Z

@georgesdib if you add your address, I can ping the bot another time.

georgesdib · 2021-11-19T19:57:56Z

@georgesdib if you add your address, I can ping the bot another time.

Done, thank you very much!

bkchr · 2021-11-19T20:13:19Z

/tip small

substrate-tip-bot · 2021-11-19T20:13:32Z

A small tip was successfully submitted for georgesdib (131dPecTmpTC1p1ofemufqFBJo9vNbV7dkgN7vWwKnaSMkC4 on polkadot).

https://polkadot.js.org/apps/#/treasury/tips

…h#10127) * Moving `pallet-asset-tx-payment` from cumulus * move pallet-asset-tx-payment into transaction payment directory * cargo +nightly fmt * Adding `pallet-asset-tx-payment` to node runtime I had to change the Balance type to u128. Also harmonised that pallet's version * Updating cargo.lock after merge * forgot this * Adding tx-payment signature * Missed one more * `transaction-payment` replaced in`SignedExtension` by `asset-tx-payment` and not added * Fixing benches * add test to verify that we don't charge on post-dispatch if we didn't on pre-dispatch * add (failing) test for asset tx payment of unsigned extrinsics * fix test by removing debug_assert * cargo +nightly fmt * typo in `Cargo.lock` * Object defined twice in lock file * cargo update * remove todo * Apply formatting suggestions from code review Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Refactoring `post_dispatch` of `asset-tx-payment` to reuse `post_dispatch` of `transaction-payment` if the fee asset is native Removing unneeded imports. * Removing redundant `TODO` * Reverting an accidental bump of `impl-serde` from `0.3.1` to `0.3.2` * Revert unneeded changes to `cargo.lock` * Update frame/transaction-payment/asset-tx-payment/src/payment.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Fixing cargo fmt Reverting changes which broke cargo fmt Co-authored-by: Alexander Popiak <alexander.popiak@parity.io> Co-authored-by: Alexander Popiak <alexander.popiak@gmail.com> Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

…aritytech#10127)" This reverts commit 4581dd9.

…h#10127) * Moving `pallet-asset-tx-payment` from cumulus * move pallet-asset-tx-payment into transaction payment directory * cargo +nightly fmt * Adding `pallet-asset-tx-payment` to node runtime I had to change the Balance type to u128. Also harmonised that pallet's version * Updating cargo.lock after merge * forgot this * Adding tx-payment signature * Missed one more * `transaction-payment` replaced in`SignedExtension` by `asset-tx-payment` and not added * Fixing benches * add test to verify that we don't charge on post-dispatch if we didn't on pre-dispatch * add (failing) test for asset tx payment of unsigned extrinsics * fix test by removing debug_assert * cargo +nightly fmt * typo in `Cargo.lock` * Object defined twice in lock file * cargo update * remove todo * Apply formatting suggestions from code review Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Refactoring `post_dispatch` of `asset-tx-payment` to reuse `post_dispatch` of `transaction-payment` if the fee asset is native Removing unneeded imports. * Removing redundant `TODO` * Reverting an accidental bump of `impl-serde` from `0.3.1` to `0.3.2` * Revert unneeded changes to `cargo.lock` * Update frame/transaction-payment/asset-tx-payment/src/payment.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> * Fixing cargo fmt Reverting changes which broke cargo fmt Co-authored-by: Alexander Popiak <alexander.popiak@parity.io> Co-authored-by: Alexander Popiak <alexander.popiak@gmail.com> Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

Moving pallet-asset-tx-payment from cumulus

7eae0c6

This was referenced Oct 30, 2021

Moving pallet-asset-tx-payment from cumulus to substrate paritytech/cumulus#712

Merged

Move pallet-asset-tx-payment into Substrate paritytech/cumulus#688

Closed

apopiak self-requested a review November 3, 2021 09:19

move pallet-asset-tx-payment into transaction payment directory

f8e7b6b

apopiak requested a review from shawntabrizi November 3, 2021 09:40

apopiak added the A0-please_review Pull request needs code review. label Nov 3, 2021

Merge branch 'master' of github.com:paritytech/substrate into asset-t…

d22d83a

…x-payment-from-cumulus

apopiak added B3-apinoteworthy C1-low PR touches the given topic and has a low impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Nov 3, 2021

apopiak requested a review from tomusdrw November 3, 2021 09:44

cargo +nightly fmt

78bbed9

Adding pallet-asset-tx-payment to node runtime

2791253

I had to change the Balance type to u128. Also harmonised that pallet's version

Georges added 6 commits November 3, 2021 23:04

Merge branch 'master' into asset-tx-payment-from-cumulus

2de2612

Updating cargo.lock after merge

a90cbb6

Merge branch 'asset-tx-payment-from-cumulus' of https://github.com/ge…

f19b1a2

…orgesdib/substrate into asset-tx-payment-from-cumulus

forgot this

02bf137

Adding tx-payment signature

74cbc9c

Missed one more

8498007

apopiak reviewed Nov 4, 2021

View reviewed changes

apopiak added the A3-in_progress Pull request is in progress. No review needed at this stage. label Nov 4, 2021

Reverting an accidental bump of impl-serde

03fa69b

from `0.3.1` to `0.3.2`

georgesdib requested a review from kianenigma November 11, 2021 23:21

apopiak added D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Nov 12, 2021

Georges Dib added 2 commits November 14, 2021 20:16

Merge branch 'master' of upsteam

6107177

into asset-tx-payment-from-cumulus

Revert unneeded changes to cargo.lock

6b56ab1

kianenigma reviewed Nov 18, 2021

View reviewed changes

frame/transaction-payment/asset-tx-payment/src/payment.rs Show resolved Hide resolved

kianenigma approved these changes Nov 18, 2021

View reviewed changes

georgesdib and others added 2 commits November 18, 2021 16:37

Update frame/transaction-payment/asset-tx-payment/src/payment.rs

21d9cc3

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

Fixing cargo fmt

9d01c8c

Reverting changes which broke cargo fmt

paritytech-processbot bot merged commit 4581dd9 into paritytech:master Nov 19, 2021

georgesdib deleted the asset-tx-payment-from-cumulus branch November 19, 2021 09:30

github-actions bot mentioned this pull request Dec 2, 2021

Update substrate/polkadot/cumulus from v0.9.12 to v0.9.13 moonbeam-foundation/moonbeam#1050

Closed

ghzlatarev mentioned this pull request Dec 6, 2021

[Manta] Integrate v0.9.13 upstream changes Manta-Network/Manta#294

Merged

23 tasks

h4x3rotab mentioned this pull request Jan 7, 2022

Support pallet-asset-tx-payment Phala-Network/phala-blockchain#621

Closed

MRamanenkau added a commit to Cerebellum-Network/pos-network-node that referenced this pull request Oct 14, 2022

Revert "Moving pallet-asset-tx-payment from cumulus to substrate (p…

d12cba1

…aritytech#10127)" This reverts commit 4581dd9.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moving `pallet-asset-tx-payment` from cumulus to substrate #10127

Moving `pallet-asset-tx-payment` from cumulus to substrate #10127

georgesdib commented Oct 30, 2021 •

edited

Loading

apopiak commented Nov 3, 2021

apopiak commented Nov 3, 2021

bkchr commented Nov 3, 2021

apopiak commented Nov 3, 2021

georgesdib commented Nov 3, 2021

apopiak left a comment

apopiak commented Nov 4, 2021

jacogr commented Nov 4, 2021

georgesdib commented Nov 11, 2021 •

edited

Loading

georgesdib commented Nov 11, 2021

kianenigma left a comment

georgesdib commented Nov 18, 2021

apopiak commented Nov 19, 2021

bkchr commented Nov 19, 2021

substrate-tip-bot bot commented Nov 19, 2021

bkchr commented Nov 19, 2021

georgesdib commented Nov 19, 2021

bkchr commented Nov 19, 2021

substrate-tip-bot bot commented Nov 19, 2021

Moving pallet-asset-tx-payment from cumulus to substrate #10127

Moving pallet-asset-tx-payment from cumulus to substrate #10127

Conversation

georgesdib commented Oct 30, 2021 • edited Loading

apopiak commented Nov 3, 2021

apopiak commented Nov 3, 2021

bkchr commented Nov 3, 2021

apopiak commented Nov 3, 2021

georgesdib commented Nov 3, 2021

apopiak left a comment

Choose a reason for hiding this comment

apopiak commented Nov 4, 2021

jacogr commented Nov 4, 2021

georgesdib commented Nov 11, 2021 • edited Loading

georgesdib commented Nov 11, 2021

kianenigma left a comment

Choose a reason for hiding this comment

georgesdib commented Nov 18, 2021

apopiak commented Nov 19, 2021

bkchr commented Nov 19, 2021

substrate-tip-bot bot commented Nov 19, 2021

bkchr commented Nov 19, 2021

georgesdib commented Nov 19, 2021

bkchr commented Nov 19, 2021

substrate-tip-bot bot commented Nov 19, 2021

Moving `pallet-asset-tx-payment` from cumulus to substrate #10127

Moving `pallet-asset-tx-payment` from cumulus to substrate #10127

georgesdib commented Oct 30, 2021 •

edited

Loading

georgesdib commented Nov 11, 2021 •

edited

Loading