Skip to content

feat: univ3 unhedged#28

Open
fp-crypto wants to merge 48 commits intomasterfrom
fp-univ3
Open

feat: univ3 unhedged#28
fp-crypto wants to merge 48 commits intomasterfrom
fp-univ3

Conversation

@fp-crypto
Copy link
Owner

No description provided.

fp-crypto and others added 21 commits March 18, 2022 11:19
* fix: renamed SOLID specific tests and added run_check for dex

* fix: necessary changes to setup the UniV3Joint

* fix: set pool state variable during initialize

* fix: add collection call to the burnLP function and couple of return values

* fix: fixed return values

* fix: fixed call to estimated provider assets from joint

* chore: included hardhat chain id in eth addresses

* feat: test harvesting to open and close positions

* fix: re-use getReward function collecting the owed tokens
* feat: test lossy harvest swapping both from token A and token B

* chore: rename harvesting test file

* feat: swap values based on pool reserves

* feat: included choppy harvest test function

* feat: choppy harvest passing

* feat: added percentage reserve movement

* feat: uni_v3_pool fixture reading abi from USDC-WETH pool

* feat: added views for tick and liquidity stuff

* feat: simulate swap fixture

* feat: added limit price option to sell token

* feat: function testing price moving 1 tick left and right

* feat: testing multiple tick movement outside the provided range

* fix: increased RATIO_PRECISION for lower profit values

* fix: change percentageMaxLoss based on current value

* feat: new univ3 helper fuctions to buy token and empty pool reserve of one token

* fix: fixed choppy harvest to ensure wanted trades are always < than available reserves

* feat: added empty pool test

* chore: remove debugging events

* chore: remove debugging estimating function

* feat: added uni v3 function to rebelance pool between tests

* feat: added RATIO_PRECISION fixture

* feat: added CRV swap test parameter

* feat: added checkAllowance function

* feat: added option to rebalance using CRV pool instead of uniswap pool

* chore: rename library from SimulateSwap to UniswapHelperViews

* chore: create testing library for views only used to created testing scenarios and remove them from uniswap views library

* chore: renamed getFeeGrowth view to getEeesEarned

* fix: removed variabls used only once to optimize

* chore: added license information for interfaces

* chore: documentation for all functions + added liquidity check when burning LP

* feat: added security functions tomanually burn the LP and collect rewards specifying the min and max ticks and set the min and max ticks manually

* chore: added comments to libraries part

* chore: renamed WETH to referenceToken

* chore: add natspec documentation

* feat: return value for CRV swap

* feat: ensure lossy scenarios when swapping through CRV by reducing reserves

* fix: tests passing

* chore: change libraries name

* chore: change useUnipool to useCRVPool

* feat: renamed to UniV3StablesJoint

* feat: make CRV swaps work with any token

* feat: included setTicksFromCurrent function

* chore: tests cleanup

* fix: change tests to ensure they run with other decimals and tighter pools

* feat: included clone + migration tests
@fp-crypto
Copy link
Owner Author

#32 (comment)

Copy link
Collaborator

@jmonteer jmonteer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks pretty good.

still some deep diving to do... mainly in libraries.

* contract
*/
function getReward() internal override {
IUniswapV3Pool(pool).collect(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need to update the position first so fees are accumulated in tokensOwed0 and 1?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally yes but I have not found an external function in Uni to update it so the function is always used after a position updating call (burnLP for example) and we need to include a burnLP(0) (does not revert, already tested) in the call to collect rewards manually. I propose also to change the name to collectOwedTokens as this is used both to collect rewards and principal

@16slim
Copy link
Collaborator

16slim commented May 18, 2022

Implemented main comments from j in https://github.com/fp-crypto/joint-strategy/pull/34/files

An additional comment that I have is that the swapTokenForTokenManually is declared in the Joint.sol but it's an empty implementation so we still need to finish that one, shouldn't be very complex as the swap internal function is already working

@16slim
Copy link
Collaborator

16slim commented May 19, 2022

Changes implemented here: #34

@16slim 16slim mentioned this pull request May 19, 2022
* fix: refactor swapRewardTokens as per @jmonteer comment

* fix: removed else case in findSwapTo as it wasn´t adding any value

* fix: gas savings as per @jmonteer comments

* fix: change modifier of setCRVPool to onlyGovernance

* fix: minor gas savings

* fix: convert pool to address

* chore: change getReward for collectOwedTokens and burn 0 LP before harvesting the rewards manually

* fix: minor gas saving

* feat: add revert if CRV index is not found

* fix: comment typo

* fix: comment typo

* chore: removed unused line

* fix: turn harvest and harvestTrigger virtual so any inheriting Joint can override it

* fix: change findSwapTo argument to from_token

* fix: re-organized the burn + collect functions

* fix: remove setCRVPool function to avoid address injection

* fix: added force parameter to setTicksManually

* fix: edited the if condition and natspec on setTicksManually

* fix: re-factor findSwapTo

* feat: implemented swapTokenForTokenManually

* chore: minor changes to tests to make them more robust

* feat: new test for the manual operation of the strategy

* fix: remove modifier and empty implementation from swap manually
@jmonteer
Copy link
Collaborator

LGTM

Copy link
Collaborator

@jmonteer jmonteer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


uint256[] memory _pendingRewards = pendingRewards();

if (_pendingRewards[0] >= _minRewardToHarvest * (10**IERC20Extended(rewardTokens[0]).decimals()) / RATIO_PRECISION &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I would replace && with || here but I see it would be very rare to see to have rewards only on one side

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants