Skip to content
This repository has been archived by the owner on Apr 30, 2024. It is now read-only.

Refactor Initialization process of IPAccount registration #108

Merged
merged 4 commits into from
Feb 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 12 additions & 9 deletions contracts/IPAccountImpl.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,23 @@ import { Errors } from "./lib/Errors.sol";
/// @title IPAccountImpl
/// @notice The Story Protocol's implementation of the IPAccount.
contract IPAccountImpl is IERC165, IIPAccount {
address public accessController;
address public immutable accessController;

uint256 public state;

receive() external payable override(IERC6551Account) {}

/// @notice Creates a new IPAccountImpl contract instance
/// @dev Initializes the IPAccountImpl with an AccessController address which is stored
/// in the implementation code's storage.
/// This means that each cloned IPAccount will inherently use the same AccessController
/// without the need for individual configuration.
/// @param accessController_ The address of the AccessController contract to be used for permission checks
constructor(address accessController_) {
LeoHChen marked this conversation as resolved.
Show resolved Hide resolved
if (accessController_ == address(0)) revert Errors.IPAccount__InvalidAccessController();
accessController = accessController_;
}

/// @notice Checks if the contract supports a specific interface
/// @param interfaceId_ The interface identifier, as specified in ERC-165
/// @return True if the contract supports the interface, false otherwise
Expand All @@ -34,14 +45,6 @@ contract IPAccountImpl is IERC165, IIPAccount {
interfaceId_ == type(IERC165).interfaceId);
}

/// @notice Initializes the IPAccount with the given access controller
/// @param accessController_ The address of the access controller
// TODO: can only be called by IPAccountRegistry
function initialize(address accessController_) external {
require(accessController_ != address(0), "Invalid access controller");
accessController = accessController_;
}

/// @notice Returns the identifier of the non-fungible token which owns the account
/// @return chainId The EIP-155 ID of the chain the token exists on
/// @return tokenContract The contract address of the token
Expand Down
1 change: 1 addition & 0 deletions contracts/lib/Errors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ library Errors {
error IPAccount__InvalidSignature();
error IPAccount__ExpiredSignature();
error IPAccount__InvalidCalldata();
error IPAccount__InvalidAccessController();

////////////////////////////////////////////////////////////////////////////
// Module //
Expand Down
12 changes: 1 addition & 11 deletions contracts/registries/IPAccountRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,15 @@ contract IPAccountRegistry is IIPAccountRegistry {
address public immutable IP_ACCOUNT_IMPL;
bytes32 public immutable IP_ACCOUNT_SALT;
address public immutable ERC6551_PUBLIC_REGISTRY;
address public immutable ACCESS_CONTROLLER;

/// @notice Constructor for the IPAccountRegistry contract.
/// @param erc6551Registry_ The address of the ERC6551 registry.
/// @param accessController_ The address of the access controller.
/// @param ipAccountImpl_ The address of the IP account implementation.
constructor(address erc6551Registry_, address accessController_, address ipAccountImpl_) {
constructor(address erc6551Registry_, address ipAccountImpl_) {
if (ipAccountImpl_ == address(0)) revert Errors.IPAccountRegistry_InvalidIpAccountImpl();
IP_ACCOUNT_IMPL = ipAccountImpl_;
IP_ACCOUNT_SALT = bytes32(0);
ERC6551_PUBLIC_REGISTRY = erc6551Registry_;
ACCESS_CONTROLLER = accessController_;
}

/// @notice Deploys an IPAccount contract with the IPAccount implementation and returns the address of the new IP.
Expand All @@ -38,20 +35,13 @@ contract IPAccountRegistry is IIPAccountRegistry {
address tokenContract_,
uint256 tokenId_
) public returns (address ipAccountAddress) {
bytes memory initData = abi.encodeWithSignature("initialize(address)", ACCESS_CONTROLLER);
ipAccountAddress = IERC6551Registry(ERC6551_PUBLIC_REGISTRY).createAccount(
IP_ACCOUNT_IMPL,
IP_ACCOUNT_SALT,
chainId_,
tokenContract_,
tokenId_
);
(bool success, bytes memory result) = ipAccountAddress.call(initData);
if (!success) {
assembly {
revert(add(result, 32), mload(result))
}
}
emit IPAccountRegistered(ipAccountAddress, IP_ACCOUNT_IMPL, chainId_, tokenContract_, tokenId_);
}

Expand Down
4 changes: 1 addition & 3 deletions contracts/registries/IPAssetRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,16 @@ contract IPAssetRegistry is IIPAssetRegistry, IPAccountRegistry, Governable {

/// @notice Initializes the IP Asset Registry.
/// @param erc6551Registry The address of the ERC6551 registry.
/// @param accessController The address of the access controller.
/// @param ipAccountImpl The address of the IP account implementation.
/// @param moduleRegistry The address of the module registry.
/// @param governance The address of the governance contract.
/// TODO: Utilize module registry for fetching different modules.
constructor(
address accessController,
address erc6551Registry,
address ipAccountImpl,
address moduleRegistry,
address governance
) IPAccountRegistry(erc6551Registry, accessController, ipAccountImpl) Governable(governance) {
) IPAccountRegistry(erc6551Registry, ipAccountImpl) Governable(governance) {
MODULE_REGISTRY = IModuleRegistry(moduleRegistry);
_metadataProvider = IMetadataProviderMigratable(new MetadataProviderV1(address(this)));
}
Expand Down
4 changes: 1 addition & 3 deletions script/foundry/deployment/Main.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ contract Main is Script, BroadcastManager, JsonDeploymentHandler {

contractKey = "IPAccountImpl";
_predeploy(contractKey);
ipAccountImpl = new IPAccountImpl();
ipAccountImpl = new IPAccountImpl(address(accessController));
_postdeploy(contractKey, address(ipAccountImpl));

contractKey = "ModuleRegistry";
Expand All @@ -162,15 +162,13 @@ contract Main is Script, BroadcastManager, JsonDeploymentHandler {
_predeploy(contractKey);
ipAccountRegistry = new IPAccountRegistry(
ERC6551_REGISTRY,
address(accessController),
address(ipAccountImpl)
);
_postdeploy(contractKey, address(ipAccountRegistry));

contractKey = "IPAssetRegistry";
_predeploy(contractKey);
ipAssetRegistry = new IPAssetRegistry(
address(accessController),
ERC6551_REGISTRY,
address(ipAccountImpl),
address(moduleRegistry),
Expand Down
8 changes: 2 additions & 6 deletions test/foundry/AccessController.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,8 @@ contract AccessControllerTest is Test {
function setUp() public {
governance = new Governance(address(this));
accessController = new AccessController(address(governance));
implementation = new IPAccountImpl();
ipAccountRegistry = new IPAccountRegistry(
address(erc6551Registry),
address(accessController),
address(implementation)
);
implementation = new IPAccountImpl(address(accessController));
ipAccountRegistry = new IPAccountRegistry(address(erc6551Registry), address(implementation));
moduleRegistry = new ModuleRegistry(address(governance));
accessController.initialize(address(ipAccountRegistry), address(moduleRegistry));
nft.mintId(owner, tokenId);
Expand Down
4 changes: 2 additions & 2 deletions test/foundry/IPAccount.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ contract IPAccountTest is Test {
function setUp() public {
governance = new Governance(address(this));
moduleRegistry = new ModuleRegistry(address(governance));
implementation = new IPAccountImpl();
registry = new IPAccountRegistry(address(erc6551Registry), address(accessController), address(implementation));
implementation = new IPAccountImpl(address(accessController));
registry = new IPAccountRegistry(address(erc6551Registry), address(implementation));
module = new MockModule(address(registry), address(moduleRegistry), "MockModule");
}

Expand Down
4 changes: 2 additions & 2 deletions test/foundry/IPAccountMetaTx.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ contract IPAccountMetaTxTest is Test {
owner = vm.addr(ownerPrivateKey);
caller = vm.addr(callerPrivateKey);

implementation = new IPAccountImpl();
registry = new IPAccountRegistry(address(erc6551Registry), address(accessController), address(implementation));
implementation = new IPAccountImpl(address(accessController));
registry = new IPAccountRegistry(address(erc6551Registry), address(implementation));
accessController.initialize(address(registry), address(moduleRegistry));
module = new MockModule(address(registry), address(moduleRegistry), "Module1WithPermission");
metaTxModule = new MockMetaTxModule(address(registry), address(moduleRegistry), address(accessController));
Expand Down
8 changes: 2 additions & 6 deletions test/foundry/access/AccessControlled.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,8 @@ contract AccessControlledTest is Test {
function setUp() public {
governance = new Governance(address(this));
accessController = new AccessController(address(governance));
implementation = new IPAccountImpl();
ipAccountRegistry = new IPAccountRegistry(
address(erc6551Registry),
address(accessController),
address(implementation)
);
implementation = new IPAccountImpl(address(accessController));
ipAccountRegistry = new IPAccountRegistry(address(erc6551Registry), address(implementation));
moduleRegistry = new ModuleRegistry(address(governance));
accessController.initialize(address(ipAccountRegistry), address(moduleRegistry));
nft.mintId(owner, tokenId);
Expand Down
8 changes: 2 additions & 6 deletions test/foundry/governance/Governance.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,8 @@ contract GovernanceTest is Test {
function setUp() public {
governance = new Governance(address(this));
accessController = new AccessController(address(governance));
implementation = new IPAccountImpl();
ipAccountRegistry = new IPAccountRegistry(
address(erc6551Registry),
address(accessController),
address(implementation)
);
implementation = new IPAccountImpl(address(accessController));
ipAccountRegistry = new IPAccountRegistry(address(erc6551Registry), address(implementation));
moduleRegistry = new ModuleRegistry(address(governance));
accessController.initialize(address(ipAccountRegistry), address(moduleRegistry));
nft.mintId(owner, tokenId);
Expand Down
6 changes: 3 additions & 3 deletions test/foundry/modules/licensing/LicensingModule.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pragma solidity ^0.8.23;
import { Strings } from "@openzeppelin/contracts/utils/Strings.sol";

// contracts
import { IPAccountImpl } from "contracts/IPAccountImpl.sol";
import { IIPAccount } from "contracts/interfaces/IIPAccount.sol";
import { AccessPermission } from "contracts/lib/AccessPermission.sol";
import { Errors } from "contracts/lib/Errors.sol";
import { Licensing } from "contracts/lib/Licensing.sol";
Expand Down Expand Up @@ -255,7 +255,7 @@ contract LicensingModuleTest is BaseTest {
function test_LicensingModule_mintLicense_revert_callerNotLicensorAndIpIdHasNoPolicy() public {
licensingModule.registerPolicyFrameworkManager(address(mockPFM));

IPAccountImpl ipAccount1 = IPAccountImpl(payable(ipId1));
IIPAccount ipAccount1 = IIPAccount(payable(ipId1));

vm.prank(address(mockPFM));
uint256 policyId = licensingModule.registerPolicy(true, address(mockRoyaltyPolicyLAP), "", _createMockPolicy());
Expand Down Expand Up @@ -290,7 +290,7 @@ contract LicensingModuleTest is BaseTest {
licensingModule.registerPolicyFrameworkManager(address(mockPFM));

bytes memory policy = _createMockPolicy();
IPAccountImpl ipAccount1 = IPAccountImpl(payable(ipId1));
IIPAccount ipAccount1 = IIPAccount(payable(ipId1));

vm.startPrank(address(mockPFM));
uint256 policyId1 = licensingModule.registerPolicy(true, address(mockRoyaltyPolicyLAP), "", policy);
Expand Down
11 changes: 2 additions & 9 deletions test/foundry/registries/IPAccountRegistry.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,16 @@ contract RegistryTest is Test {
uint256 internal tokenId;

function setUp() public {
implementation = new IPAccountImpl();
erc6551Registry = new ERC6551Registry();
accessController = new MockAccessController();
implementation = new IPAccountImpl(address(accessController));
chainId = 100;
tokenAddress = address(200);
tokenId = 300;
}

function test_IPAccountRegistry_registerIpAccount() public {
registry = new IPAccountRegistry(address(erc6551Registry), address(accessController), address(implementation));
registry = new IPAccountRegistry(address(erc6551Registry), address(implementation));
address ipAccountAddr;
ipAccountAddr = registry.registerIpAccount(chainId, tokenAddress, tokenId);

Expand All @@ -48,11 +48,4 @@ contract RegistryTest is Test {

assertTrue(registry.isRegistered(chainId, tokenAddress, tokenId));
}

function test_IPAccountRegistry_revert_createAccount_ifInitFailed() public {
// expect init revert for invalid accessController address
registry = new IPAccountRegistry(address(erc6551Registry), address(0), address(implementation));
vm.expectRevert("Invalid access controller");
registry.registerIpAccount(chainId, tokenAddress, tokenId);
}
}
14 changes: 7 additions & 7 deletions test/foundry/utils/DeployHelper.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,13 @@ contract DeployHelper {
DeployConditions memory dc = deployConditions; // alias

erc6551Registry = new ERC6551Registry();
ipAccountImpl = new IPAccountImpl();

_deployMockAssets();

_deployAccessConditionally(dc.access);

ipAccountImpl = new IPAccountImpl(address(accessController));

_deployRegistryConditionally(dc.registry);
// Registration module requires ipResolver
_deployIPResolverConditionally(dc.misc);
Expand Down Expand Up @@ -230,6 +232,9 @@ contract DeployHelper {
// TODO: Use mock IPAccountRegistry, instead of forcing deployment of actual IPAccountRegistry
// contract when using AccessController.
// deployConditions.registry.ipAccountRegistry = true;
} else {
accessController = new MockAccessController();
console2.log("DeployHelper: Using Mock AccessController");
}
}

Expand All @@ -241,16 +246,11 @@ contract DeployHelper {
}

// TODO: Allow using mock IPAccountRegistry, instead of forcing deployment of actual IPAccountRegistry.
ipAccountRegistry = new IPAccountRegistry(
address(erc6551Registry),
getAccessController(),
address(ipAccountImpl)
);
ipAccountRegistry = new IPAccountRegistry(address(erc6551Registry), address(ipAccountImpl));
console2.log("DeployHelper: Using REAL IPAccountRegistry");

// TODO: Allow using mock IPAssetRegistry, instead of forcing deployment of actual IPAssetRegistry.
ipAssetRegistry = new IPAssetRegistry(
getAccessController(),
address(erc6551Registry),
address(ipAccountImpl),
getModuleRegistry(),
Expand Down
Loading