Skip to content

Conversation

ncitron
Copy link
Contributor

@ncitron ncitron commented Aug 3, 2021

No description provided.

Copy link
Contributor

@0xSachinK 0xSachinK left a comment

Choose a reason for hiding this comment

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

Diff between existing FlexibleLeverageStrategyExtension and AaveFLIStrategyExtension: https://gist.github.com/Alpha-Guy/1f9463f4c729e2467b98086c2b1eb001

I see that the diff is mainly due to changes in the calculation of position units and their decimal places. Would be great if you can explain the changes in the PR description or within the contract as comments.

Copy link
Contributor

@richardliang richardliang left a comment

Choose a reason for hiding this comment

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

Diff is here

I think we should adjust the base units in the _calculateChunkRebalanceNotional function, so we dont have to add .div(10 ** (18 - strategy.xxx) to every calculate units function


/**
* Derive the min repay units from collateral units for delever. Units are calculated as target collateral rebalance units multiplied by slippage tolerance
* and pair price (collateral oracle price / borrow oracle price). Output is measured to 18 decimals.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is confusing if output is 18 decimals, while units in other contracts all account for decimals

Copy link
Contributor

@0xSachinK 0xSachinK left a comment

Choose a reason for hiding this comment

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

Latest diff

@ncitron
Copy link
Contributor Author

ncitron commented Aug 17, 2021

Closing in favor of #72

@ncitron ncitron closed this Aug 17, 2021
ace3 pushed a commit to ace3/index-coop-smart-contracts that referenced this pull request Feb 2, 2024
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