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

(3) "wad-overflow" not likely for lockNgt and freeNgt #10

Open
oldchili opened this issue Dec 26, 2023 · 3 comments
Open

(3) "wad-overflow" not likely for lockNgt and freeNgt #10

oldchili opened this issue Dec 26, 2023 · 3 comments

Comments

@oldchili
Copy link
Contributor

oldchili commented Dec 26, 2023

lockNgt and freeNgt minor optimization - I think as long as mkrNgtRate is larger than 2 then "wad-overflow" can never happen for these. We should consider adding that check to the constructor and moving the check to only lock and free.

@oldchili oldchili changed the title "wad-overflow" not likely for lockNgt and freeNgt (3) "wad-overflow" not likely for lockNgt and freeNgt Jan 8, 2024
@sunbreak1211
Copy link
Collaborator

echo "(2^256-1) / 3 < 2^255-1" | bc
1
echo "(2^256-1) / 2 < 2^255-1" | bc
0

If the rate of NGT/MKR is >= 3 then it is safe to remove the require for NGT. However not sure if I feel comfortable leaving that to a variable that comes from an external contract. The only exception would be if we put a require in the constructor to verify that.

@oldchili
Copy link
Contributor Author

oldchili commented Jan 9, 2024

echo "(2^256-1) / 3 < 2^255-1" | bc
1
echo "(2^256-1) / 2 < 2^255-1" | bc
0

If the rate of NGT/MKR is >= 3 then it is safe to remove the require for NGT. However not sure if I feel comfortable leaving that to a variable that comes from an external contract. The only exception would be if we put a require in the constructor to verify that.

Yes, was thinking of a constructor check as well.
Not super critical, so feel free to ignore this issue.

@sunbreak1211
Copy link
Collaborator

Let's leave it open for now and we can take a decision later.

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

No branches or pull requests

2 participants