Skip to content

Commit

Permalink
fix: address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
jaypaik committed Jan 27, 2024
1 parent 1eb130d commit 1d35562
Show file tree
Hide file tree
Showing 7 changed files with 249 additions and 55 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Alchemy's Modular Account is a maximally modular, upgradeable Smart Contract Acc
This repository contains:

- [Modular Account implementation](src/account)
- [Modular Account factories](src/factory)
- [Modular Account factory](src/factory)
- 2 ERC-6900 compatible plugins:
- [MultiOwnerPlugin](src/plugins/owner) is a plugin supporting 1+ ECDSA owners.
- [SessionKeyPlugin](src/plugins/session) enables session keys with optional permissions such as time ranges, token spend limits, and gas spend limits.
Expand Down
2 changes: 1 addition & 1 deletion foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ out = 'out'
test = 'test'
libs = ['lib']
optimizer = true
optimizer_runs = 1600
optimizer_runs = 1000
ignored_error_codes = []

[fuzz]
Expand Down
64 changes: 64 additions & 0 deletions src/account/TokenReceiver.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// This file is part of Modular Account.
//
// Copyright 2024 Alchemy Insights, Inc.
//
// SPDX-License-Identifier: GPL-3.0-or-later
//
// This program is free software: you can redistribute it and/or modify it under the terms of the GNU General
// Public License as published by the Free Software Foundation, either version 3 of the License, or (at your
// option) any later version.
//
// This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the
// implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
// more details.
//
// You should have received a copy of the GNU General Public License along with this program. If not, see
// <https://www.gnu.org/licenses/>.

pragma solidity ^0.8.22;

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

abstract contract TokenReceiver is IERC165, IERC721Receiver, IERC777Recipient, IERC1155Receiver {
/// @inheritdoc IERC777Recipient
function tokensReceived(address, address, address, uint256, bytes calldata, bytes calldata)
external
pure
override
// solhint-disable-next-line no-empty-blocks
{}

/// @inheritdoc IERC721Receiver
function onERC721Received(address, address, uint256, bytes calldata) external pure override returns (bytes4) {
return IERC721Receiver.onERC721Received.selector;
}

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

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

/// @inheritdoc IERC165
function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {
return interfaceId == type(IERC721Receiver).interfaceId
|| interfaceId == type(IERC777Recipient).interfaceId || interfaceId == type(IERC1155Receiver).interfaceId;
}
}
63 changes: 11 additions & 52 deletions src/account/UpgradeableModularAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@

pragma solidity ^0.8.22;

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

import {UUPSUpgradeable} from "../../ext/UUPSUpgradeable.sol";
Expand All @@ -41,6 +38,7 @@ import {AccountExecutor} from "./AccountExecutor.sol";
import {AccountLoupe} from "./AccountLoupe.sol";
import {AccountStorageInitializable} from "./AccountStorageInitializable.sol";
import {PluginManagerInternals} from "./PluginManagerInternals.sol";
import {TokenReceiver} from "./TokenReceiver.sol";

/// @title Upgradeable Modular Account
/// @author Alchemy
Expand All @@ -50,13 +48,10 @@ contract UpgradeableModularAccount is
AccountLoupe,
AccountStorageInitializable,
PluginManagerInternals,
TokenReceiver,
IAccount,
IAccountInitializable,
IAccountView,
IERC165,
IERC721Receiver,
IERC777Recipient,
IERC1155Receiver,
IPluginExecutor,
IStandardExecutor,
UUPSUpgradeable
Expand Down Expand Up @@ -376,25 +371,22 @@ contract UpgradeableModularAccount is
_postNativeFunction(postExecHooks, postHookArgs);
}

/// @inheritdoc IERC165
function supportsInterface(bytes4 interfaceId) external view override returns (bool) {
if (interfaceId == _INTERFACE_ID_INVALID) {
return false;
}
if (interfaceId == _IERC165_INTERFACE_ID) {
return true;
}

return _getAccountStorage().supportedInterfaces[interfaceId] > 0;
}

/// @inheritdoc UUPSUpgradeable
function upgradeToAndCall(address newImplementation, bytes calldata data) public payable override onlyProxy {
(FunctionReference[][] memory postExecHooks, bytes[] memory postHookArgs) = _preNativeFunction();
UUPSUpgradeable.upgradeToAndCall(newImplementation, data);
_postNativeFunction(postExecHooks, postHookArgs);
}

/// @inheritdoc IERC165
function supportsInterface(bytes4 interfaceId) public view override returns (bool) {
if (interfaceId == _INTERFACE_ID_INVALID) {
return false;
}
return interfaceId == _IERC165_INTERFACE_ID || super.supportsInterface(interfaceId)
|| _getAccountStorage().supportedInterfaces[interfaceId] > 0;
}

/// @inheritdoc IAccountView
function entryPoint() public view override returns (IEntryPoint) {
return _ENTRY_POINT;
Expand All @@ -405,39 +397,6 @@ contract UpgradeableModularAccount is
return _ENTRY_POINT.getNonce(address(this), 0);
}

/// @inheritdoc IERC777Recipient
function tokensReceived(address, address, address, uint256, bytes calldata, bytes calldata)
external
pure
override
// solhint-disable-next-line no-empty-blocks
{}

/// @inheritdoc IERC721Receiver
function onERC721Received(address, address, uint256, bytes calldata) external pure override returns (bytes4) {
return IERC721Receiver.onERC721Received.selector;
}

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

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

// INTERNAL FUNCTIONS

/// @dev Wraps execution of a native function with runtime validation and hooks. Used for upgradeToAndCall,
Expand Down
2 changes: 1 addition & 1 deletion src/helpers/KnownSelectors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ library KnownSelectors {
|| selector == IAccountLoupe.getExecutionHooks.selector
|| selector == IAccountLoupe.getPreValidationHooks.selector
|| selector == IAccountLoupe.getInstalledPlugins.selector
// check against token receiver methods
// check against TokenReceiver methods
|| selector == IERC777Recipient.tokensReceived.selector
|| selector == IERC721Receiver.onERC721Received.selector
|| selector == IERC1155Receiver.onERC1155Received.selector
Expand Down
157 changes: 157 additions & 0 deletions test/account/TokenReceiver.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
// This file is part of Modular Account.
//
// Copyright 2024 Alchemy Insights, Inc.
//
// SPDX-License-Identifier: GPL-3.0-or-later
//
// This program is free software: you can redistribute it and/or modify it under the terms of the GNU General
// Public License as published by the Free Software Foundation, either version 3 of the License, or (at your
// option) any later version.
//
// This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the
// implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
// more details.
//
// You should have received a copy of the GNU General Public License along with this program. If not, see
// <https://www.gnu.org/licenses/>.

pragma solidity ^0.8.22;

import {Test} from "forge-std/Test.sol";

import {ERC721PresetMinterPauserAutoId} from
"@openzeppelin/contracts/token/ERC721/presets/ERC721PresetMinterPauserAutoId.sol";
import {EntryPoint} from "@eth-infinitism/account-abstraction/core/EntryPoint.sol";
import {IERC1155Receiver} from "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol";
import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
import {IERC777Recipient} from "@openzeppelin/contracts/token/ERC777/IERC777Recipient.sol";

import {UpgradeableModularAccount} from "../../src/account/UpgradeableModularAccount.sol";
import {MultiOwnerMSCAFactory} from "../../src/factory/MultiOwnerMSCAFactory.sol";
import {IEntryPoint} from "../../src/interfaces/erc4337/IEntryPoint.sol";
import {MultiOwnerPlugin} from "../../src/plugins/owner/MultiOwnerPlugin.sol";
import {MockERC1155} from "../mocks/tokens/MockERC1155.sol";
import {MockERC777} from "../mocks/tokens/MockERC777.sol";

contract TokenReceiverTest is Test, IERC1155Receiver {
UpgradeableModularAccount public acct;

ERC721PresetMinterPauserAutoId public t0;
MockERC777 public t1;
MockERC1155 public t2;
MultiOwnerMSCAFactory public factory;
MultiOwnerPlugin public multiOwnerPlugin;
IEntryPoint public entryPoint;

address public owner;
address[] public owners;

// init dynamic length arrays for use in args
address[] public defaultOperators;
uint256[] public tokenIds;
uint256[] public tokenAmts;
uint256[] public zeroTokenAmts;

uint256 internal constant _TOKEN_AMOUNT = 1 ether;
uint256 internal constant _TOKEN_ID = 0;
uint256 internal constant _BATCH_TOKEN_IDS = 5;

function setUp() public {
entryPoint = IEntryPoint(address(new EntryPoint()));
multiOwnerPlugin = new MultiOwnerPlugin();
factory = new MultiOwnerMSCAFactory(
address(this),
address(multiOwnerPlugin),
address(new UpgradeableModularAccount(entryPoint)),
keccak256(abi.encode(multiOwnerPlugin.pluginManifest())),
entryPoint
);
(owner,) = makeAddrAndKey("owner");
owners = new address[](1);
owners[0] = owner;
acct = UpgradeableModularAccount(payable(factory.createAccount(0, owners)));

t0 = new ERC721PresetMinterPauserAutoId("t0", "t0", "");
t0.mint(address(this));

t1 = new MockERC777();
t1.mint(address(this), _TOKEN_AMOUNT);

t2 = new MockERC1155();
t2.mint(address(this), _TOKEN_ID, _TOKEN_AMOUNT);
for (uint256 i = 1; i < _BATCH_TOKEN_IDS; i++) {
t2.mint(address(this), i, _TOKEN_AMOUNT);
tokenIds.push(i);
tokenAmts.push(_TOKEN_AMOUNT);
zeroTokenAmts.push(0);
}
}

function test_passERC721Transfer() public {
assertEq(t0.ownerOf(_TOKEN_ID), address(this));
t0.safeTransferFrom(address(this), address(acct), _TOKEN_ID);
assertEq(t0.ownerOf(_TOKEN_ID), address(acct));
}

function test_passERC777Transfer() public {
assertEq(t1.balanceOf(address(this)), _TOKEN_AMOUNT);
assertEq(t1.balanceOf(address(acct)), 0);
t1.transfer(address(acct), _TOKEN_AMOUNT);
assertEq(t1.balanceOf(address(this)), 0);
assertEq(t1.balanceOf(address(acct)), _TOKEN_AMOUNT);
}

function test_passERC1155Transfer() public {
assertEq(t2.balanceOf(address(this), _TOKEN_ID), _TOKEN_AMOUNT);
assertEq(t2.balanceOf(address(acct), _TOKEN_ID), 0);
t2.safeTransferFrom(address(this), address(acct), _TOKEN_ID, _TOKEN_AMOUNT, "");
assertEq(t2.balanceOf(address(this), _TOKEN_ID), 0);
assertEq(t2.balanceOf(address(acct), _TOKEN_ID), _TOKEN_AMOUNT);

for (uint256 i = 1; i < _BATCH_TOKEN_IDS; i++) {
assertEq(t2.balanceOf(address(this), i), _TOKEN_AMOUNT);
assertEq(t2.balanceOf(address(acct), i), 0);
}
t2.safeBatchTransferFrom(address(this), address(acct), tokenIds, tokenAmts, "");
for (uint256 i = 1; i < _BATCH_TOKEN_IDS; i++) {
assertEq(t2.balanceOf(address(this), i), 0);
assertEq(t2.balanceOf(address(acct), i), _TOKEN_AMOUNT);
}
}

function test_passIntrospection() public {
bool isSupported;

isSupported = acct.supportsInterface(type(IERC721Receiver).interfaceId);
assertEq(isSupported, true);
isSupported = acct.supportsInterface(type(IERC777Recipient).interfaceId);
assertEq(isSupported, true);
isSupported = acct.supportsInterface(type(IERC1155Receiver).interfaceId);
assertEq(isSupported, true);
}

/**
* NON-TEST FUNCTIONS - USED SO MINT DOESNT FAIL
*/
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) external pure override returns (bool) {
return false;
}
}
14 changes: 14 additions & 0 deletions test/helpers/KnownSelectors.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ import {Test} from "forge-std/Test.sol";
import {BaseAccount} from "@eth-infinitism/account-abstraction/core/BaseAccount.sol";
import {IAggregator} from "@eth-infinitism/account-abstraction/interfaces/IAggregator.sol";
import {IPaymaster} from "@eth-infinitism/account-abstraction/interfaces/IPaymaster.sol";
import {IERC1155Receiver} from "@openzeppelin/contracts/interfaces/IERC1155Receiver.sol";
import {IERC777Recipient} from "@openzeppelin/contracts/interfaces/IERC777Recipient.sol";
import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol";
import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol";

import {KnownSelectors} from "../../src/helpers/KnownSelectors.sol";
Expand Down Expand Up @@ -68,6 +71,17 @@ contract KnownSelectorsTest is Test {
assertTrue(KnownSelectors.isNativeFunction(IAccountLoupe.getPreValidationHooks.selector));
assertTrue(KnownSelectors.isNativeFunction(IAccountLoupe.getInstalledPlugins.selector));

// TokenReceiver methods
assertTrue(KnownSelectors.isNativeFunction(IAccountLoupe.getExecutionFunctionConfig.selector));
assertTrue(KnownSelectors.isNativeFunction(IAccountLoupe.getExecutionHooks.selector));
assertTrue(KnownSelectors.isNativeFunction(IAccountLoupe.getPreValidationHooks.selector));
assertTrue(KnownSelectors.isNativeFunction(IAccountLoupe.getInstalledPlugins.selector));

assertTrue(KnownSelectors.isNativeFunction(IERC777Recipient.tokensReceived.selector));
assertTrue(KnownSelectors.isNativeFunction(IERC721Receiver.onERC721Received.selector));
assertTrue(KnownSelectors.isNativeFunction(IERC1155Receiver.onERC1155Received.selector));
assertTrue(KnownSelectors.isNativeFunction(IERC1155Receiver.onERC1155BatchReceived.selector));

assertFalse(KnownSelectors.isNativeFunction(IPaymaster.validatePaymasterUserOp.selector));
}

Expand Down

0 comments on commit 1d35562

Please sign in to comment.