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

Make weights compatible with u64 #4192

Merged
merged 5 commits into from
Nov 29, 2019
Merged

Make weights compatible with u64 #4192

merged 5 commits into from
Nov 29, 2019

Conversation

stanislav-tkach
Copy link
Contributor

Closes #4141.

@stanislav-tkach stanislav-tkach added the A3-in_progress Pull request is in progress. No review needed at this stage. label Nov 23, 2019
@kianenigma
Copy link
Contributor

need to check if this change is breaking in anyway for Kusama

@stanislav-tkach stanislav-tkach removed the A3-in_progress Pull request is in progress. No review needed at this stage. label Nov 25, 2019
@stanislav-tkach stanislav-tkach changed the title [In Progress] Make weights compatible with u64 Make weights compatible with u64 Nov 25, 2019
bkchr
bkchr reviewed Nov 25, 2019
View reviewed changes
bin/node/runtime/src/impls.rs Outdated
@@ -339,8 +339,8 @@ mod tests {
100 * kb,
mb,
10 * mb,
Weight::max_value() / 2,
Weight::max_value()
(u32::max_value() / 2).into(),
Copy link
Member

Choose a reason for hiding this comment

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

This is against the whole purpose of this pr.

@gavofyork gavofyork added the A0-please_review Pull request needs code review. label Nov 26, 2019
@stanislav-tkach
Copy link
Contributor Author

Current state: Weight is used everywhere instead of u32, so changing the underlying Weight type no longer requires additional refactoring. There is an issue with u64, though: the weight_to_fee_should_not_overflow_on_large_weights test fails in that case. I have digged into that a little, but I need more time.

I think that we can merge this pull request as it is and create a separate issue for dealing with the overflow.

bkchr
bkchr approved these changes Nov 28, 2019
View reviewed changes
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Needs resolving.

@gavofyork
Copy link
Member

CI fails.

@gavofyork gavofyork removed the A0-please_review Pull request needs code review. label Nov 28, 2019
@stanislav-tkach
Copy link
Contributor Author

I have fixed the benchmarks, though gitlab-check_polkadot is failing now. As far as I can see, it is unrelated to my changes.

@bkchr bkchr merged commit d5e189e into master Nov 29, 2019
@bkchr bkchr deleted the stas-4141 branch November 29, 2019 19:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkchr bkchr bkchr approved these changes

@kianenigma kianenigma Awaiting requested review from kianenigma

@pepyakin pepyakin Awaiting requested review from pepyakin

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make weights compatible with u64
4 participants
@stanislav-tkach @kianenigma @gavofyork @bkchr