-
Notifications
You must be signed in to change notification settings - Fork 353
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(streamer): don't distribute abstained part of sponsored distribution #1097
Conversation
WalkthroughThe changes streamline the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- x/streamer/types/distr_info.go (2 hunks)
- x/streamer/types/distr_info_test.go (1 hunks)
Additional comments not posted (5)
x/streamer/types/distr_info.go (1)
38-50
: Simplified Distribution Logic Looks Good!The changes to
DistrInfoFromDistribution
simplify the logic by directly assigningg.Power
toWeight
and usingd.VotingPower
forTotalWeight
. This aligns with the PR objective of preventing redistribution of abstained portions. Ensure that this logic is consistent with the overall distribution strategy.x/streamer/types/distr_info_test.go (4)
19-26
: Test Case for Empty Distribution is Well-DefinedThe test case for an empty distribution ensures that the function correctly handles scenarios with no gauges. This is a good edge case to validate.
27-47
: Test Case for Single Gauge Distribution is CorrectThe test case effectively verifies the function's handling of a distribution with one gauge, ensuring the correct assignment of weight and total weight.
48-76
: Test Case for Multiple Gauges is ComprehensiveThe test case for multiple gauges ensures the function aggregates weights correctly, validating the logic for handling several gauges.
77-107
: Test Case for Abstained Gauge is Well-ConstructedThe test case correctly verifies that abstained portions remain unallocated, ensuring the function's behavior aligns with the intended distribution logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- x/streamer/types/distr_info.go (2 hunks)
- x/streamer/types/distr_info_test.go (1 hunks)
Additional comments not posted (2)
x/streamer/types/distr_info.go (1)
38-50
: Simplified function logic improves readability.The function
DistrInfoFromDistribution
now directly assignsd.VotingPower
toTotalWeight
and appendsDistrRecord
instances more concisely. This change enhances readability and maintains the intended functionality of handling abstained portions by not redistributing them.x/streamer/types/distr_info_test.go (1)
13-115
: Comprehensive test coverage forDistrInfoFromDistribution
.The test cases effectively cover scenarios such as empty distributions, single and multiple gauges, and handling of abstained gauges. This ensures the function's correctness and robustness across different inputs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- x/streamer/types/distr_info.go (2 hunks)
- x/streamer/types/distr_info_test.go (1 hunks)
Additional comments not posted (2)
x/streamer/types/distr_info.go (1)
38-50
: Simplified weight calculation logic.The changes streamline the function by directly assigning
Weight
fromg.Power
andTotalWeight
fromd.VotingPower
, removing unnecessary calculations. This enhances clarity and aligns with the PR objective of preventing redistribution of abstained portions.x/streamer/types/distr_info_test.go (1)
13-115
: Comprehensive test coverage forDistrInfoFromDistribution
.The test cases effectively cover scenarios such as empty distributions, single and multiple gauges, and abstained gauges. This ensures robust validation of the updated distribution logic and aligns with the PR objectives.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curiuos what is the difference between streamer DistrInfo and sponsorship Distribution?
Description
Don't distribute the abstained part of the sponsored distribution.
Closes #1095
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.
PR review checkboxes:
I have...
Unreleased
section inCHANGELOG.md
godoc
commentsSDK Checklist
map
time.Now()
sendCoin
and notSendCoins
Full security checklist here
For Reviewer:
After reviewer approval:
Summary by CodeRabbit
New Features
DistrInfoFromDistribution
function to ensure accuracy and reliability.Bug Fixes
Documentation