-
Notifications
You must be signed in to change notification settings - Fork 8
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
check and update two slots #87
check and update two slots #87
Conversation
Changes to gas cost
🧾 Summary (5% most significant diffs)
Full diff report 👇
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #87 +/- ##
==========================================
- Coverage 81.10% 81.01% -0.10%
==========================================
Files 35 35
Lines 757 769 +12
Branches 120 107 -13
==========================================
+ Hits 614 623 +9
- Misses 132 135 +3
Partials 11 11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
contracts/Nexus.sol
Outdated
@@ -285,9 +299,20 @@ contract Nexus is INexus, EIP712, BaseAccount, ExecutionHelper, ModuleManager, U | |||
} | |||
|
|||
/// Upgrades the contract to a new implementation and calls a function on the new contract. | |||
/// @notice Updates two slots 1. 1967 slot and 2. address() slot in case if it's upgraded earlier from Biconomy V2 account. |
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.
...as Biconomy v2 Account (proxy) reads implementation from the slot that is defined by its address.
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.
fixed
} | ||
if(implementation == address(0)) { | ||
assembly { | ||
implementation := sload(address()) |
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.
what is this magic?
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.
which line?
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.
address()
opcode returns the same as address(this)
so the contract's address is used as number of the storage slot (it is converted to bytes32) to store the address of the implementation
so this line loads it from storage using the opcode to obtain the slot where the implementation address is stored.
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.
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.
LGTM only UpgradeToAndCall negative tests are missing
check and update two slots
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
…ithub.com/bcnmy/erc7579-modular-smart-account into feat/manage-multiple-implementation-slots
🤖 Slither Analysis Report 🔎Slither report
# Slither report
_This comment was automatically generated by the GitHub Actions workflow._
THIS CHECKLIST IS NOT COMPLETE. Use |
No description provided.