Skip to content

Commit

Permalink
Refactor registerIpAccount to be Internal function (#155)
Browse files Browse the repository at this point in the history 
* Make registerIpAccount be internal function

* fix lint
  • Loading branch information
@kingster-will
kingster-will authored Jun 27, 2024
1 parent ae20c7d commit 3007656
Show file tree
Hide file tree
Showing 15 changed files with 96 additions and 79 deletions.
17 changes: 0 additions & 17 deletions contracts/interfaces/registries/IIPAccountRegistry.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,6 @@ interface IIPAccountRegistry {
uint256 tokenId
);

/// @notice Returns the IPAccount implementation address
function IP_ACCOUNT_IMPL() external view returns (address);

/// @notice Returns the IPAccount salt
function IP_ACCOUNT_SALT() external view returns (bytes32);

/// @notice Returns the public ERC6551 registry address
function ERC6551_PUBLIC_REGISTRY() external view returns (address);

/// @notice Deploys an IPAccount contract with the IPAccount implementation and returns the address of the new IP
/// @dev The IPAccount deployment deltegates to public ERC6551 Registry
/// @param chainId The chain ID where the IP Account will be created
/// @param tokenContract The address of the token contract to be associated with the IP Account
/// @param tokenId The ID of the token to be associated with the IP Account
/// @return ipAccountAddress The address of the newly created IP Account
function registerIpAccount(uint256 chainId, address tokenContract, uint256 tokenId) external returns (address);

/// @notice Returns the IPAccount address for the given NFT token.
/// @param chainId The chain ID where the IP Account is located
/// @param tokenContract The address of the token contract associated with the IP Account
Expand Down
38 changes: 19 additions & 19 deletions contracts/registries/IPAccountRegistry.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,32 @@ abstract contract IPAccountRegistry is IIPAccountRegistry {
ERC6551_PUBLIC_REGISTRY = erc6551Registry;
}

/// @notice Deploys an IPAccount contract with the IPAccount implementation and returns the address of the new IP
/// @dev The IPAccount deployment deltegates to public ERC6551 Registry
/// @notice Returns the IPAccount address for the given NFT token.
/// @param chainId The chain ID where the IP Account is located
/// @param tokenContract The address of the token contract associated with the IP Account
/// @param tokenId The ID of the token associated with the IP Account
/// @return ipAccountAddress The address of the IP Account associated with the given NFT token
function ipAccount(uint256 chainId, address tokenContract, uint256 tokenId) public view returns (address) {
return _get6551AccountAddress(chainId, tokenContract, tokenId);
}

/// @notice Returns the IPAccount implementation address.
/// @return The address of the IPAccount implementation
function getIPAccountImpl() external view override returns (address) {
return IP_ACCOUNT_IMPL;
}

/// @dev Deploys an IPAccount contract with the IPAccount implementation and returns the address of the new IP
/// The IPAccount deployment delegates to public ERC6551 Registry
/// @param chainId The chain ID where the IP Account will be created
/// @param tokenContract The address of the token contract to be associated with the IP Account
/// @param tokenId The ID of the token to be associated with the IP Account
/// @return ipAccountAddress The address of the newly created IP Account
function registerIpAccount(
function _registerIpAccount(
uint256 chainId,
address tokenContract,
uint256 tokenId
) public returns (address ipAccountAddress) {
) internal returns (address ipAccountAddress) {
ipAccountAddress = IERC6551Registry(ERC6551_PUBLIC_REGISTRY).createAccount(
IP_ACCOUNT_IMPL,
IP_ACCOUNT_SALT,
Expand All @@ -52,21 +67,6 @@ abstract contract IPAccountRegistry is IIPAccountRegistry {
emit IPAccountRegistered(ipAccountAddress, IP_ACCOUNT_IMPL, chainId, tokenContract, tokenId);
}

/// @notice Returns the IPAccount address for the given NFT token.
/// @param chainId The chain ID where the IP Account is located
/// @param tokenContract The address of the token contract associated with the IP Account
/// @param tokenId The ID of the token associated with the IP Account
/// @return ipAccountAddress The address of the IP Account associated with the given NFT token
function ipAccount(uint256 chainId, address tokenContract, uint256 tokenId) public view returns (address) {
return _get6551AccountAddress(chainId, tokenContract, tokenId);
}

/// @notice Returns the IPAccount implementation address.
/// @return The address of the IPAccount implementation
function getIPAccountImpl() external view override returns (address) {
return IP_ACCOUNT_IMPL;
}

/// @dev Helper function to get the IPAccount address from the ERC6551 registry.
function _get6551AccountAddress(
uint256 chainId,
Expand Down
2 changes: 1 addition & 1 deletion contracts/registries/IPAssetRegistry.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ contract IPAssetRegistry is IIPAssetRegistry, IPAccountRegistry, ProtocolPausabl
address tokenContract,
uint256 tokenId
) external whenNotPaused returns (address id) {
id = registerIpAccount(chainid, tokenContract, tokenId);
id = _registerIpAccount(chainid, tokenContract, tokenId);
IIPAccount ipAccount = IIPAccount(payable(id));

if (bytes(ipAccount.getString("NAME")).length != 0) {
Expand Down
34 changes: 24 additions & 10 deletions test/foundry/IPAccount.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,31 @@ import { ERC6551 } from "@solady/src/accounts/ERC6551.sol";

import { IIPAccount } from "../../contracts/interfaces/IIPAccount.sol";
import { Errors } from "../../contracts/lib/Errors.sol";
import { IPAccountRegistry } from "../../contracts/registries/IPAccountRegistry.sol";

import { MockModule } from "./mocks/module/MockModule.sol";
import { BaseTest } from "./utils/BaseTest.t.sol";

contract MockIPAccountRegistry is IPAccountRegistry {
constructor(address erc6551Registry, address ipAccountImpl) IPAccountRegistry(erc6551Registry, ipAccountImpl) {}

function registerIpAccount(uint256 chainId, address tokenContract, uint256 tokenId) public returns (address) {
return _registerIpAccount(chainId, tokenContract, tokenId);
}
}

contract IPAccountTest is BaseTest {
MockModule public module;
MockIPAccountRegistry public mockIpAccountRegistry;

function setUp() public override {
super.setUp();

module = new MockModule(address(ipAssetRegistry), address(moduleRegistry), "MockModule");
mockIpAccountRegistry = new MockIPAccountRegistry(
ipAccountRegistry.ERC6551_PUBLIC_REGISTRY(),
ipAccountRegistry.IP_ACCOUNT_IMPL()
);

vm.startPrank(u.admin); // used twice, name() and registerModule()
moduleRegistry.registerModule(module.name(), address(module));
Expand All @@ -33,14 +47,14 @@ contract IPAccountTest is BaseTest {

vm.prank(owner, owner);

address deployedAccount = ipAssetRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address deployedAccount = mockIpAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);

assertTrue(deployedAccount != address(0));

assertEq(predictedAccount, deployedAccount);

// Create account twice
deployedAccount = ipAssetRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
deployedAccount = mockIpAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
assertEq(predictedAccount, deployedAccount);
}

Expand All @@ -51,7 +65,7 @@ contract IPAccountTest is BaseTest {
mockNFT.mintId(owner, tokenId);

vm.prank(owner, owner);
address account = ipAssetRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = mockIpAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);

IIPAccount ipAccount = IIPAccount(payable(account));

Expand Down Expand Up @@ -86,7 +100,7 @@ contract IPAccountTest is BaseTest {
mockNFT.mintId(owner, tokenId);

vm.prank(owner, owner);
address account = ipAssetRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = mockIpAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);

uint256 subTokenId = 111;
mockNFT.mintId(account, subTokenId);
Expand Down Expand Up @@ -204,7 +218,7 @@ contract IPAccountTest is BaseTest {

mockNFT.mintId(owner, tokenId);

address account = ipAssetRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = mockIpAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);

uint256 subTokenId = 111;
mockNFT.mintId(account, subTokenId);
Expand All @@ -231,7 +245,7 @@ contract IPAccountTest is BaseTest {

mockNFT.mintId(owner, tokenId);

address account = ipAssetRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = mockIpAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);

uint256 subTokenId = 111;
mockNFT.mintId(account, subTokenId);
Expand All @@ -251,7 +265,7 @@ contract IPAccountTest is BaseTest {
mockNFT.mintId(owner, tokenId);

vm.prank(owner, owner);
address account = ipAssetRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = mockIpAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);

address otherOwner = vm.addr(2);
uint256 otherTokenId = 200;
Expand All @@ -269,7 +283,7 @@ contract IPAccountTest is BaseTest {
mockNFT.mintId(owner, tokenId);

vm.prank(owner, owner);
address account = ipAssetRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = mockIpAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);

ERC6551 ipAccount = ERC6551(payable(account));

Expand Down Expand Up @@ -304,7 +318,7 @@ contract IPAccountTest is BaseTest {
mockNFT.mintId(owner, tokenId);

vm.prank(owner, owner);
address account = ipAssetRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = mockIpAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);

ERC6551 ipAccount = ERC6551(payable(account));

Expand All @@ -325,7 +339,7 @@ contract IPAccountTest is BaseTest {
mockNFT.mintId(owner, tokenId);

vm.prank(owner, owner);
address account = ipAssetRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = mockIpAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);

ERC6551 ipAccount = ERC6551(payable(account));

Expand Down
2 changes: 1 addition & 1 deletion test/foundry/IPAccountImpl.btt.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ contract IPAccountImplBTT is BaseTest {

ipOwner = u.alice;
mockNFT.mintId(ipOwner, tokenId);
ipAcct = IIPAccount(payable(ipAssetRegistry.registerIpAccount(chainId, address(mockNFT), tokenId)));
ipAcct = IIPAccount(payable(ipAssetRegistry.register(chainId, address(mockNFT), tokenId)));
}

function test_IPAccountImpl_supportsInterface() public {
Expand Down
26 changes: 13 additions & 13 deletions test/foundry/IPAccountMetaTx.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ contract IPAccountMetaTxTest is BaseTest {

mockNFT.mintId(owner, tokenId);

address account = ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId);

IIPAccount ipAccount = IIPAccount(payable(account));

Expand Down Expand Up @@ -117,7 +117,7 @@ contract IPAccountMetaTxTest is BaseTest {

mockNFT.mintId(owner, tokenId);

address account = ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId);

IIPAccount ipAccount = IIPAccount(payable(account));

Expand Down Expand Up @@ -193,7 +193,7 @@ contract IPAccountMetaTxTest is BaseTest {

mockNFT.mintId(owner, tokenId);

address account = ipAssetRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId);

IIPAccount ipAccount = IIPAccount(payable(account));

Expand Down Expand Up @@ -258,7 +258,7 @@ contract IPAccountMetaTxTest is BaseTest {

mockNFT.mintId(owner, tokenId);

address account = ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId);

IIPAccount ipAccount = IIPAccount(payable(account));

Expand Down Expand Up @@ -305,7 +305,7 @@ contract IPAccountMetaTxTest is BaseTest {

mockNFT.mintId(owner, tokenId);

address account = ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId);

IIPAccount ipAccount = IIPAccount(payable(account));

Expand Down Expand Up @@ -351,7 +351,7 @@ contract IPAccountMetaTxTest is BaseTest {

mockNFT.mintId(owner, tokenId);

address account = ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId);

IIPAccount ipAccount = IIPAccount(payable(account));

Expand Down Expand Up @@ -396,7 +396,7 @@ contract IPAccountMetaTxTest is BaseTest {

mockNFT.mintId(owner, tokenId);

address account = ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId);

IIPAccount ipAccount = IIPAccount(payable(account));

Expand Down Expand Up @@ -442,13 +442,13 @@ contract IPAccountMetaTxTest is BaseTest {

mockNFT.mintId(owner, tokenId);

address account = ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId);

IIPAccount ipAccount = IIPAccount(payable(account));

uint256 tokenId2 = 101;
mockNFT.mintId(owner, tokenId2);
address account2 = ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId2);
address account2 = ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId2);
IIPAccount ipAccount2 = IIPAccount(payable(account2));

uint deadline = block.timestamp + 1000;
Expand Down Expand Up @@ -482,7 +482,7 @@ contract IPAccountMetaTxTest is BaseTest {

mockNFT.mintId(owner, tokenId);

address account = ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId);

IIPAccount ipAccount = IIPAccount(payable(account));

Expand Down Expand Up @@ -536,7 +536,7 @@ contract IPAccountMetaTxTest is BaseTest {

mockNFT.mintId(owner, tokenId);

address account = ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId);

IIPAccount ipAccount = IIPAccount(payable(account));

Expand Down Expand Up @@ -583,7 +583,7 @@ contract IPAccountMetaTxTest is BaseTest {

mockNFT.mintId(owner, tokenId);

address account = ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId);

IIPAccount ipAccount = IIPAccount(payable(account));

Expand Down Expand Up @@ -628,7 +628,7 @@ contract IPAccountMetaTxTest is BaseTest {

mockNFT.mintId(owner, tokenId);

address account = ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address account = ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId);

IIPAccount ipAccount = IIPAccount(payable(account));

Expand Down
2 changes: 1 addition & 1 deletion test/foundry/IPAccountStorage.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ contract IPAccountStorageTest is BaseTest, BaseModule {
address owner = vm.addr(1);
uint256 tokenId = 100;
mockNFT.mintId(owner, tokenId);
ipAccount = IIPAccount(payable(ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId)));
ipAccount = IIPAccount(payable(ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId)));
vm.startPrank(admin);
moduleRegistry.registerModule("MockModule", address(module));
moduleRegistry.registerModule("IPAccountStorageTest", address(this));
Expand Down
2 changes: 1 addition & 1 deletion test/foundry/IPAccountStorageOps.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ contract IPAccountStorageOpsTest is BaseTest, BaseModule {
address owner = vm.addr(1);
uint256 tokenId = 100;
mockNFT.mintId(owner, tokenId);
ipAccount = IIPAccount(payable(ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId)));
ipAccount = IIPAccount(payable(ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId)));
vm.startPrank(admin);
moduleRegistry.registerModule("MockModule", address(module));
moduleRegistry.registerModule("IPAccountStorageOpsTest", address(this));
Expand Down
4 changes: 2 additions & 2 deletions test/foundry/access/AccessControlled.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ contract AccessControlledTest is BaseTest {
super.setUp();

mockNFT.mintId(owner, tokenId);
address deployedAccount = ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address deployedAccount = ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId);
ipAccount = IIPAccount(payable(deployedAccount));

mockModule = new MockAccessControlledModule(
Expand Down Expand Up @@ -117,7 +117,7 @@ contract AccessControlledTest is BaseTest {

function test_AccessControlled_revert_callIpAccountOrPermissionFunction_withOtherIpAccount() public {
mockNFT.mintId(owner, 101);
address otherIpAccountAddr = ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), 101);
address otherIpAccountAddr = ipAssetRegistry.register(block.chainid, address(mockNFT), 101);
IIPAccount otherIpAccount = IIPAccount(payable(otherIpAccountAddr));
vm.expectRevert(
abi.encodeWithSelector(
Expand Down
2 changes: 1 addition & 1 deletion test/foundry/access/AccessController.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ contract AccessControllerTest is BaseTest {
super.setUp();

mockNFT.mintId(owner, tokenId);
address deployedAccount = ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
address deployedAccount = ipAssetRegistry.register(block.chainid, address(mockNFT), tokenId);
ipAccount = IIPAccount(payable(deployedAccount));

mockModule = new MockModule(address(ipAccountRegistry), address(moduleRegistry), "MockModule");
Expand Down
4 changes: 2 additions & 2 deletions test/foundry/modules/external/TokenWithdrawalModule.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ contract TokenWithdrawalModuleTest is BaseTest {
mockNFT.mintId(alice, 1);
mockNFT.mintId(alice, 2);

ipAcct1 = IIPAccount(payable(ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), 1)));
ipAcct2 = IIPAccount(payable(ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), 2)));
ipAcct1 = IIPAccount(payable(ipAssetRegistry.register(block.chainid, address(mockNFT), 1)));
ipAcct2 = IIPAccount(payable(ipAssetRegistry.register(block.chainid, address(mockNFT), 2)));

vm.label(address(ipAcct1), "IPAccount1");
vm.label(address(ipAcct2), "IPAccount2");
Expand Down
Loading

0 comments on commit 3007656

Please sign in to comment.