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 refund external cost #2747

Closed
wants to merge 2 commits into from
Closed

Conversation

ahmadkaouk
Copy link
Contributor

@ahmadkaouk ahmadkaouk commented Apr 10, 2024

What does it do?

Fixes refund_external_cost which is not working because the current implementation creates a temporary copy of weight_info and applies changes to it, resulting in changes not being saved.

Frontier PR: polkadot-evm/frontier#1377

What important points reviewers should know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

@ahmadkaouk ahmadkaouk added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit not-breaking Does not need to be mentioned in breaking changes labels Apr 10, 2024
Copy link
Contributor

github-actions bot commented Apr 10, 2024

Coverage Report

@@                        Coverage Diff                         @@
##           master   ahmad-fix-refund-external-cost      +/-   ##
==================================================================
+ Coverage   72.46%                           72.79%   +0.33%     
- Files         229                              228       -1     
- Lines       70512                            70206     -306     
==================================================================
+ Hits        51094                            51105      +11     
- Misses      19418                            19101     -317     
Files Changed Coverage
/pallets/xcm-transactor/src/lib.rs 85.74% (+0.26%) 🔼

Coverage generated Wed Apr 10 19:15:16 UTC 2024

@ahmadkaouk ahmadkaouk marked this pull request as ready for review April 10, 2024 14:07
@RomarQ RomarQ self-requested a review April 10, 2024 14:20
@RomarQ
Copy link
Contributor

RomarQ commented Apr 10, 2024

Closing. The frontier fork was updated in #2745

@RomarQ RomarQ closed this Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D3-trivial PR contains trivial changes in a runtime directory that do not require an audit not-breaking Does not need to be mentioned in breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants