-
Notifications
You must be signed in to change notification settings - Fork 0
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: configure rewards controller && update init params #64
base: staging
Are you sure you want to change the base?
Conversation
✅ Linked to Story DEV-585 · VPND LM - Unlocking period |
WalkthroughThe changes involve updating various facets and utilities within a Solidity-based smart contract system. The most significant inclusions are the Changes
Poem
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 (
|
@mejiasd3v We Need to discuss about Tier setup and update the contracts. |
@royvardhan could you please fix the CI? I think it was broken before but please have a look if its an easy fix |
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 (1)
- src/upgradeInitializers/DiamondInit.sol (3 hunks)
Additional comments not posted (3)
src/upgradeInitializers/DiamondInit.sol (3)
39-39
: Addition ofrewardsController
parameter toArgs
struct looks good.
155-155
: Initialization ofrewardsControllerAddress
looks good.
153-155
: The overall structure and logic of theinit
function, including the newrewardsControllerAddress
, are sound.
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: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/facets/DiamondManagerFacet.sol (2 hunks)
- src/libraries/LStratosphere.sol (2 hunks)
- test/foundry/Facets/UnlockFacet.t.sol (3 hunks)
- test/foundry/utils/DiamondTest.sol (2 hunks)
Additional comments not posted (8)
src/libraries/LStratosphere.sol (1)
6-6
: Refactor: Import statement for IRewardsController.Ensure that the imported interface is used properly within the library. This import is crucial for the updated logic in the
getDetails
function.test/foundry/Facets/UnlockFacet.t.sol (4)
11-12
: Code Cleanup: Import statement for RewardsControllerMock.The import is correctly placed and necessary for the mock's usage in tests. This setup is essential for simulating the behavior of the rewards controller in a test environment.
19-19
: Initialization ofRewardsControllerMock
.The mock is properly initialized, ensuring that the rewards controller behavior can be simulated during tests. This is crucial for testing the integration of the rewards controller with the UnlockFacet.
34-34
: Correctness: Constant declaration forCOOLDOWN_PERIOD
.The constant is defined correctly and used in subsequent test logic to simulate time-based conditions. This is a good practice for making tests readable and maintainable.
56-56
: Ensure proper usage ofsetRewardControllerAddress
.This line sets the mock address for the rewards controller, which is crucial for ensuring that the facet interacts correctly with the controller in the test environment. Verify that this setup is consistent with the intended test scenarios.
test/foundry/utils/DiamondTest.sol (1)
Line range hint
181-212
: Refactor: Updated function selectors for DiamondManagerFacet.The addition of new function selectors to handle the increased functionality of the
DiamondManagerFacet
is crucial for ensuring that all facet functions are correctly registered in the diamond. This change is necessary due to the introduction of new functions in the facet.src/facets/DiamondManagerFacet.sol (2)
240-246
: Enhancement: Addition ofsetRewardControllerAddress
function.This function is a critical addition, allowing for dynamic configuration of the rewards controller address. It includes a check for a zero address, which is a good practice to prevent misconfiguration. Ensure thorough testing of this function to confirm it behaves as expected under various conditions.
376-378
: Correctness: Implementation ofgetUnlockDiscount
.This getter function provides access to unlock discounts based on tiers. It's a straightforward implementation that should function correctly provided that the state variable
unlockTimestampDiscountForStratosphereMembers
is properly managed elsewhere in the contract.
IRewardsController _rewardsController = IRewardsController(s.rewardsControllerAddress); | ||
tier = _rewardsController.tierOf(_tokenId); // Revert if rewardsControllerAddress is not set |
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.
Potential Issue: Ensure rewardsControllerAddress
is correctly initialized.
The code now depends on rewardsControllerAddress
being properly set. If this address is not correctly initialized, the call to tierOf
will fail and revert the transaction. Consider adding a check to ensure that rewardsControllerAddress
is not zero before making the call.
- tier = _rewardsController.tierOf(_tokenId); // Revert if rewardsControllerAddress is not set
+ if (s.rewardsControllerAddress == address(0)) revert("RewardsController address not set");
+ tier = _rewardsController.tierOf(_tokenId);
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
IRewardsController _rewardsController = IRewardsController(s.rewardsControllerAddress); | |
tier = _rewardsController.tierOf(_tokenId); // Revert if rewardsControllerAddress is not set | |
IRewardsController _rewardsController = IRewardsController(s.rewardsControllerAddress); | |
if (s.rewardsControllerAddress == address(0)) revert("RewardsController address not set"); | |
tier = _rewardsController.tierOf(_tokenId); |
Summary by CodeRabbit
New Features
setRewardControllerAddress
andgetUnlockDiscountForStratosphereMember
functions in DiamondManagerFacet.IRewardsController
interface for retrieving member tiers.Enhancements
rewardsControllerAddress
for better rewards management.DiamondInit
struct with newrewardsController
field and updated discount arrays.Tests
RewardsControllerMock
for extensive testing.Refactor