-
Notifications
You must be signed in to change notification settings - Fork 1k
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
add reverting tests, fix fuzz for hooks.t.sol #548
Conversation
Forge code coverage:
|
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.
Just a few cleanup things but great new tests
test/libraries/Hooks.t.sol
Outdated
// Set the upper `hooksPermissionCount` number of bits to get the full mask in uint16. | ||
uint16 allHooksMask = (~uint16(0) << uint16(16 - hookPermissionCount)); | ||
// We want any combination except all hooks. | ||
vm.assume(mask < allHooksMask); |
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.
the name of the test is failsAllHooks
as if it has all hooks - but this test explicitly doesnt have all hooks?
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.
OH i see its that validateHookPermissions
has all hooks
can you make all the import paths in Hooks.t.sol relative? |
i forget, did we want to change the hook flags to be uint160 instead of uint256? |
Related Issue
Which issue does this pull request resolve?
Description of changes