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

Feat/tests unit and fuzz #5

Merged
merged 21 commits into from
Aug 3, 2024
Merged

Feat/tests unit and fuzz #5

merged 21 commits into from
Aug 3, 2024

Conversation

0xtekgrinder
Copy link
Collaborator

No description provided.

@0xtekgrinder 0xtekgrinder marked this pull request as draft July 23, 2024 08:46
contracts/BaseStrategy.sol Outdated Show resolved Hide resolved
@@ -486,8 +492,6 @@ abstract contract BaseStrategy is ERC4626, AccessControl {
* @custom:requires INTEGRATOR_ROLE
*/
function setVestingPeriod(uint64 newVestingPeriod) external onlyRole(INTEGRATOR_ROLE) {

Choose a reason for hiding this comment

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

why did we remove that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought it wasn't in our best interest to set boundaries for a vesting period

@0xtekgrinder 0xtekgrinder marked this pull request as ready for review July 23, 2024 16:23
contracts/BaseStrategy.sol Outdated Show resolved Hide resolved
contracts/BaseStrategy.sol Show resolved Hide resolved
test/unit/Accumulate.t.sol Outdated Show resolved Hide resolved
test/unit/Deposit.t.sol Outdated Show resolved Hide resolved
test/unit/Deposit.t.sol Show resolved Hide resolved
test/unit/MaxMint.t.sol Show resolved Hide resolved
test/unit/MaxRedeem.t.sol Show resolved Hide resolved
test/unit/SetDeveloperFeeRecipient.t.sol Outdated Show resolved Hide resolved
test/fuzz/Deposit.t.sol Show resolved Hide resolved
abi.encode(50e18)
);
uint256 maxRedeem = strategy.maxRedeem(alice);
assertEq(maxRedeem, strategy.convertToShares(50e18));

Choose a reason for hiding this comment

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

Instead of doing strategy.convertToShares(50e18) can we compute the actual number, just to have at least one place where we do the actual computation

abi.encode(50e18)
);
uint256 maxWithdraw = strategy.maxWithdraw(alice);
assertEq(maxWithdraw, 50e18);

Choose a reason for hiding this comment

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

It would be great also to have a test on the minimum of a user balance

);
assertEq(
strategy.maxMint(alice),
115625846136565629368682392502753472473372881815114206356643976577531593328100

Choose a reason for hiding this comment

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

type(uint256).max ?

@0xtekgrinder 0xtekgrinder merged commit 535bf2a into main Aug 3, 2024
5 of 7 checks passed
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.

2 participants