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

Take holds into account for balance-consistency smoke test #2621

Merged
merged 4 commits into from
Jan 19, 2024

Conversation

elfedy
Copy link
Contributor

@elfedy elfedy commented Jan 18, 2024

What does it do?

S300C100 Smoke test fails with runtime 2700 due to some reserve amount being added to hold, so test needs to be adapted to that change by looking at holds and adding them to the expected reserves.

@elfedy elfedy requested a review from timbrinded January 18, 2024 21:09
@librelois
Copy link
Collaborator

LGTM, but I don't master the style of this part of the code, so we'd need an additional review from someone who knows better, maybe @timbrinded or @Agusrodri ?

@librelois librelois added B0-silent Changes should not be mentioned in any release notes D2-notlive PR doesn't change runtime code (so can't be audited) labels Jan 18, 2024
Copy link
Contributor

github-actions bot commented Jan 18, 2024

Coverage Report

@@                  Coverage Diff                   @@
##           master   elfedy-fix-smoke-bc     +/-   ##
======================================================
  Coverage   80.94%                80.94%   0.00%     
  Files         287                   287             
  Lines       94330                 94330             
======================================================
- Hits        76355                 76354      -1     
+ Misses      17975                 17976      +1     
Files Changed Coverage
/primitives/xcm/src/fee_handlers.rs 98.10% (-0.32%) 🔽

Coverage generated Fri Jan 19 14:06:57 UTC 2024

@librelois
Copy link
Collaborator

@elfedy can you please apply the new logic only if specVersion >= 2700 ? Otherwise it will fail for live networks with old runtimes

elfedy and others added 2 commits January 19, 2024 09:17
@elfedy
Copy link
Contributor Author

elfedy commented Jan 19, 2024

@elfedy can you please apply the new logic only if specVersion >= 2700 ? Otherwise it will fail for live networks with old runtimes

@librelois done

@timbrinded
Copy link
Contributor

This test case is a nightmare (for perf reasons) but this change looks good to me.

@elfedy elfedy merged commit b0846d4 into master Jan 19, 2024
@elfedy elfedy deleted the elfedy-fix-smoke-bc branch January 19, 2024 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B0-silent Changes should not be mentioned in any release notes D2-notlive PR doesn't change runtime code (so can't be audited)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants