-
Notifications
You must be signed in to change notification settings - Fork 79
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
Automatic AMO rebalances #2291
Automatic AMO rebalances #2291
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2291 +/- ##
==========================================
- Coverage 53.26% 53.03% -0.24%
==========================================
Files 79 79
Lines 4089 4107 +18
Branches 1074 1080 +6
==========================================
Hits 2178 2178
- Misses 1908 1926 +18
Partials 3 3 ☔ View full report in Codecov by Sentry. |
|
||
/// @dev information regarding the trading volume of the rebalancer | ||
struct RebalancerVolumeTally { | ||
uint256 startBlock; |
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.
Nit: Could make both uint128 and fit in one slot. Block number and swap volume are not really gonna overflow
*/ | ||
modifier onlyRebalancer() { | ||
require( | ||
msg.sender == IVault(vaultAddress).strategistAddr() || |
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.
There's no rebalancer check here?
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.
thanks fixed: 81289d2
) external nonReentrant onlyRebalancer { | ||
uint256 volumeRebalancedCurrentPeriod = _amountToSwap; | ||
|
||
// no rebalance transaction from the rebalancer in the last hour |
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.
We can probably change it so that we are doing it as regenerative rate limit instead. Let me think and get back to this in a bit
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.
I guess the implementation could be a linear decay of the tallied swap amounts. Meaning:
- there is a certain threshold that musn't be surpassed
- the function keeps tally of the amount that has been swapped and each block decays (threshold / 900). (might make sense to make the decay a 3 hour thing instead of 1 to allow for some variance in the defender action)
Will implement this tomorrow and see if it can be done so it doesn't increase the complexity of the code too much
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.
Yeah, what I was thinking was basically something like (currentTime - lastRebalanceTime) * regenerationAmountPerSecond
. We can probably have an upper limit as well (say 500 WETH per rebalance). This will ensure that you cannot rebalance more than 500 WETH per hour. This will also limit the operator/rebalancer from doing a rebalance back to back for any reason
require(volumeRebalancedCurrentPeriod <= rebalancerVolumeLimit, "Amount to swap exceeds limit"); | ||
rebalancerVolumeTally.volumeRebalaced = volumeRebalancedCurrentPeriod; | ||
|
||
_rebalance(_amountToSwap, _swapWeth, _minTokenReceived); |
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.
Would be great to ensure that minTokenReceived isn't some crazy low amount
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.
good point I guess we could enforce it to be 1bp below swapping amount. Since the automatic rebalancing shouldn't work outside the [-1,0] tick. And by work I mean start and end point being the tick
Adds a new rebalance function which a permissioned account is able to call to automatically rebalance a limited volume within a time-frame. The limited volume and rebalancer account is specified by the govenor.
Code Change Checklist
To be completed before internal review begins:
Internal review:
Deploy checklist
Two reviewers complete the following checklist: