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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jun 2, 2024
Merged

check and update two slots #87

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
26d92e5
check and update two slots
livingrockrises May 30, 2024
9a91b01
respond to PR comments
livingrockrises May 31, 2024
32d5bd9
Merge branch 'dev' into feat/manage-multiple-implementation-slots
Aboudjem Jun 1, 2024
ef7c414
negative test cases
livingrockrises Jun 1, 2024
3a2a610
Merge branch 'feat/manage-multiple-implementation-slots' of https://g…
livingrockrises Jun 1, 2024
2fc766e
negative test + refactor
livingrockrises Jun 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion contracts/Nexus.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,20 @@
return IValidator(validator).isValidSignatureWithSender(msg.sender, computeHash, truncatedSignature);
}

/// @notice Retrieves the address of the current implementation from the EIP-1967 slot.
/// @notice Checks the 1967 implementation slot, if not found then checks the slot defined by address (Biconomy V2 smart account)
/// @return implementation The address of the current contract implementation.
function getImplementation() external view returns (address implementation) {
assembly {
implementation := sload(_ERC1967_IMPLEMENTATION_SLOT)
}
if(implementation == address(0)) {
assembly {

Check warning on line 248 in contracts/Nexus.sol

View check run for this annotation

Codecov / codecov/patch

contracts/Nexus.sol#L248 

Added line #L248 was not covered by tests
implementation := sload(address())
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this magic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

which line?

Copy link
Collaborator

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup,
https://github.com/bcnmy/scw-contracts/blob/main/contracts/smart-account/Proxy.sol#L23

}
}
}

/// @notice Checks if a specific module type is supported by this smart account.
/// @param moduleTypeId The identifier of the module type to check.
/// @return True if the module type is supported, false otherwise.
Expand Down Expand Up @@ -285,9 +299,20 @@
}

/// 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.
Copy link
Collaborator

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

/// @param newImplementation The address of the new contract implementation.
/// @param data The calldata to be sent to the new implementation.
function upgradeToAndCall(address newImplementation, bytes calldata data) public payable virtual override {
function upgradeToAndCall(address newImplementation, bytes calldata data) public payable virtual override onlyEntryPointOrSelf {
if(newImplementation == address(0)) revert InvalidImplementationAddress();
bool res;
assembly {

Check warning on line 308 in contracts/Nexus.sol

View check run for this annotation

Codecov / codecov/patch

contracts/Nexus.sol#L308 

Added line #L308 was not covered by tests
res := gt(extcodesize(newImplementation), 0)
}
if(res == false) revert InvalidImplementationAddress();
// update the address() storage slot as well.
assembly {

Check warning on line 313 in contracts/Nexus.sol

View check run for this annotation

Codecov / codecov/patch

contracts/Nexus.sol#L313 

Added line #L313 was not covered by tests
sstore(address(), newImplementation)
}
UUPSUpgradeable.upgradeToAndCall(newImplementation, data);
}

Expand Down
6 changes: 6 additions & 0 deletions contracts/interfaces/INexus.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ interface INexus is IERC4337Account, IERC7579Account, INexusEventsAndErrors {

/// @notice Throws if zero address has been provided as Entry Point address
error EntryPointCannotBeZero();

/// @notice Throws if the implementation address is invalid
error InvalidImplementationAddress();

/// @notice Throws if the implementation is not a contract
error ImplementationIsNotAContract();

/// @notice Initializes the smart account with a validator and custom data.
/// @dev This method sets up the account for operation, linking it with a validator and initializing it with specific data.
Expand Down
5 changes: 3 additions & 2 deletions test/foundry/fork/arbitrum/ArbitrumSmartAccountUpgradeTest.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,9 @@ contract ArbitrumSmartAccountUpgradeTest is NexusTest_Base, ArbitrumSettings {
address beforeUpgradeImplementation = IBiconomySmartAccountV2(SMART_ACCOUNT_V2_ADDRESS).getImplementation();
assertNotEq(beforeUpgradeImplementation, address(newImplementation), "Implementation address does not match before upgrade.");
test_UpgradeV2ToV3AndInitialize();
bytes32 SLOT = bytes32(uint256(uint160(SMART_ACCOUNT_V2_ADDRESS)));
address afterUpgradeImplementation = address(uint160(uint256(vm.load(SMART_ACCOUNT_V2_ADDRESS, SLOT))));
// bytes32 SLOT = bytes32(uint256(uint160(SMART_ACCOUNT_V2_ADDRESS)));
// address afterUpgradeImplementation = address(uint160(uint256(vm.load(SMART_ACCOUNT_V2_ADDRESS, SLOT))));
address afterUpgradeImplementation = Nexus(payable(SMART_ACCOUNT_V2_ADDRESS)).getImplementation();
address expectedImplementation = address(newImplementation);
assertEq(afterUpgradeImplementation, expectedImplementation, "Implementation address does not match after upgrade.");
}
Expand Down
10 changes: 6 additions & 4 deletions test/foundry/integration/UpgradeSmartAccountTest.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ contract UpgradeSmartAccountTest is NexusTest_Base {

/// @notice Tests that the current implementation address is correct
function test_currentImplementationAddress() public {
bytes32 _ERC1967_IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
address currentImplementation = address(uint160(uint256(vm.load(address(BOB_ACCOUNT), _ERC1967_IMPLEMENTATION_SLOT))));
// bytes32 _ERC1967_IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
// address currentImplementation = address(uint160(uint256(vm.load(address(BOB_ACCOUNT), _ERC1967_IMPLEMENTATION_SLOT))));
address currentImplementation = BOB_ACCOUNT.getImplementation();
assertEq(currentImplementation, address(ACCOUNT_IMPLEMENTATION), "Current implementation address mismatch");
}

Expand All @@ -33,8 +34,9 @@ contract UpgradeSmartAccountTest is NexusTest_Base {

PackedUserOperation[] memory userOps = buildPackedUserOperation(BOB, BOB_ACCOUNT, EXECTYPE_DEFAULT, execution, address(VALIDATOR_MODULE));
ENTRYPOINT.handleOps(userOps, payable(address(BOB.addr)));
bytes32 _ERC1967_IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
address newImplementation = address(uint160(uint256(vm.load(address(BOB_ACCOUNT), _ERC1967_IMPLEMENTATION_SLOT))));
// bytes32 _ERC1967_IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
// address newImplementation = address(uint160(uint256(vm.load(address(BOB_ACCOUNT), _ERC1967_IMPLEMENTATION_SLOT))));
address newImplementation = BOB_ACCOUNT.getImplementation();
assertEq(newImplementation, address(newSmartAccount), "New implementation address mismatch");
}

Expand Down
5 changes: 3 additions & 2 deletions test/hardhat/smart-account/MSA.Basics.specs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ describe("Nexus Basic Specs", function () {
});

it("Should get implementation address of smart account", async () => {
const slot =
/*const slot =
"0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc";
// Get the provider (default to Hardhat's local network)
const provider = ethers.provider;
Expand All @@ -131,7 +131,8 @@ describe("Nexus Basic Specs", function () {
"latest",
]);
// Convert the storage value to an address
const saImplementation = ethers.getAddress(toHex(BigInt(storageValue)));
const saImplementation = ethers.getAddress(toHex(BigInt(storageValue)));*/
const saImplementation = await smartAccount.getImplementation();
console.log("Implementation Address: ", saImplementation);
expect(saImplementation).to.not.equal(ZeroAddress);
});
Expand Down
Loading