-
Notifications
You must be signed in to change notification settings - Fork 49
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
Multiple rewards Liquidity Mining #377
base: development
Are you sure you want to change the base?
Multiple rewards Liquidity Mining #377
Conversation
…ards feat: new liquidity mining that rewards in multiple tokens
Slither fixes on LM
…eposited-to-V2 feat:allow lending with LM V2 during migration period
…ests ci: increased nodejs memory so it can run the tests properly on node 14.x
address _migrator, | ||
IERC20 _SOV | ||
) external onlyAuthorized { | ||
/// @dev Non-idempotent function. Must be called just once. |
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.
@py-ro we should add a check to prevent initialize
to be called twice ... in the previous version this check was put in place
require(address(SOV) == address(0), "Already initialized");
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.
👍
contracts/farm/LiquidityMiningV2.sol
Outdated
function initialize( | ||
address _wrapper, | ||
address _migrator, | ||
IERC20 _SOV |
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.
this parameter is never used I guess ... let's double check just to be sure
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.
you are right, it is never used
contracts/farm/LiquidityMiningV1.sol
Outdated
function finishMigrationGracePeriod() external onlyAuthorized onlyBeforeMigrationGracePeriodFinished { | ||
require(migrationGracePeriodState == MigrationGracePeriodStates.Started, "Migration hasn't started yet"); |
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.
The require
introduces a harder contraint making the onlyBeforeMigrationGracePeriodFinished
worthless ... as suggested by Haku we might create a new modifier onlyMigrationGracePerioidStarted
that checks the same condition as the require
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.
👍
fix: LM contracts initialize only once
this could be useful in the future, hence leaving it open |
Multiple rewards Liquidity Mining
Goal
A new liquidity mining contract will be deployed that will let users to get multiple tokens in exchange of staking LP Tokens as opposed to the current version which only rewards
SOV
:Implementation
Upgrading the original
LiquidityMining.sol
contract would have been extremely complex the storage layout would require lots of changes. In order to make users experience smoother and not to make them to actively move their funds to the new contract and pay for the migration trnasaction fees, the following plan is proposed:LiquidityMiningV2
.LiquidityMining
contract with a new version that contains a new set of capabilities:a. Allows admins to stop some operations (see
3
)b. Exposes a new interface to extract some data from it's state
stopMining
LiquidityMining.sol
contract will be locked the users will not be able to neither withdraw or claim rewards.LiquidityMiningV2
that will:a. Copy the original pools and rewards configurations into the new liquidity mining contract
b. Move the remaining
SOV
andLP Tokens
stored inLiquidityMiningV1
Deployment steps
LiquidityMining
implementation byLiquidityMiningV1
LiquidityMiningV2
LMV1toLMV2Migrator
asLiquidityMiningV2
andLiquidityMiningV1
adminLiquidityMiningV1
deposits