Skip to content

Commit

Permalink
Improving testing
Browse files Browse the repository at this point in the history
  • Loading branch information
eloi010 committed Nov 21, 2023
1 parent c4700e2 commit b5f3c5e
Show file tree
Hide file tree
Showing 16 changed files with 645 additions and 534 deletions.
4 changes: 4 additions & 0 deletions contracts/core/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Openfort Accounts

There are three types of Openfort accounts:
1. Upgradeable accounts (with recoverable features)
2. Managed accounts (with recoverable features)
3. ERC6551 accounts (experiment)

## Recoverable Accounts
Recoverable accounts are a special type of accounts that support social recovery.
Expand Down
31 changes: 16 additions & 15 deletions contracts/core/base/BaseOpenfortAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import {EIP712Upgradeable} from "@openzeppelin/contracts-upgradeable/utils/crypt
import {IERC1271Upgradeable} from "@openzeppelin/contracts-upgradeable/interfaces/IERC1271Upgradeable.sol";
import {SafeCastUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/math/SafeCastUpgradeable.sol";
import {BaseAccount, UserOperation, IEntryPoint, UserOperationLib} from "account-abstraction/core/BaseAccount.sol";
import {TokenCallbackHandler} from "account-abstraction/samples/callback/TokenCallbackHandler.sol";
import {_packValidationData} from "account-abstraction/core/Helpers.sol";
import {TokenCallbackHandler} from "./TokenCallbackHandler.sol";
import {OpenfortErrorsAndEvents} from "../../interfaces/OpenfortErrorsAndEvents.sol";

/**
Expand Down Expand Up @@ -46,15 +46,15 @@ abstract contract BaseOpenfortAccount is
* @param validUntil this sessionKey is valid only until this timestamp.
* @param limit limit of uses remaining
* @param masterSessionKey if set to true, the session key does not have any limitation other than the validity time
* @param whitelising if set to true, the session key has to follow whitelisting rules
* @param whitelisting if set to true, the session key has to follow whitelisting rules
* @param whitelist - this session key can only interact with the addresses in the whitelist.
*/
struct SessionKeyStruct {
uint48 validAfter;
uint48 validUntil;
uint48 limit;
bool masterSessionKey;
bool whitelising;
bool whitelisting;
mapping(address contractAddress => bool allowed) whitelist;
address registrarAddress;
}
Expand All @@ -71,7 +71,7 @@ abstract contract BaseOpenfortAccount is
/**
* Require the function call went through EntryPoint or owner
*/
function _requireFromEntryPointOrOwner() internal view {
function _requireFromEntryPointOrOwner() internal view virtual {
if (msg.sender != address(entryPoint()) && msg.sender != owner()) {
revert NotOwnerOrEntrypoint();
}
Expand All @@ -91,14 +91,14 @@ abstract contract BaseOpenfortAccount is
/**
* Check current account deposit in the entryPoint
*/
function getDeposit() public view returns (uint256) {
function getDeposit() public view virtual returns (uint256) {
return entryPoint().balanceOf(address(this));
}

/*
* @notice Return whether a sessionKey is valid.
*/
function isValidSessionKey(address _sessionKey, bytes calldata _callData) public returns (bool) {
function isValidSessionKey(address _sessionKey, bytes calldata _callData) public virtual returns (bool) {
SessionKeyStruct storage sessionKey = sessionKeys[_sessionKey];
// If not owner and the session key is revoked, return false
if (sessionKey.validUntil == 0) return false;
Expand Down Expand Up @@ -132,7 +132,7 @@ abstract contract BaseOpenfortAccount is
} // Only masterSessionKey can reenter

// If there is no whitelist or there is, but the target is whitelisted, return true
if (!sessionKey.whitelising || sessionKey.whitelist[toContract]) {
if (!sessionKey.whitelisting || sessionKey.whitelist[toContract]) {
return true;
}

Expand All @@ -152,7 +152,7 @@ abstract contract BaseOpenfortAccount is
if (toContracts[i] == address(this)) {
return false;
} // Only masterSessionKey can reenter
if (sessionKey.whitelising && !sessionKey.whitelist[toContracts[i]]) {
if (sessionKey.whitelisting && !sessionKey.whitelist[toContracts[i]]) {
return false;
} // One contract's not in the sessionKey's whitelist (if any)
unchecked {
Expand All @@ -170,7 +170,7 @@ abstract contract BaseOpenfortAccount is
* @notice See EIP-1271
* Owner and session keys need to sign using EIP712.
*/
function isValidSignature(bytes32 _hash, bytes memory _signature) public view override returns (bytes4) {
function isValidSignature(bytes32 _hash, bytes memory _signature) public view virtual override returns (bytes4) {
bytes32 structHash = keccak256(abi.encode(OF_MSG_TYPEHASH, _hash));
bytes32 digest = _hashTypedDataV4(structHash);
address signer = digest.recover(_signature);
Expand Down Expand Up @@ -222,7 +222,7 @@ abstract contract BaseOpenfortAccount is
/**
* Deposit funds for this account in the EntryPoint
*/
function addDeposit() public payable {
function addDeposit() public payable virtual {
entryPoint().depositTo{value: msg.value}(address(this));
}

Expand All @@ -232,15 +232,15 @@ abstract contract BaseOpenfortAccount is
* @param _amount to withdraw
* @notice ONLY the owner can call this function (it's not using _requireFromEntryPointOrOwner())
*/
function withdrawDepositTo(address payable _withdrawAddress, uint256 _amount) external {
function withdrawDepositTo(address payable _withdrawAddress, uint256 _amount) external virtual {
_requireFromOwner();
entryPoint().withdrawTo(_withdrawAddress, _amount);
}

/**
* @dev Call a target contract and reverts if it fails.
*/
function _call(address _target, uint256 _value, bytes calldata _calldata) internal {
function _call(address _target, uint256 _value, bytes calldata _calldata) internal virtual {
(bool success, bytes memory result) = _target.call{value: _value}(_calldata);
if (!success) {
assembly {
Expand All @@ -254,6 +254,7 @@ abstract contract BaseOpenfortAccount is
*/
function _validateSignature(UserOperation calldata userOp, bytes32 userOpHash)
internal
virtual
override
returns (uint256 validationData)
{
Expand Down Expand Up @@ -286,7 +287,7 @@ abstract contract BaseOpenfortAccount is
uint48 _validUntil,
uint48 _limit,
address[] calldata _whitelist
) public {
) public virtual {
_requireFromEntryPointOrOwner();

require(_whitelist.length < 11, "Whitelist too big");
Expand All @@ -299,7 +300,7 @@ abstract contract BaseOpenfortAccount is
}
if (i != 0) {
// If there was some whitelisting, it is not a masterSessionKey
sessionKeys[_key].whitelising = true;
sessionKeys[_key].whitelisting = true;
sessionKeys[_key].masterSessionKey = false;
} else {
if (_limit == ((2 ** 48) - 1)) sessionKeys[_key].masterSessionKey = true;
Expand All @@ -318,7 +319,7 @@ abstract contract BaseOpenfortAccount is
* Revoke a session key from the account
* @param _key session key to revoke
*/
function revokeSessionKey(address _key) external {
function revokeSessionKey(address _key) external virtual {
_requireFromEntryPointOrOwner();
if (sessionKeys[_key].validUntil != 0) {
sessionKeys[_key].validUntil = 0;
Expand Down
16 changes: 8 additions & 8 deletions contracts/core/base/BaseRecoverableAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -130,22 +130,22 @@ abstract contract BaseRecoverableAccount is BaseOpenfortAccount, Ownable2StepUpg
/**
* @notice Helper method to check if a wallet is locked.
*/
function isLocked() public view returns (bool) {
function isLocked() public view virtual returns (bool) {
return guardiansConfig.lock > block.timestamp;
}

/**
* @notice Returns the release time of a wallet lock or 0 if the wallet is unlocked.
* @return _releaseAfter The epoch time at which the lock will release (in seconds).
*/
function getLock() external view returns (uint256 _releaseAfter) {
function getLock() external view virtual returns (uint256 _releaseAfter) {
_releaseAfter = isLocked() ? guardiansConfig.lock : 0;
}

/**
* @notice Lets a guardian lock a wallet.
*/
function lock() external {
function lock() external virtual {
if (!isGuardian(msg.sender)) revert MustBeGuardian();
if (isLocked()) revert AccountLocked();
_setLock(block.timestamp + lockPeriod);
Expand All @@ -154,7 +154,7 @@ abstract contract BaseRecoverableAccount is BaseOpenfortAccount, Ownable2StepUpg
/**
* @notice Lets a guardian unlock a locked wallet.
*/
function unlock() external {
function unlock() external virtual {
if (!isGuardian(msg.sender)) revert MustBeGuardian();
if (!isLocked()) revert AccountNotLocked();
_setLock(0);
Expand All @@ -176,15 +176,15 @@ abstract contract BaseRecoverableAccount is BaseOpenfortAccount, Ownable2StepUpg
* @notice Returns the number of guardians for the Openfort account.
* @return the number of guardians.
*/
function guardianCount() public view returns (uint256) {
function guardianCount() public view virtual returns (uint256) {
return guardiansConfig.guardians.length;
}

/**
* @notice Gets the list of guardians for the Openfort account.
* @return the list of guardians.
*/
function getGuardians() external view returns (address[] memory) {
function getGuardians() external view virtual returns (address[] memory) {
address[] memory guardians = new address[](guardiansConfig.guardians.length);
uint256 i;
for (i; i < guardiansConfig.guardians.length;) {
Expand Down Expand Up @@ -340,7 +340,7 @@ abstract contract BaseRecoverableAccount is BaseOpenfortAccount, Ownable2StepUpg
* Must be confirmed by N guardians, where N = ceil(Nb Guardians / 2).
* @param _recoveryAddress The address to which ownership should be transferred.
*/
function startRecovery(address _recoveryAddress) external {
function startRecovery(address _recoveryAddress) external virtual {
if (!isGuardian(msg.sender)) revert MustBeGuardian();
_requireRecovery(false);
if (isGuardian(_recoveryAddress)) revert GuardianCannotBeOwner();
Expand All @@ -356,7 +356,7 @@ abstract contract BaseRecoverableAccount is BaseOpenfortAccount, Ownable2StepUpg
* @param _signatures Array of guardian signatures concatenated.
* @notice The arguments should be ordered by the address of the guardian signing the message
*/
function completeRecovery(bytes[] calldata _signatures) external {
function completeRecovery(bytes[] calldata _signatures) external virtual {
_requireRecovery(true);
if (recoveryDetails.executeAfter > uint64(block.timestamp)) revert OngoingRecovery();

Expand Down
52 changes: 52 additions & 0 deletions contracts/core/base/TokenCallbackHandler.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// SPDX-License-Identifier: GPL-3.0
pragma solidity =0.8.19;

import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol";
import {IERC777Recipient} from "@openzeppelin/contracts/token/ERC777/IERC777Recipient.sol";
import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
import {IERC1155Receiver} from "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol";

/**
* Token callback handler.
* Handles supported tokens' callbacks, allowing account receiving these tokens.
*/
contract TokenCallbackHandler is IERC777Recipient, IERC721Receiver, IERC1155Receiver {
function tokensReceived(address, address, address, uint256, bytes calldata, bytes calldata)
external
pure
override
{}

function onERC721Received(address, address, uint256, bytes calldata)
external
view
virtual
override
returns (bytes4)
{
return IERC721Receiver.onERC721Received.selector;
}

function onERC1155Received(address, address, uint256, uint256, bytes calldata)
external
pure
override
returns (bytes4)
{
return IERC1155Receiver.onERC1155Received.selector;
}

function onERC1155BatchReceived(address, address, uint256[] calldata, uint256[] calldata, bytes calldata)
external
pure
override
returns (bytes4)
{
return IERC1155Receiver.onERC1155BatchReceived.selector;
}

function supportsInterface(bytes4 interfaceId) external view virtual override returns (bool) {
return interfaceId == type(IERC721Receiver).interfaceId || interfaceId == type(IERC1155Receiver).interfaceId
|| interfaceId == type(IERC165).interfaceId;
}
}
51 changes: 48 additions & 3 deletions contracts/core/erc6551/ERC6551OpenfortAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ contract ERC6551OpenfortAccount is BaseOpenfortAccount, IERC6551Account, IERC655
* @notice Initialize the smart contract wallet.
*/
function initialize(address _entrypoint) public initializer {
if (_entrypoint == address(0)) {
revert ZeroAddressNotAllowed();
}
_requireFromOwner();
if (_entrypoint == address(0)) revert ZeroAddressNotAllowed();
emit EntryPointUpdated(entrypointContract, _entrypoint);
entrypointContract = _entrypoint;
__EIP712_init("Openfort", "0.5");
Expand Down Expand Up @@ -136,4 +135,50 @@ contract ERC6551OpenfortAccount is BaseOpenfortAccount, IERC6551Account, IERC655
|| _interfaceId == type(IERC165).interfaceId
);
}

function onERC721Received(address, address, uint256 receivedTokenId, bytes memory)
external
view
override
returns (bytes4)
{
_revertIfOwnershipCycle(msg.sender, receivedTokenId);
return IERC721Receiver.onERC721Received.selector;
}

/**
* @dev Helper method to check if a received token is in the ownership chain of the wallet.
* @param receivedTokenAddress The address of the token being received.
* @param receivedTokenId The ID of the token being received.
*/
function _revertIfOwnershipCycle(address receivedTokenAddress, uint256 receivedTokenId) internal view virtual {
(uint256 _chainId, address _contractAddress, uint256 _tokenId) = token();
require(
_chainId != block.chainid || receivedTokenAddress != _contractAddress || receivedTokenId != _tokenId,
"Cannot own yourself"
);

address currentOwner = owner();
require(currentOwner != address(this), "Token in ownership chain");
uint256 depth = 0;
while (currentOwner.code.length > 0) {
try IERC6551Account(payable(currentOwner)).token() returns (
uint256 chainId, address contractAddress, uint256 tokenId
) {
require(
chainId != block.chainid || contractAddress != receivedTokenAddress || tokenId != receivedTokenId,
"Token in ownership chain"
);
// Advance up the ownership chain
currentOwner = IERC721(contractAddress).ownerOf(tokenId);
require(currentOwner != address(this), "Token in ownership chain");
} catch {
break;
}
unchecked {
++depth;
}
if (depth == 5) revert("Ownership chain too deep");
}
}
}
42 changes: 42 additions & 0 deletions contracts/core/erc6551/ERC6551OpenfortProxy.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// SPDX-License-Identifier: GPL-3.0
pragma solidity =0.8.19;

import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";

error InvalidImplementation();

/**
* @title ERC6551OpenfortProxy (Non-upgradeable)
* @notice Contract to create the ERC6551 proxies
* It inherits from:
* - ERC1967Proxy
*/
contract ERC6551OpenfortProxy is ERC1967Proxy {
address internal immutable defaultImplementation;

constructor(address _logic, bytes memory _data) ERC1967Proxy(_logic, _data) {
if (_logic == address(0)) revert InvalidImplementation();
defaultImplementation = _logic;
}

// constructor(address _logic, bytes memory _data) ERC1967Proxy(_logic, _data) {}

function implementation() external view returns (address) {
return _implementation();
}

function _implementation() internal view virtual override returns (address implementationAddress) {
implementationAddress = _getImplementation();
if (implementationAddress == address(0)) return defaultImplementation;
}

function _beforeFallback() internal virtual override {
super._beforeFallback();
if (msg.data.length == 0) {
if (_getImplementation() == address(0)) {
_upgradeTo(defaultImplementation);
_delegate(defaultImplementation);
}
}
}
}
Loading

0 comments on commit b5f3c5e

Please sign in to comment.