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

fix: address best practices #5

Conversation

jaypaik
Copy link
Collaborator

@jaypaik jaypaik commented Sep 20, 2023

  1. Use custom errors in place of require statements for gas savings.
    • Fixed.
  2. Address.isContract() will be removed in the OZ-contracts 5.0 release. Consider replacing it with (address(this).code.length > 0)
    • Fixed.
  3. Typo "_getInitialiazableStorage()" ⇒ "_getInitializableStorage()".
    • Fixed.
  4. Global state variables, such as CustomSlotInitializable._storagePosition , should not have a leading underscore.
    • Acknowledged. We'll keep this to follow our convention of leading underscores for internal / private variables.
  5. For loops can be gas-optimized by cashing the array.length in a memory variable and incrementing the iterator via unchecked {++i;} .
    • Fixed.
  6. LightAccount.transferOwnership() can be restricted to external visibility.
    • Fixed.
  7. _getInitialiazableStorage() can load directly from a constant and does not need a temporary variable.
    • Acknowledged. Assembly access to immutable variables is not supported, so we're keeping this.

@jaypaik jaypaik force-pushed the 09-20-_ALC-5_update_initialized_from_uint8_to_uint64 branch from ca6c9f3 to 5ee69ed Compare September 20, 2023 19:22
@jaypaik jaypaik force-pushed the 09-20-fix_address_best_practices branch from 87d33aa to 581f73a Compare September 20, 2023 19:22
@jaypaik jaypaik force-pushed the 09-20-_ALC-5_update_initialized_from_uint8_to_uint64 branch from 5ee69ed to 9b88fbd Compare September 20, 2023 19:27
@jaypaik jaypaik force-pushed the 09-20-fix_address_best_practices branch from 581f73a to 84716cf Compare September 20, 2023 19:27
@howydev howydev force-pushed the 09-20-_ALC-5_update_initialized_from_uint8_to_uint64 branch from 2045691 to bf99b74 Compare September 22, 2023 16:49
@howydev howydev force-pushed the 09-20-fix_address_best_practices branch 2 times, most recently from 17e5df6 to 84716cf Compare September 22, 2023 16:56
howydev and others added 3 commits September 22, 2023 14:09
…xplaining_contract_owner_limitations_for_user_operation_validation

[ALC-2] add comment explaining contract owner limitations for user operation validation
…lized_from_uint8_to_uint64

[ALC-5] update initialized from uint8 to uint64
@howydev howydev force-pushed the 09-20-fix_address_best_practices branch from 84716cf to e07c3d6 Compare September 22, 2023 18:39
@howydev howydev merged commit 55ecfb5 into 09-20-_ALC-5_update_initialized_from_uint8_to_uint64 Sep 22, 2023
2 checks passed
@howydev howydev deleted the 09-20-fix_address_best_practices branch September 22, 2023 19:09
@howydev howydev restored the 09-20-fix_address_best_practices branch September 22, 2023 19:11
@howydev howydev deleted the 09-20-fix_address_best_practices branch September 22, 2023 23:13
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