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

feat(streamer): added sponsored distribution support #1045

Merged
merged 7 commits into from
Aug 12, 2024

Conversation

keruch
Copy link
Contributor

@keruch keruch commented Aug 6, 2024

Description

Part of #983

This PR adds x/sponsorship support for x/streamer:

  • Added sponsored flag for the stream type
  • Allowed creating sponsored streams
  • Query x/sponsorship to get the distr for sponsored gauges at the end of the epoch

  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Targeted PR against the correct branch
  • included the correct type prefix in the PR title
  • Linked to the GitHub issue with discussion and accepted design
  • Targets only one GitHub issue
  • Wrote unit and integration tests
  • Wrote relevant migration scripts if necessary
  • All CI checks have passed
  • Added relevant godoc comments
  • Updated the scripts for local run, e.g genesis_config_commands.sh if the PR changes parameters <-- in the next PR
  • Add an issue in the e2e-tests repo if necessary

SDK Checklist

  • Import/Export Genesis
  • Registered Invariants
  • Registered Events
  • Updated openapi.yaml
  • No usage of go map
  • No usage of time.Now()
  • Used fixed point arithmetic and not float arithmetic
  • Avoid panicking in Begin/End block as much as possible
  • No unexpected math Overflow
  • Used sendCoin and not SendCoins
  • Out-of-block compute is bounded
  • No serialized ID at the end of store keys
  • UInt to byte conversion should use BigEndian

Full security checklist here


For Reviewer:

  • Confirmed the correct type prefix in the PR title
  • Reviewers assigned
  • Confirmed all author checklist items have been addressed

After reviewer approval:

  • In case the PR targets the main branch, PR should not be squash merge in order to keep meaningful git history.
  • In case the PR targets a release branch, PR must be rebased.

Summary by CodeRabbit

  • New Features

    • Introduced the ability to create sponsored streams within the application.
    • Added sponsorship-related parameters to stream creation proposals and distribution mechanisms.
    • Enhanced stream handling by supporting both sponsored and non-sponsored types.
  • Bug Fixes

    • Improved testing for sponsored streams, ensuring accurate distribution calculations based on user votes.
  • Tests

    • Expanded test coverage to include scenarios for both sponsored and non-sponsored streams.

@keruch keruch requested a review from mtsitrin August 6, 2024 18:38
@keruch keruch self-assigned this Aug 6, 2024
@keruch keruch requested a review from a team as a code owner August 6, 2024 18:38
Copy link
Contributor

coderabbitai bot commented Aug 6, 2024

Walkthrough

The recent changes enhance the streaming application's functionality by integrating sponsorship management into key components. This includes updates to keepers and tests, which now accommodate both sponsored and non-sponsored streams, allowing for more flexible stream creation. The modifications aim to streamline the sponsorship process, improve test coverage, and provide clearer interfaces for managing sponsorship distributions, ultimately enriching the ecosystem surrounding stream creation and management.

Changes

Files Change Summary
app/keepers/keepers.go, x/streamer/keeper/... Integrated SponsorshipKeeper into the AppKeepers and updated initialization for StreamerKeeper, enhancing sponsorship functionality.
x/streamer/keeper/keeper_create_stream_test.go Added NonSponsored parameter to CreateStream tests and introduced TestCreateSponsoredStream to cover scenarios for both sponsored and non-sponsored streams.

Poem

🐇 In fields so green, changes arise,
With sponsorship dreams that touch the skies.
Streams now dance with new delight,
A hop, a skip, in morning light!
Join the fun, the joy, the cheer,
With every stream, let sponsorship near!


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@keruch keruch changed the base branch from main to kirill/sponsorship-voting August 6, 2024 18:42
@keruch keruch changed the title Kirill/streamer sponsorship feat(streamer): added sponsored distribution support Aug 6, 2024
@keruch keruch added the c:sponsorship x/sponsorship module label Aug 7, 2024
@keruch keruch force-pushed the kirill/sponsorship-voting branch from 269ae5d to c8a5dd9 Compare August 8, 2024 09:22
@keruch keruch force-pushed the kirill/streamer-sponsorship branch from 9a57c23 to 34f2e42 Compare August 8, 2024 09:27
x/streamer/types/distr_info.go Outdated Show resolved Hide resolved
x/streamer/types/distr_info.go Show resolved Hide resolved
x/streamer/keeper/keeper.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@keruch keruch force-pushed the kirill/sponsorship-voting branch from c8a5dd9 to 1433c2e Compare August 9, 2024 13:37
@keruch keruch force-pushed the kirill/streamer-sponsorship branch from 34f2e42 to c7f4714 Compare August 9, 2024 13:47
@keruch keruch force-pushed the kirill/sponsorship-voting branch from 1433c2e to 53e0d32 Compare August 9, 2024 16:07
@keruch keruch force-pushed the kirill/streamer-sponsorship branch from c7f4714 to 1711972 Compare August 9, 2024 16:09
@keruch keruch requested a review from omritoptix August 10, 2024 08:05
Base automatically changed from kirill/sponsorship-voting to main August 10, 2024 10:02
omritoptix
omritoptix previously approved these changes Aug 10, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Outside diff range, codebase verification and nitpick comments (2)
x/streamer/types/stream.go (1)

Ensure NewStream Calls Include sponsored Parameter

The call to NewStream in x/streamer/keeper/keeper.go does not include the sponsored parameter. Please update this instance to ensure consistency with the new function signature.

  • x/streamer/keeper/keeper.go: Update NewStream call to include the sponsored parameter.
Analysis chain

Line range hint 10-20:
Ensure consistent usage of the sponsored parameter.

The addition of the sponsored parameter to the NewStream function and the Stream struct is well-implemented. Ensure that all instances of stream creation throughout the codebase are updated to use this new parameter appropriately.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of `NewStream` usage are updated with the `sponsored` parameter.

# Test: Search for the `NewStream` function calls. Expect: All calls include the `sponsored` parameter.
rg --type go -A 5 $'NewStream('

Length of output: 103


Script:

#!/bin/bash
# Description: Verify all instances of `NewStream` usage are updated with the `sponsored` parameter.

# Test: Search for the `NewStream` function calls. Expect: All calls include the `sponsored` parameter.
rg --type go -A 5 'NewStream('

Length of output: 103


Script:

#!/bin/bash
# Description: Verify all instances of `NewStream` usage are updated with the `sponsored` parameter.

# Test: Search for the `NewStream` function calls. Expect: All calls include the `sponsored` parameter.
rg 'NewStream(' --type go -A 5

Length of output: 103


Script:

#!/bin/bash
# Description: Verify all instances of `NewStream` usage are updated with the `sponsored` parameter.

# Test: Use grep to search for the `NewStream` function calls. Expect: All calls include the `sponsored` parameter.
grep -r 'NewStream(' --include \*.go -A 5

Length of output: 796

x/streamer/types/expected_keepers.go (1)

6-12: Import order adjustment noted.

The change in import order is stylistic and does not affect functionality. Ensure consistency with project guidelines.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 836e500 and 5f37893.

Files ignored due to path filters (2)
  • x/streamer/types/gov_stream.pb.go is excluded by !**/*.pb.go
  • x/streamer/types/stream.pb.go is excluded by !**/*.pb.go
Files selected for processing (17)
  • app/keepers/keepers.go (1 hunks)
  • proto/dymensionxyz/dymension/streamer/gov_stream.proto (1 hunks)
  • proto/dymensionxyz/dymension/streamer/stream.proto (1 hunks)
  • testutil/keeper/streamer.go (2 hunks)
  • x/streamer/client/cli/query_test.go (2 hunks)
  • x/streamer/client/cli/tx_create_stream.go (4 hunks)
  • x/streamer/keeper/distribute.go (2 hunks)
  • x/streamer/keeper/distribute_test.go (2 hunks)
  • x/streamer/keeper/genesis_test.go (2 hunks)
  • x/streamer/keeper/keeper.go (4 hunks)
  • x/streamer/keeper/keeper_create_stream_test.go (4 hunks)
  • x/streamer/keeper/suite_test.go (3 hunks)
  • x/streamer/proposal_handler.go (2 hunks)
  • x/streamer/types/distr_info.go (2 hunks)
  • x/streamer/types/expected_keepers.go (2 hunks)
  • x/streamer/types/proposal_stream.go (2 hunks)
  • x/streamer/types/stream.go (2 hunks)
Additional comments not posted (28)
testutil/keeper/streamer.go (2)

15-16: The addition of require enhances test assertions.

Including the require package from testify is a good practice for making robust assertions in tests. Ensure that this is used effectively in the test cases.


47-47: Verify the purpose of the additional nil argument.

The addition of a nil argument in the keeper.NewKeeper function call suggests a change in the keeper's constructor. Verify that this change aligns with the intended functionality and does not introduce any unintended behavior.

Verification successful

The additional nil argument in NewKeeper is likely intentional for testing purposes.

The nil arguments are placeholders for optional or test-specific implementations of keeper interfaces like BankKeeper, EpochKeeper, etc., in the NewKeeper function. This is common in test environments where full implementations are not necessary. Ensure that the test context correctly handles these nil values to avoid unintended behavior.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the purpose of the additional `nil` argument in `NewKeeper`.

# Test: Search for the `NewKeeper` function definition and usages to understand the role of the new parameter.
rg --type go -A 5 $'NewKeeper('

Length of output: 103


Script:

#!/bin/bash
# Description: Corrected search for the `NewKeeper` function definition and usages to understand the role of the new parameter.
rg --type go -A 5 'NewKeeper('

Length of output: 103


Script:

#!/bin/bash
# Description: Search for the `NewKeeper` function definition and usages with corrected pattern.
rg --type go -A 5 'NewKeeper\('

Length of output: 14363


Script:

#!/bin/bash
# Description: Retrieve the full implementation of the `NewKeeper` function in `x/streamer/keeper/keeper.go` to understand the role of the `nil` argument.
ast-grep --lang go --pattern 'func NewKeeper($_, $_, $$$) *Keeper { $$$ }'

Length of output: 7145

proto/dymensionxyz/dymension/streamer/gov_stream.proto (1)

41-43: The addition of the sponsored field enriches the CreateStreamProposal message.

Adding the sponsored field provides additional context for the stream's nature. Ensure that this field is correctly handled in serialization, deserialization, and any logic that processes CreateStreamProposal messages.

x/streamer/types/expected_keepers.go (1)

42-44: Addition of SponsorshipKeeper interface looks good.

The SponsorshipKeeper interface is a clear addition that extends the module's capabilities. Ensure that implementations of this interface handle errors appropriately.

x/streamer/types/distr_info.go (2)

41-64: Addition of DistrInfoFromDistribution function looks good.

The function effectively converts a sponsorshiptypes.Distribution to DistrInfo with proper validation. Ensure that the weight calculation logic aligns with the intended distribution model.


4-7: New import for sponsorshiptypes is appropriate.

The addition of the sponsorshiptypes import is necessary for the new functionality.

proto/dymensionxyz/dymension/streamer/stream.proto (1)

54-55: Addition of sponsored field to Stream message looks good.

The sponsored field provides valuable information about the stream's sponsorship status. Ensure that all related logic correctly interprets this field.

x/streamer/proposal_handler.go (1)

31-33: LGTM! Verify the usage of CreateStream.

The addition of the sponsored parameter in the HandleCreateStreamProposal function is correct and aligns with the new feature requirements.

Ensure that all calls to k.CreateStream across the codebase are updated to include the new parameter.

Verification successful

All CreateStream calls include the sponsored parameter.

The verification confirms that the CreateStream function is consistently called with the sponsored parameter across the codebase. No further action is required.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `CreateStream` include the new `sponsored` parameter.

# Test: Search for the function usage. Expect: Only occurrences with the new parameter.
rg --type go -A 5 'CreateStream'

Length of output: 35168

x/streamer/client/cli/tx_create_stream.go (2)

19-20: Add FlagSponsored for sponsored stream support.

The introduction of the FlagSponsored constant is correct and necessary for handling sponsored streams.


38-51: Ensure correct handling of the sponsored flag.

The logic to determine if a stream is sponsored and bypass record parsing is implemented correctly.

Verify that the FlagSponsored is used consistently across related CLI commands.

x/streamer/client/cli/query_test.go (1)

36-38: Update CreateStream for sponsored stream testing.

The addition of the sponsored parameter in the CreateStream function is appropriate for testing the new functionality.

Ensure that test cases cover both sponsored and non-sponsored streams to verify the new functionality.

Verification successful

Test coverage for sponsored and non-sponsored streams is adequate.

The CreateStream function is tested with both Sponsored and NonSponsored parameters in various test files, ensuring the functionality is covered.

  • x/streamer/keeper/suite_test.go: Tests both sponsored and non-sponsored streams.
  • x/streamer/keeper/keeper_create_stream_test.go: Includes tests for both types of streams.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify test coverage for sponsored and non-sponsored streams.

# Test: Search for test cases related to `CreateStream`. Expect: Test cases for both sponsored and non-sponsored streams.
rg --type go 'CreateStream('

Length of output: 107


Script:

#!/bin/bash
# Description: Verify test coverage for sponsored and non-sponsored streams.

# Test: Search for test cases related to `CreateStream`. Expect: Test cases for both sponsored and non-sponsored streams.
rg --type go 'CreateStream\('

Length of output: 5210

x/streamer/keeper/keeper.go (3)

27-27: Addition of SponsorshipKeeper to Keeper struct.

The addition of the sk field of type types.SponsorshipKeeper to the Keeper struct is appropriate for managing sponsorship operations.


Line range hint 31-43: Update to NewKeeper function signature.

The NewKeeper function has been updated to include a SponsorshipKeeper parameter, aligning with the struct changes for sponsorship support.


53-70: Enhancement of CreateStream method with sponsorship logic.

The CreateStream method now supports sponsored streams by introducing a conditional path that queries the sponsorship distribution. The logic appears sound, and error handling is appropriately managed.

Ensure that all calls to CreateStream are updated to use the new method signature.

Verification successful

All CreateStream method calls updated to the new signature.

The search results confirm that all calls to the CreateStream method have been updated to include the sponsored parameter, matching the new method signature.

  • x/streamer/proposal_handler.go: Line showing CreateStream call with the sponsored parameter.
  • x/streamer/client/cli/query_test.go: Test cases using the updated method signature.
  • x/streamer/keeper/suite_test.go: Test cases for both non-sponsored and sponsored streams.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `CreateStream` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type go -A 5 $'CreateStream'

Length of output: 35168

x/streamer/types/proposal_stream.go (1)

Line range hint 32-41: Enhancement of NewCreateStreamProposal function with sponsorship parameter.

The addition of the sponsored parameter allows for the creation of stream proposals with sponsorship status. The logic correctly populates the Sponsored field in the CreateStreamProposal struct.

x/streamer/keeper/distribute.go (1)

95-106: Addition of sponsorship logic to distributeStream function.

The function now handles streams with sponsorship plans by querying the sponsorship distribution and updating the stream's distribution information. The logic is well-integrated, and error handling is appropriate.

x/streamer/keeper/genesis_test.go (2)

54-54: Ensure correct handling of the NonSponsored parameter.

The addition of the NonSponsored parameter to the CreateStream method aligns with the new functionality. Verify that all relevant test cases correctly handle this parameter.


Line range hint 89-109:
Verify alignment with new stream creation functionality.

Ensure that the initialization logic in TestStreamerInitGenesis aligns with the updated stream creation functionality, particularly regarding sponsorship.

x/streamer/keeper/suite_test.go (4)

23-25: Good use of constants for clarity.

The addition of Sponsored and NonSponsored constants enhances code readability and reduces potential errors related to boolean values.


77-88: Separation of concerns in stream creation.

The update to CreateStream and the addition of CreateSponsoredStream effectively separate the logic for sponsored and non-sponsored streams, improving clarity and maintainability.


132-160: Encapsulation of voting logic is well-implemented.

The encapsulation of voting logic in the vote method is a good practice, enhancing the test suite's ability to simulate complex interactions.


162-212: Well-structured methods for validator creation and delegation.

The CreateValidator and Delegate methods are well-structured and enhance the test suite's capabilities in simulating staking and delegation scenarios.

x/streamer/keeper/keeper_create_stream_test.go (3)

Line range hint 77-100:
Ensure correct handling of the NonSponsored parameter in tests.

The updates to include the NonSponsored parameter in CreateStream calls maintain test coverage for non-sponsored streams. Verify that all relevant test cases correctly handle this parameter.


204-211: Comprehensive test coverage for stream creation.

The updates to include the NonSponsored parameter in CreateStream calls ensure comprehensive test coverage for various stream creation scenarios.


213-285: Enhanced test coverage for sponsored streams.

The addition of the TestCreateSponsoredStream function ensures that sponsored streams are adequately tested, enhancing the test suite's comprehensiveness.

x/streamer/keeper/distribute_test.go (1)

92-294: Comprehensive test coverage for sponsored distribution.

The TestSponsoredDistribute function effectively tests various scenarios involving sponsored distributions. The structured approach with multiple test cases enhances readability and maintainability. Ensure that all edge cases are covered and consider adding comments to explain complex logic, if any.

app/keepers/keepers.go (2)

381-388: Integration of SponsorshipKeeper looks good.

The SponsorshipKeeper is correctly instantiated and assigned. Ensure that it is fully integrated into the application and that all necessary interactions with other modules are handled.


397-397: Update to StreamerKeeper approved.

The inclusion of SponsorshipKeeper in StreamerKeeper is appropriate for integrating sponsorship functionality. Verify that the new functionality is correctly implemented and tested.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Commits

Files that changed from the base of the PR and between 5f37893 and b805af3.

Files selected for processing (2)
  • app/keepers/keepers.go (1 hunks)
  • x/streamer/keeper/keeper_create_stream_test.go (4 hunks)
Files skipped from review as they are similar to previous changes (1)
  • app/keepers/keepers.go
Additional comments not posted (3)
x/streamer/keeper/keeper_create_stream_test.go (3)

Line range hint 75-98:
LGTM! Ensure correct usage of NonSponsored.

The addition of the NonSponsored parameter to the CreateStream method calls is consistent with the PR objectives. The test logic correctly reflects this change.


202-202: LGTM! Ensure correct usage of NonSponsored.

The addition of the NonSponsored parameter to the CreateStream method calls is consistent with the PR objectives. The test cases correctly reflect this change.


211-282: Well-designed test cases for sponsored streams.

The new test function TestCreateSponsoredStream includes comprehensive test cases covering various scenarios for sponsored streams. This enhances the test coverage significantly.

Copy link
Contributor

@danwt danwt left a comment

Choose a reason for hiding this comment

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

Approved but could be tidied

x/streamer/keeper/distribute.go Show resolved Hide resolved
x/streamer/keeper/suite_test.go Show resolved Hide resolved
x/streamer/types/distr_info.go Show resolved Hide resolved
x/streamer/types/distr_info.go Show resolved Hide resolved
x/streamer/types/distr_info.go Show resolved Hide resolved
@keruch keruch merged commit 5933d5c into main Aug 12, 2024
111 of 116 checks passed
@keruch keruch deleted the kirill/streamer-sponsorship branch August 12, 2024 18:45
@keruch keruch mentioned this pull request Aug 12, 2024
29 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:sponsorship x/sponsorship module dym-internal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants