-
Notifications
You must be signed in to change notification settings - Fork 9
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
FuseERC4626 Vaults : unit & integration tests #6
base: master
Are you sure you want to change the base?
Conversation
src/vaults/fuse/FuseERC4626.sol
Outdated
returns (uint256) | ||
{ | ||
require(amount != 0, "ZERO_ASSETS"); // Deposit 0 makes no sense | ||
require(amount <= type(uint128).max, "MANY_ASSETS"); // Check for overly large values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why add this uint128 check? I don't think its necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on large values the CToken fails with MINT_EXCHANGE_CALCULATION_FAILED
and there can be issues with wadMul, better to put a boundary (uint128.max is pretty large already)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right but its gas intensive to add. In practice token values don't get this large
Deploy script :
|
Deployed :
|
This PR adds npm scripts to run unit and integration tests :
npm run test
to run unit testsnpm run test:integration
to run integration tests with a fixed block numbernpm run test:integration:latest
to run integration tests using the latest blockUnit tests are added for
FuseERC4626
:And integration tests are added for
Fuse ERC4626
: