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

Implement foreign asset transaction fees #1022

Merged
merged 23 commits into from
Jul 18, 2023
Merged

Conversation

@Chralt98 Chralt98 added the s:in-progress The pull requests is currently being worked on label Jun 13, 2023
@Chralt98 Chralt98 self-assigned this Jun 13, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jun 13, 2023

Codecov Report

Merging #1022 (5c028e0) into main (31b7d64) will decrease coverage by 1.58%.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main    paritytech/substrate#1022      +/-   ##
==========================================
- Coverage   94.50%   92.93%   -1.58%     
==========================================
  Files          92       92              
  Lines       21070    21589     +519     
==========================================
+ Hits        19913    20064     +151     
- Misses       1157     1525     +368     
Flag Coverage Δ
tests 92.93% <ø> (-1.58%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 21 files with indirect coverage changes

@Chralt98 Chralt98 requested a review from sea212 as a code owner June 23, 2023 06:45
@Chralt98 Chralt98 added s:review-needed The pull request requires reviews and removed s:in-progress The pull requests is currently being worked on labels Jun 23, 2023
@Chralt98 Chralt98 requested review from maltekliemann and removed request for maltekliemann June 23, 2023 08:05
@Chralt98 Chralt98 added s:in-progress The pull requests is currently being worked on and removed s:review-needed The pull request requires reviews labels Jul 4, 2023
runtime/common/src/fees.rs Outdated Show resolved Hide resolved
@Chralt98 Chralt98 added s:in-progress The pull requests is currently being worked on and removed s:in-progress The pull requests is currently being worked on labels Jul 5, 2023
@Chralt98 Chralt98 closed this Jul 5, 2023
@Chralt98 Chralt98 reopened this Jul 5, 2023
@Chralt98 Chralt98 added s:review-needed The pull request requires reviews and removed s:in-progress The pull requests is currently being worked on labels Jul 5, 2023
@sea212 sea212 added this to the v0.4.0 milestone Jul 7, 2023
@sea212 sea212 added s:revision-needed The pull requests must be revised and removed s:review-needed The pull request requires reviews labels Jul 17, 2023
@sea212
Copy link
Member

sea212 commented Jul 17, 2023

Please also update docs/changelog_for_devs.md

@Chralt98 Chralt98 added s:in-progress The pull requests is currently being worked on and removed s:revision-needed The pull requests must be revised labels Jul 17, 2023
@Chralt98 Chralt98 requested a review from sea212 July 17, 2023 13:18
@Chralt98 Chralt98 added s:review-needed The pull request requires reviews and removed s:in-progress The pull requests is currently being worked on labels Jul 17, 2023
sea212
sea212 previously approved these changes Jul 17, 2023
Comment on lines 54 to 57
let _ = <Tokens as Balanced<AccountId>>::resolve(
&TreasuryPalletId::get().into_account_truncating(),
fees_and_tips,
);
Copy link
Member

Choose a reason for hiding this comment

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

debug_assert! would be useful here to know when funds are just lost.

Copy link
Member Author

Choose a reason for hiding this comment

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

true, 5c028e0

maltekliemann
maltekliemann previously approved these changes Jul 17, 2023
@Chralt98 Chralt98 dismissed stale reviews from maltekliemann and sea212 via 5c028e0 July 18, 2023 07:29
@maltekliemann maltekliemann self-requested a review July 18, 2023 07:39
@Chralt98 Chralt98 added s:accepted This pull request is ready for merge and removed s:review-needed The pull request requires reviews labels Jul 18, 2023
@Chralt98 Chralt98 merged commit ea97e10 into main Jul 18, 2023
12 checks passed
@Chralt98 Chralt98 deleted the chralt98-foreign-tx-fee branch July 18, 2023 09:34
@sea212 sea212 added i:spec-changed ⚠️ Implies change in spec version i:transactions-changed ⚠️ Implies change in transaction version labels Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i:spec-changed ⚠️ Implies change in spec version i:transactions-changed ⚠️ Implies change in transaction version s:accepted This pull request is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants