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

Split ValueBalance methods into NegativeAllowed and NonNegative #2649

Merged
merged 2 commits into from
Aug 20, 2021

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Aug 20, 2021

Motivation

Some ValueBalance methods only make sense for NegativeAllowed or NonNegative.

If we make them generic, it's harder to use them correctly.
And type inference can fail on the generic, which makes code harder to write.

Solution

  • split out some methods into ValueBalance<NonNegative> and ValueBalance<NegativeAllowed>
  • fix serialization tests to use ValueBalance<NonNegative>, like the finalized state
  • fix an unused import clippy lint

Review

@oxarbitrage can review this PR, it also includes some of the test methods from PR #2647 and oxarbitrage#130

Reviewer Checklist

  • Code implements consensus rules
  • Existing tests work

This helps with type inference when passing values to some methods.
@teor2345 teor2345 added A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement P-Medium labels Aug 20, 2021
@teor2345 teor2345 requested a review from oxarbitrage August 20, 2021 03:33
@teor2345 teor2345 self-assigned this Aug 20, 2021
Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

Looks good, thanks.

@oxarbitrage oxarbitrage enabled auto-merge (squash) August 20, 2021 13:06
@oxarbitrage oxarbitrage merged commit 4691a87 into main Aug 20, 2021
@oxarbitrage oxarbitrage deleted the split-value-balance-methods branch August 20, 2021 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants