Chore: Automate end start swapping #235
Conversation
Codecov Report
@@ Coverage Diff @@
## main #235 +/- ##
==========================================
+ Coverage 98.90% 98.94% +0.04%
==========================================
Files 13 13
Lines 456 475 +19
Branches 108 114 +6
==========================================
+ Hits 451 470 +19
Misses 5 5
Continue to review full report at Codecov.
|
| uint256 blockTime = 5 minutes; | ||
| bool isSwapEnabled = lbp.getSwapEnabled(); | ||
| (startTime, , ) = lbp.getGradualWeightUpdateParams(); | ||
| require( |
contracts/lbp/LBPManager.sol
Outdated
| (startTime, , ) = lbp.getGradualWeightUpdateParams(); | ||
| require( | ||
| !isSwapEnabled && state == TaskState.Waiting, | ||
| "LBPManager: swapping already active" |
There was a problem hiding this comment.
if state is Ended, message would be wrong
There was a problem hiding this comment.
Should refer to "enabled" OR "disabled", not "active" (state terminology is internal to the contract)
There was a problem hiding this comment.
I don't think we should revert if !isSwapEnabled. Should just set the state to Started and return.
There was a problem hiding this comment.
I have implemented the changes you requested.
contracts/lbp/LBPManager.sol
Outdated
| ); | ||
| require( | ||
| block.timestamp > startTime - blockTime, | ||
| "LBPManager: Only once block before start time" |
There was a problem hiding this comment.
incorrect grammar and unclear. should be "LBPManager: More than one block before start time"?
There was a problem hiding this comment.
improved the error string
contracts/lbp/LBPManager.sol
Outdated
| (, endTime, ) = lbp.getGradualWeightUpdateParams(); | ||
| require( | ||
| isSwapEnabled && state == TaskState.Started, | ||
| "LBPManager: swapping already active" |
There was a problem hiding this comment.
Should be "LBPManager: swapping is not enabled"
There was a problem hiding this comment.
anyway, I don't think we should revert if isSwapEnabled. Should just set the state to Ended and return.
There was a problem hiding this comment.
I have implemented the changes you requested.
contracts/lbp/LBPManager.sol
Outdated
| ); | ||
| require( | ||
| block.timestamp > endTime, | ||
| "LBPManager: Only once block before start time" |
There was a problem hiding this comment.
should be >=
incorrect grammar and unclear. should be "LBPManager: haven't reached endTime"?
There was a problem hiding this comment.
I have implemented the changes you requested.
contracts/lbp/LBPManager.sol
Outdated
| @@ -302,6 +349,7 @@ contract LBPManager { | |||
| * @param _swapEnabled Enables/disables swapping. | |||
| */ | |||
| function setSwapEnabled(bool _swapEnabled) external onlyAdmin { | |||
| require(TaskState.Ended != state, "LBPManager: LBP ended"); | |||
There was a problem hiding this comment.
Why? you can only call this function if the LBP is ended? For what purpose?
There was a problem hiding this comment.
You can call this method only if the LBP is not ended. So that nobody can restart swapping after end time.
There was a problem hiding this comment.
Improved the condition, and is now not dependent on state.
| state = TaskState.Ended; | ||
| lbp.setSwapEnabled(false); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Exit pool or remove liquidity from pool. |
There was a problem hiding this comment.
documentation for removeLiquidity?
There was a problem hiding this comment.
The documentation on the line linked to this comment. "Exit pool or remove liquidity from pool."
There was a problem hiding this comment.
That documentation is completely wrong for setSwapEnabled
There was a problem hiding this comment.
hm, This documentation is for removeLiquidity. The lines between 286-349 are hidden due to no changes. The correct docs for setSwapEnabled starts at line 347.
…pEnabled if required
contracts/lbp/LBPManager.sol
Outdated
| ); | ||
| require( | ||
| block.timestamp >= startTimeEndTime[1], | ||
| "LBPManager: haven't reached end time" |
There was a problem hiding this comment.
| "LBPManager: haven't reached end time" | |
| "LBPManager: awaits end time" |
contracts/lbp/LBPManager.sol
Outdated
| require(state == TaskState.Waiting, "LBPManager: already started"); | ||
| require( | ||
| block.timestamp > startTimeEndTime[0] - buffer, | ||
| "LBPManager: one block before start time" |
There was a problem hiding this comment.
| "LBPManager: one block before start time" | |
| "LBPManager: awaits start time" |
closes #234