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.

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

Amount input #246

Closed
wants to merge 3 commits into from
Closed

Amount input #246

wants to merge 3 commits into from

Conversation

MathisGD
Copy link
Contributor

@MathisGD MathisGD commented Aug 8, 2023

Gas diff (sorry I don't know how @Rubilmax does his ones):
image

MerlinEgalite
MerlinEgalite previously approved these changes Aug 8, 2023
src/Blue.sol Show resolved Hide resolved
src/Blue.sol Show resolved Hide resolved
@Rubilmax
Copy link
Collaborator

Rubilmax commented Aug 9, 2023

Gas diff (sorry I don't know how @Rubilmax does his ones)

I'll never tell you it's my secret sauce

@MerlinEgalite MerlinEgalite requested review from julien-devatom, a team, Rubilmax, pakim249CAL, Jean-Grimal and makcandrov and removed request for a team August 9, 2023 12:34
@MathisGD MathisGD self-assigned this Aug 9, 2023
Copy link
Contributor

@Jean-Grimal Jean-Grimal left a comment

Choose a reason for hiding this comment

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

IMO the best solution

Copy link
Contributor

@julien-devatom julien-devatom left a comment

Choose a reason for hiding this comment

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

This is not so dirty to use a variable for a second usage. We use a specific int value (maxUint256) as a boolean. This is precisely what Ethereum does with the empty to field in a tx:
if to == undefined, then I call CREATE opcode, else call the code at the specified address.

The point is that this method is reducing the number of possibilities, such as "I want to withdraw 40% of my position"

@MerlinEgalite
Copy link
Contributor

The point is that this method is reducing the number of possibilities, such as "I want to withdraw 40% of my position"

If you care about some wei yes.

Also another option is taking the max between the balance of the user and the amount passed as argument

@MerlinEgalite
Copy link
Contributor

Well I've tried to create an alternative solution using the min between the amount passed and the balance of the user, but I'm facing to many roundings to make it a creadible alternative

@MathisGD
Copy link
Contributor Author

MathisGD commented Aug 9, 2023

ciao

@MathisGD MathisGD closed this Aug 9, 2023
@MathisGD MathisGD deleted the feat/amount-input-plus-max branch August 9, 2023 19:45
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