[TRA-416] Set market mapper revenue share on market creation #1755
Conversation
Signed-off-by: Shrenuj Bansal <[email protected]>
WalkthroughThe changes primarily focus on updating various files across the Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant PricesKeeper as PricesKeeper
participant RevShareKeeper as RevShareKeeper
participant Test as Test Functions
App ->> PricesKeeper: Initialize with new arguments
App ->> RevShareKeeper: Initialize and pass as argument
PricesKeeper ->> RevShareKeeper: CreateNewMarketRevShare
Test ->> PricesKeeper: Call PricesKeepers with additional parameters
Poem
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 Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Outside diff range and nitpick comments (1)
protocol/testutil/keeper/prices.go (1)
[!TIP]
Codebase VerificationMissing definition for
createRevShareKeeperThe function
createRevShareKeeperreferenced inprotocol/testutil/keeper/prices.gois not defined or imported anywhere in the codebase. This will likely cause runtime errors.
protocol/testutil/keeper/prices.go: Line 41 - Reference tocreateRevShareKeeperwithout a corresponding definition.Please ensure that
createRevShareKeeperis properly defined and imported to avoid runtime issues.Analysis chain
Line range hint
62-94: Ensure initialization of all components increatePricesKeeper.The function now accepts a
revShareKeeper, which should be properly initialized to ensure it does not cause runtime errors during tests. Consider verifying if all dependencies are correctly set up before use.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check initialization of revShareKeeper in test setups. rg --type go 'createRevShareKeeper' protocol/testutil/keeper/Length of output: 163
Script:
#!/bin/bash # Locate the implementation of createRevShareKeeper function rg --type go 'func createRevShareKeeper'Length of output: 42
Script:
#!/bin/bash # Search for the term createRevShareKeeper in the entire codebase to locate its definition or related information rg 'createRevShareKeeper'Length of output: 127
Script:
#!/bin/bash # Search for any import statements or dynamic definitions related to createRevShareKeeper rg 'createRevShareKeeper' --glob '!protocol/testutil/keeper/prices.go'Length of output: 70
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (30)
- protocol/app/app.go (1 hunks)
- protocol/app/prepare/prepare_proposal_test.go (2 hunks)
- protocol/app/process/full_node_process_proposal_test.go (1 hunks)
- protocol/app/process/market_prices_test.go (3 hunks)
- protocol/app/process/process_proposal_test.go (1 hunks)
- protocol/app/process/transactions_test.go (4 hunks)
- protocol/testutil/keeper/assets.go (2 hunks)
- protocol/testutil/keeper/clob.go (1 hunks)
- protocol/testutil/keeper/perpetuals.go (1 hunks)
- protocol/testutil/keeper/prices.go (4 hunks)
- protocol/testutil/keeper/rewards.go (2 hunks)
- protocol/testutil/keeper/sending.go (1 hunks)
- protocol/testutil/keeper/subaccounts.go (2 hunks)
- protocol/x/prices/genesis_test.go (5 hunks)
- protocol/x/prices/keeper/grpc_query_market_test.go (5 hunks)
- protocol/x/prices/keeper/keeper.go (3 hunks)
- protocol/x/prices/keeper/keeper_test.go (1 hunks)
- protocol/x/prices/keeper/market.go (1 hunks)
- protocol/x/prices/keeper/market_param_test.go (5 hunks)
- protocol/x/prices/keeper/market_price_test.go (5 hunks)
- protocol/x/prices/keeper/market_test.go (5 hunks)
- protocol/x/prices/keeper/msg_server_create_oracle_market_test.go (2 hunks)
- protocol/x/prices/keeper/msg_server_update_market_param_test.go (2 hunks)
- protocol/x/prices/keeper/msg_server_update_market_prices_test.go (4 hunks)
- protocol/x/prices/keeper/slinky_adapter_test.go (5 hunks)
- protocol/x/prices/keeper/update_price_test.go (1 hunks)
- protocol/x/prices/keeper/validate_market_price_updates_test.go (4 hunks)
- protocol/x/prices/module.go (1 hunks)
- protocol/x/prices/module_test.go (3 hunks)
- protocol/x/prices/types/expected_keepers.go (2 hunks)
Files not reviewed due to errors (3)
- protocol/x/prices/keeper/keeper.go (no review received)
- protocol/app/process/full_node_process_proposal_test.go (no review received)
- protocol/app/prepare/prepare_proposal_test.go (no review received)
Files skipped from review due to trivial changes (4)
- protocol/x/prices/keeper/grpc_query_market_test.go
- protocol/x/prices/keeper/market_param_test.go
- protocol/x/prices/keeper/slinky_adapter_test.go
- protocol/x/prices/keeper/validate_market_price_updates_test.go
Additional comments not posted (27)
protocol/x/prices/keeper/keeper_test.go (1)
11-11: Ensure the new parameter is properly documented and utilized.The change in the
keepertest.PricesKeepers(t)function call now includes additional parameters to accommodate the newrevShareKeeper. Ensure that this change is reflected in the documentation and that therevShareKeeperis properly utilized within the test setup.protocol/x/prices/genesis_test.go (3)
54-54: Approve the addition of the underscore parameter inPricesKeepers.The inclusion of an underscore parameter in the
PricesKeepersfunction call correctly reflects the updated function signature with the newrevShareKeeperparameter. Adding a comment to explain the use of the underscore as a placeholder would enhance clarity.
98-98: Approve the addition of the underscore parameter inPricesKeepers.The inclusion of an underscore parameter in the
PricesKeepersfunction call correctly reflects the updated function signature with the newrevShareKeeperparameter. Adding a comment to explain the use of the underscore as a placeholder would enhance clarity.
108-108: Approve the addition of the underscore parameter inPricesKeepers.The inclusion of an underscore parameter in the
PricesKeepersfunction call correctly reflects the updated function signature with the newrevShareKeeperparameter. Adding a comment to explain the use of the underscore as a placeholder would enhance clarity.protocol/testutil/keeper/sending.go (1)
60-60: Confirm the use ofnilforrevShareKeeperin test setup.The addition of
nilas therevShareKeeperargument increatePricesKeepermight be intended for testing. Confirm if this is a temporary placeholder or part of the final implementation.Verification successful
Confirmed: Consistent use of
nilforrevShareKeeperin test setups.The usage of
nilas therevShareKeeperargument in thecreatePricesKeeperfunction is consistent across multiple test setup files. This suggests that it is an intentional placeholder or not required for these tests.
protocol/testutil/keeper/assets.goprotocol/testutil/keeper/rewards.goprotocol/testutil/keeper/subaccounts.goprotocol/testutil/keeper/sending.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Confirm the usage of `nil` for `revShareKeeper` in test setups across the codebase. # Test: Search for the usage of `nil` for `revShareKeeper` in test setups. Expect: Consistent usage of `nil` or a valid object across similar setups. rg --type go $'createPricesKeeper(.+nil)'Length of output: 597
protocol/x/prices/keeper/update_price_test.go (1)
126-126: LGTM!The addition of an extra underscore parameter in
PricesKeepersaligns with the updated function signatures across the codebase.protocol/testutil/keeper/subaccounts.go (1)
56-56: Confirm the use ofnilforrevShareKeeperin test setup.The addition of
nilas therevShareKeeperargument increatePricesKeepermight be intended for testing. Confirm if this is a temporary placeholder or part of the final implementation.protocol/x/prices/keeper/market_price_test.go (6)
28-28: Approved the addition of the revShareKeeper parameter in the test setup.This change aligns with the PR's objective to integrate revenue sharing logic into the market creation process.
63-63: Approved the addition of the revShareKeeper parameter in the test setup.This change is necessary to ensure the new functionality is covered in the tests.
92-92: Approved the addition of the revShareKeeper parameter in the test setup.Ensures that the tests are consistent with the new architecture.
107-107: Approved the addition of the revShareKeeper parameter in the test setup.This update is crucial for maintaining the integrity of the test environment with the new system architecture.
113-113: Approved the addition of the revShareKeeper parameter in the test setup.This ensures that the test setup reflects the changes in the main codebase.
129-129: Approved the addition of the revShareKeeper parameter in the test setup.This change is consistent with the PR's goal to integrate revenue sharing logic, ensuring that the test environment matches the production code.
protocol/x/prices/module.go (1)
104-122: Approved the integration of the RevShareKeeper into the AppModule structure.The addition of
RevShareKeeperto theAppModulestructure and its constructor functionNewAppModuleis well-aligned with the PR's objectives to handle revenue sharing. This change is necessary for the new functionality to be effectively integrated into the module.protocol/app/process/market_prices_test.go (3)
68-68: Approved the addition of the revShareKeeper parameter in the test setup.This update is essential for ensuring that the test environment accurately reflects the changes made to the main codebase regarding the integration of revenue sharing logic.
123-123: Approved the addition of the revShareKeeper parameter in the test setup.This ensures that the test setup is consistent with the new system architecture, which includes the revenue sharing logic.
165-165: Approved the addition of the revShareKeeper parameter in the test setup.This change is crucial for maintaining the integrity of the test environment with the new system architecture.
protocol/x/prices/keeper/msg_server_update_market_param_test.go (1)
147-147: Approved the addition of the revShareKeeper parameter in the test setup.This change ensures that the test setup reflects the new architecture and is crucial for testing the updated functionality related to market parameter management.
protocol/x/prices/keeper/market_test.go (1)
61-64: Approve new functionality testing for market revenue share creation.The test checks the retrieval of market revenue share details, which is a direct reflection of the new functionality introduced. This is a crucial test to ensure the
revShareKeeperintegration works as expected.protocol/testutil/keeper/prices.go (1)
Line range hint
29-54: Enhancement: Improved test setup withrevShareKeeper.The function
PricesKeepershas been extended to includerevShareKeeper, which is consistent with the modifications in the keeper's structure. This ensures that all tests will have access to the required keepers, reflecting the new architectural changes.protocol/testutil/keeper/clob.go (1)
82-88: Approved Change: Addition ofnilparameter tocreatePricesKeeperfunction call.The added
nilparameter aligns with the introduction of therevShareKeeperin the broader system, ensuring consistency across the codebase.protocol/app/process/process_proposal_test.go (1)
261-261: Approved Change: Updated function call to include an additional underscore parameter.This change is necessary to accommodate the new
revShareKeeperparameter, ensuring that the function signature is consistent across the codebase.protocol/app/process/transactions_test.go (1)
111-111: Approved Changes: Updated function calls to include an additional underscore parameter.These changes are necessary to accommodate the new
revShareKeeperparameter, ensuring that the function signature remains consistent across the codebase, including in test setups.Also applies to: 191-191
protocol/x/prices/keeper/msg_server_update_market_prices_test.go (4)
189-189: Approved the addition of the new keeper parameter.The addition of an additional return value
_fromPricesKeepersaligns with the PR's changes. It's good practice to use a placeholder for unused variables to maintain clarity.
315-315: Approved the addition of the new keeper parameter.The addition of an additional return value
_fromPricesKeepersis consistent with the integration of the newrevShareKeeperfunctionality. Using a placeholder for unused variables is a clean approach to handle changes in function signatures.
379-379: Approved the addition of the new keeper parameter.The addition of an additional return value
_fromPricesKeepersis consistent with the integration of the newrevShareKeeperfunctionality. Using a placeholder for unused variables is a clean approach to handle changes in function signatures.
402-402: Approved the addition of the new keeper parameter.The addition of an additional return value
_fromPricesKeepersis consistent with the integration of the newrevShareKeeperfunctionality. Using a placeholder for unused variables is a clean approach to handle changes in function signatures.
Signed-off-by: Shrenuj Bansal <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- protocol/x/prices/types/expected_keepers.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- protocol/x/prices/types/expected_keepers.go
Signed-off-by: Shrenuj Bansal <[email protected]>
Signed-off-by: Shrenuj Bansal <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- protocol/app/app.go (2 hunks)
- protocol/app/simulation_test.go (2 hunks)
- protocol/testutil/app/app.go (3 hunks)
- protocol/testutil/keeper/assets.go (2 hunks)
- protocol/testutil/keeper/clob.go (1 hunks)
- protocol/testutil/keeper/perpetuals.go (1 hunks)
- protocol/testutil/keeper/revshare.go (1 hunks)
- protocol/testutil/keeper/rewards.go (2 hunks)
- protocol/testutil/keeper/sending.go (1 hunks)
- protocol/testutil/keeper/subaccounts.go (2 hunks)
Files skipped from review as they are similar to previous changes (7)
- protocol/app/app.go
- protocol/testutil/keeper/assets.go
- protocol/testutil/keeper/clob.go
- protocol/testutil/keeper/perpetuals.go
- protocol/testutil/keeper/rewards.go
- protocol/testutil/keeper/sending.go
- protocol/testutil/keeper/subaccounts.go
Additional comments not posted (4)
protocol/app/simulation_test.go (1)
138-138: Addition ofrevsharetypes.ModuleNameto Genesis ModulesThe inclusion of
revsharetypes.ModuleNamein the genesis module order is a crucial update for ensuring that the new revenue sharing functionality is integrated into the simulation tests. This change is appropriate and aligns with the broader updates made across the system to support revenue sharing features.protocol/testutil/app/app.go (3)
18-19: Added import for revshare types.The addition of
revsharetypesfromgithub.aaakk.us.kg/dydxprotocol/v4-chain/protocol/x/revshare/typesis aligned with the PR's focus on integrating revenue sharing features. This change is necessary to support the types used in the modifications related to revenue sharing.
207-208: Updated GenesisStates type to include revsharetypes.GenesisState.This change integrates the
revsharetypes.GenesisStateinto theGenesisStatestype, which is crucial for ensuring that therevsharetypesmodule can participate in genesis state handling. This modification is necessary for initializing and updating the genesis state with revenue sharing data, aligning with the PR's objectives to enhance market creation features with revenue sharing capabilities.
264-265: Added case for revsharetypes.GenesisState in UpdateGenesisDocWithAppStateForModule.Adding a case for
revsharetypes.GenesisStatein theUpdateGenesisDocWithAppStateForModulefunction is essential for handling the initialization and updates of the revenue sharing module's genesis state. This ensures that any modifications or initializations specific to the revenue sharing features are processed correctly during the app's genesis state setup.
Signed-off-by: Shrenuj Bansal <[email protected]>
Signed-off-by: Shrenuj Bansal <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- protocol/app/app.go (3 hunks)
- protocol/x/prices/keeper/market_test.go (5 hunks)
Files skipped from review as they are similar to previous changes (2)
- protocol/app/app.go
- protocol/x/prices/keeper/market_test.go
Changelist
Test Plan
Added tests
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores