fix: correctly check hooks length in installValidation #243
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jaypaik
reviewed
Oct 14, 2024
We don't have a length field tracking it anywhere, so we don't know how long the existing set of hooks is when we're adding to it. |
adamegyed
force-pushed
the
adam/fix-install-hooks-count
branch
from
fa803f7
to
771b232
Compare
jaypaik
approved these changes
Oct 14, 2024
adamegyed
force-pushed
the
adam/call-buffers-sig
branch
2 times, most recently
from
8c8d350
to
e441f28
Compare
adamegyed
force-pushed
the
adam/fix-install-hooks-count
branch
from
771b232
to
b29be70
Compare
Summary by OctaneNew ContractsNo new contracts were added in this PR. Updated Contracts
🔗 Commit Hash: b29be70 |
|
Contract sizes: | Contract | Size (B) | Margin (B) |
|-------------------------------|----------|------------|
| AccountFactory | 4,814 | 19,762 |
| AllowlistModule | 9,230 | 15,346 |
| ECDSAValidationModule | 3,646 | 20,930 |
-| ModularAccount | 25,574 | -998 |
+| ModularAccount | 25,655 | -1,079 |
| NativeTokenLimitModule | 4,714 | 19,862 |
| PaymasterGuardModule | 1,797 | 22,779 |
-| SemiModularAccountBytecode | 27,562 | -2,986 |
-| SemiModularAccountStorageOnly | 28,080 | -3,504 |
+| SemiModularAccountBytecode | 27,553 | -2,977 |
+| SemiModularAccountStorageOnly | 28,071 | -3,495 |
| TimeRangeModule | 2,000 | 22,576 |
| WebauthnValidationModule | 7,854 | 16,722 |Code coverage:
|
This comment was marked as resolved.
This comment was marked as resolved.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
We don't actually ever revert with the custom error for exceeding the max hook count because the check addition would overflow before that. The custom error is preferred for visibility into the issue.
Solution
Correctly detect exceeding the length and revert.