Skip to content
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.

Sign up for GitHub

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

Jump to bottom

repay/withdraw shares input #194

Merged
merged 34 commits into from
Aug 5, 2023
Merged

repay/withdraw shares input #194

merged 34 commits into from
Aug 5, 2023

Conversation

MathisGD
Copy link
Contributor

@MathisGD MathisGD commented Jul 28, 2023 •
edited
Loading

withdraw/repay exact amount maths:
main.pdf

MerlinEgalite and others added 10 commits July 12, 2023 20:05
@MerlinEgalite
feat: repay/withdraw all
e1d53c6
@MerlinEgalite
test: re-add empty market tests
d5036b7
@Rubilmax
Merge branch 'refactor/morpho-utils' of github.com:morpho-labs/blue i…
8264939 
…nto feat/repay-withdraw-all
@Rubilmax
fix(repay/withdraw): edge case
24cdfb4
@Rubilmax @Jean-Grimal
Update src/Blue.sol
ea73dd3 
Co-authored-by: Jean-Grimal <[email protected]>
Signed-off-by: Romain Milon <[email protected]>
@Rubilmax
Update src/Blue.sol
ede9cee 
Signed-off-by: Romain Milon <[email protected]>
@Rubilmax
Merge pull request #149 from morpho-labs/refactor/repay-withdraw-all-…
2e471cc 
…roundings

fix(repay/withdraw): edge case
@Rubilmax
Merge branch 'main' of github.com:morpho-labs/blue into feat/repay-wi…
2b8c672 
…thdraw-all
@Rubilmax
Merge branch 'main' of github.com:morpho-labs/blue into feat/repay-wi…
1b98702 
…thdraw-all
@MathisGD
feat: repay/withdraw with shares
a202085
Rubilmax

This comment was marked as resolved.

Sign in to view

Rubilmax
Rubilmax reviewed Jul 31, 2023
View reviewed changes
Copy link
Collaborator

@Rubilmax Rubilmax left a comment •
edited
Loading

Choose a reason for hiding this comment

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

100% in favor of this suggestion similar to EIP4626, though @QGarchery was not when we talked about it if I recall correctly

src/Blue.sol Outdated Show resolved Hide resolved
@Rubilmax
Copy link
Collaborator

Also fixes #177 for repay/withdraw (but not liquidate)

@Rubilmax Rubilmax mentioned this pull request Jul 31, 2023
Closed
@makcandrov
Copy link
Contributor

makcandrov commented Aug 1, 2023 •
edited
Loading

Imo, for this version to be viable, supply, supplyCollateral & borrow should return the new shares minted, and withdraw, withdrawCollateral & repay should return (and send in the callback) the amount withdrawn/repaid

edit: mb supplyCollateral & withdrawCollateral should be exempt from this treatment

@MathisGD
Copy link
Contributor Author

MathisGD commented Aug 1, 2023

the collateral is stored in underlying so I don't think that supplyCollateral and withdrawCollateral need it. For other functions I'll think about it.

@MathisGD MathisGD mentioned this pull request Aug 1, 2023
Closed
This was linked to issues Aug 1, 2023
Repay/Withdraw max #40
Closed
Require shares > 0 upon supply/withdraw/borrow/repay? #155
Closed
@MathisGD MathisGD self-assigned this Aug 1, 2023
@MathisGD MathisGD linked an issue Aug 1, 2023 that may be closed by this pull request
repay, withdraw, withdrawCollateral, liquidate revert with underflow in case of large input #177
Closed
@MathisGD MathisGD removed a link to an issue Aug 1, 2023
repay, withdraw, withdrawCollateral, liquidate revert with underflow in case of large input #177
Closed
@MathisGD MathisGD changed the base branch from feat/repay-withdraw-all to main August 1, 2023 10:10
@MathisGD MathisGD changed the base branch from main to feat/repay-withdraw-all August 1, 2023 10:14
@MathisGD MathisGD mentioned this pull request Aug 1, 2023
Closed
Rubilmax
Rubilmax reviewed Aug 1, 2023
View reviewed changes
src/Blue.sol Outdated Show resolved Hide resolved
@MathisGD MathisGD changed the base branch from feat/repay-withdraw-all to main August 4, 2023 15:48
@MathisGD MathisGD dismissed stale reviews from Rubilmax and julien-devatom August 4, 2023 15:48

The base branch was changed.

@MathisGD MathisGD mentioned this pull request Aug 4, 2023
Closed
Rubilmax
Rubilmax previously approved these changes Aug 4, 2023
View reviewed changes
Copy link
Collaborator

@Rubilmax Rubilmax left a comment

Choose a reason for hiding this comment

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

There are still things that deserve a discussion (or additional thoughts on my end) but we can merge like so and I'll think about it afterwards

@MathisGD MathisGD changed the title Repay withdraw max alternative repay/withdraw shares input Aug 4, 2023
Jean-Grimal
Jean-Grimal previously approved these changes Aug 4, 2023
View reviewed changes
MerlinEgalite
MerlinEgalite requested changes Aug 4, 2023
View reviewed changes
src/libraries/BlueLib.sol Outdated Show resolved Hide resolved
src/libraries/BlueLib.sol Outdated Show resolved Hide resolved
pakim249CAL
pakim249CAL previously approved these changes Aug 4, 2023
View reviewed changes
@MathisGD MathisGD dismissed stale reviews from pakim249CAL, Jean-Grimal, and Rubilmax via 9be7701 August 4, 2023 18:12
pakim249CAL
pakim249CAL previously approved these changes Aug 4, 2023
View reviewed changes
@MerlinEgalite MerlinEgalite merged commit bd93076 into main Aug 5, 2023
@MerlinEgalite MerlinEgalite deleted the feat/repay-withdraw-max-alternative branch August 5, 2023 09:27
makcandrov
makcandrov approved these changes Aug 5, 2023
View reviewed changes
test/hardhat/Blue.spec.ts Show resolved Hide resolved
@MathisGD MathisGD mentioned this pull request Aug 5, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MerlinEgalite MerlinEgalite MerlinEgalite approved these changes

@makcandrov makcandrov makcandrov approved these changes

@Jean-Grimal Jean-Grimal Jean-Grimal approved these changes

@pakim249CAL pakim249CAL pakim249CAL approved these changes

@Rubilmax Rubilmax Rubilmax approved these changes

@julien-devatom julien-devatom julien-devatom left review comments

Assignees

@MathisGD MathisGD

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Require shares > 0 upon supply/withdraw/borrow/repay? Repay/Withdraw max
7 participants
@MathisGD @Rubilmax @makcandrov @MerlinEgalite @pakim249CAL @julien-devatom @Jean-Grimal