Skip to content

Conversation

@ultrasecreth
Copy link
Contributor

  • Small test improvements
  • Implement maxFYTokenIn
  • WIP maxFYTokenOut
  • maxFYTokenOut working
  • maxSharesIn working
  • liquidity methods added to the Pool
  • Add tests for liquidity methods at the pool level, fix bug on sellFYTokenPreview

@ultrasecreth ultrasecreth requested review from alcueca and devtooligan and removed request for devtooligan August 13, 2022 10:31
@ultrasecreth ultrasecreth changed the title Liquidity functions WIP: Liquidity functions Aug 13, 2022
@ultrasecreth ultrasecreth changed the title WIP: Liquidity functions Liquidity functions Aug 13, 2022
Comment on lines +194 to +195
// I should have got the max (rounding error allowed)
assertEq(asset.balanceOf(bob), bobAssetBefore + maxBaseOut - 1);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this expected?, I've been thinking about it and it may be a problem for someone wanting to buy an exact amount

Copy link
Contributor

@devtooligan devtooligan left a comment

Choose a reason for hiding this comment

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

i went through the logic pretty deeply, i think it loos good.

some nit stuff on comments and a few questions to think about.

marco is gonna review the math as well.

i want to spend some time w the tests which i will do tomorrow but wanted to post these comments now

ultrasecreth and others added 3 commits August 24, 2022 09:18
Co-authored-by: devtooligan <71567643+devtooligan@users.noreply.github.com>
Co-authored-by: devtooligan <71567643+devtooligan@users.noreply.github.com>
Co-authored-by: devtooligan <71567643+devtooligan@users.noreply.github.com>
Co-authored-by: devtooligan <71567643+devtooligan@users.noreply.github.com>
@devtooligan
Copy link
Contributor

Let's remember to lint this right before it gets merged. I don't wanna lint it now because i'm afraid changes will get lost in the diff

Copy link
Contributor

@devtooligan devtooligan left a comment

Choose a reason for hiding this comment

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

Approving, but I'd still like to have a discussion around the methodology used with the "regular" pow function and explore whether:

a) It is appropriate to use that methodology in all edge cases
b) It can be used for the third max function

ultrasecreth and others added 2 commits August 25, 2022 11:21
Co-authored-by: devtooligan <71567643+devtooligan@users.noreply.github.com>
Co-authored-by: devtooligan <71567643+devtooligan@users.noreply.github.com>
@ultrasecreth
Copy link
Contributor Author

Let's remember to lint this right before it gets merged. I don't wanna lint it now because i'm afraid changes will get lost in the diff

yeah, I think I mentioned this somewhere I think, the linter in VSCode was changing so many things I decided not to, so the PR was clean.
I think there should be a different PR where not only the whole project is linted but also to commit the shared linting conf for the plugin and the .vscode to use it so everything is consistent in both IDE & command line for all devs

ultrasecreth and others added 8 commits August 25, 2022 11:25
Co-authored-by: devtooligan <71567643+devtooligan@users.noreply.github.com>
Co-authored-by: devtooligan <71567643+devtooligan@users.noreply.github.com>
Co-authored-by: devtooligan <71567643+devtooligan@users.noreply.github.com>
Co-authored-by: devtooligan <71567643+devtooligan@users.noreply.github.com>
Co-authored-by: devtooligan <71567643+devtooligan@users.noreply.github.com>
Co-authored-by: marcomariscal <42938673+marcomariscal@users.noreply.github.com>
Co-authored-by: marcomariscal <42938673+marcomariscal@users.noreply.github.com>
Co-authored-by: marcomariscal <42938673+marcomariscal@users.noreply.github.com>
@ultrasecreth
Copy link
Contributor Author

Approving, but I'd still like to have a discussion around the methodology used with the "regular" pow function and explore whether:

a) It is appropriate to use that methodology in all edge cases b) It can be used for the third max function

Should we schedule a zoom call?

@devtooligan devtooligan merged commit 913e579 into main Aug 26, 2022
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.

5 participants