Skip to content

Commit

Permalink
Limit IPAccount Storage Writes to Registered Modules (storyprotocol#103)
Browse files Browse the repository at this point in the history 
* only register modules or registry can write to IPAccount

* deploy with GREATE3

* add modifier
  • Loading branch information
@kingster-will
kingster-will authored Apr 15, 2024
1 parent f7ddbdf commit 6d7bed4
Show file tree
Hide file tree
Showing 8 changed files with 252 additions and 94 deletions.
17 changes: 11 additions & 6 deletions contracts/IPAccountImpl.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { IPAccountStorage } from "./IPAccountStorage.sol";
/// @title IPAccountImpl
/// @notice The Story Protocol's implementation of the IPAccount.
contract IPAccountImpl is IPAccountStorage, IIPAccount {
address public immutable accessController;
address public immutable ACCESS_CONTROLLER;

/// @notice Returns the IPAccount's internal nonce for transaction ordering.
uint256 public state;
Expand All @@ -30,10 +30,15 @@ contract IPAccountImpl is IPAccountStorage, IIPAccount {
/// 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_) {
if (accessController_ == address(0)) revert Errors.IPAccount__InvalidAccessController();
accessController = accessController_;
/// @param accessController The address of the AccessController contract to be used for permission checks
constructor(
address accessController,
address ipAssetRegistry,
address licenseRegistry,
address moduleRegistry
) IPAccountStorage(ipAssetRegistry, licenseRegistry, moduleRegistry) {
if (accessController == address(0)) revert Errors.IPAccount__InvalidAccessController();
ACCESS_CONTROLLER = accessController;
}

/// @notice Checks if the contract supports a specific interface
Expand Down Expand Up @@ -103,7 +108,7 @@ contract IPAccountImpl is IPAccountStorage, IIPAccount {
selector = bytes4(data[:4]);
}
// the check will revert if permission is denied
IAccessController(accessController).checkPermission(address(this), signer, to, selector);
IAccessController(ACCESS_CONTROLLER).checkPermission(address(this), signer, to, selector);
return true;
}

Expand Down
31 changes: 25 additions & 6 deletions contracts/IPAccountStorage.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
pragma solidity ^0.8.23;

import { IIPAccountStorage } from "./interfaces/IIPAccountStorage.sol";
import { IModuleRegistry } from "./interfaces/registries/IModuleRegistry.sol";
import { Errors } from "./lib/Errors.sol";
import { ERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol";
import { ShortString, ShortStrings } from "@openzeppelin/contracts/utils/ShortStrings.sol";
/// @title IPAccount Storage
Expand All @@ -13,15 +15,32 @@ import { ShortString, ShortStrings } from "@openzeppelin/contracts/utils/ShortSt
contract IPAccountStorage is ERC165, IIPAccountStorage {
using ShortStrings for *;

mapping(bytes32 => mapping(bytes32 => string)) public stringData;
address public immutable MODULE_REGISTRY;
address public immutable LICENSE_REGISTRY;
address public immutable IP_ASSET_REGISTRY;

mapping(bytes32 => mapping(bytes32 => bytes)) public bytesData;
mapping(bytes32 => mapping(bytes32 => bytes32)) public bytes32Data;
mapping(bytes32 => mapping(bytes32 => uint256)) public uint256Data;
mapping(bytes32 => mapping(bytes32 => address)) public addressData;
mapping(bytes32 => mapping(bytes32 => bool)) public boolData;

modifier onlyRegisteredModule() {
if (
msg.sender != IP_ASSET_REGISTRY &&
msg.sender != LICENSE_REGISTRY &&
!IModuleRegistry(MODULE_REGISTRY).isRegistered(msg.sender)
) {
revert Errors.IPAccountStorage__NotRegisteredModule(msg.sender);
}
_;
}

constructor(address ipAssetRegistry, address licenseRegistry, address moduleRegistry) {
MODULE_REGISTRY = moduleRegistry;
LICENSE_REGISTRY = licenseRegistry;
IP_ASSET_REGISTRY = ipAssetRegistry;
}

/// @inheritdoc IIPAccountStorage
function setBytes(bytes32 key, bytes calldata value) external {
function setBytes(bytes32 key, bytes calldata value) external onlyRegisteredModule {
bytesData[_toBytes32(msg.sender)][key] = value;
}
/// @inheritdoc IIPAccountStorage
Expand All @@ -34,7 +53,7 @@ contract IPAccountStorage is ERC165, IIPAccountStorage {
}

/// @inheritdoc IIPAccountStorage
function setBytes32(bytes32 key, bytes32 value) external {
function setBytes32(bytes32 key, bytes32 value) external onlyRegisteredModule {
bytes32Data[_toBytes32(msg.sender)][key] = value;
}
/// @inheritdoc IIPAccountStorage
Expand Down
5 changes: 5 additions & 0 deletions contracts/lib/Errors.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ library Errors {
error IPAccount__InvalidCalldata();
error IPAccount__InvalidAccessController();

////////////////////////////////////////////////////////////////////////////
// IPAccountStorage //
////////////////////////////////////////////////////////////////////////////
error IPAccountStorage__NotRegisteredModule(address module);

////////////////////////////////////////////////////////////////////////////
// Module //
////////////////////////////////////////////////////////////////////////////
Expand Down
5 changes: 4 additions & 1 deletion script/foundry/utils/DeployHelper.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,10 @@ contract DeployHelper is Script, BroadcastManager, JsonDeploymentHandler, Storag
bytes memory ipAccountImplCode = abi.encodePacked(
type(IPAccountImpl).creationCode,
abi.encode(
address(accessController)
address(accessController),
address(ipAssetRegistry),
address(licenseRegistry),
address(moduleRegistry)
)
);
_predeploy(contractKey);
Expand Down
70 changes: 57 additions & 13 deletions test/foundry/IPAccountStorage.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,18 @@
pragma solidity ^0.8.23;

import { IIPAccount } from "../../contracts/interfaces/IIPAccount.sol";
import { BaseModule } from "../../contracts/modules/BaseModule.sol";
import { Errors } from "../../contracts/lib/Errors.sol";

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

contract IPAccountStorageTest is BaseTest {
contract IPAccountStorageTest is BaseTest, BaseModule {
MockModule public module;
IIPAccount public ipAccount;

string public override name = "IPAccountStorageTest";

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

Expand All @@ -19,6 +23,10 @@ contract IPAccountStorageTest is BaseTest {
uint256 tokenId = 100;
mockNFT.mintId(owner, tokenId);
ipAccount = IIPAccount(payable(ipAccountRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId)));
vm.startPrank(admin);
moduleRegistry.registerModule("MockModule", address(module));
moduleRegistry.registerModule("IPAccountStorageTest", address(this));
vm.stopPrank();
}

function test_IPAccountStorage_storeBytes() public {
Expand All @@ -27,10 +35,10 @@ contract IPAccountStorageTest is BaseTest {
}

function test_IPAccountStorage_readBytes_DifferentNamespace() public {
vm.prank(vm.addr(1));
vm.prank(address(module));
ipAccount.setBytes("test", abi.encodePacked("test"));
vm.prank(vm.addr(2));
assertEq(ipAccount.getBytes(_toBytes32(vm.addr(1)), "test"), "test");
assertEq(ipAccount.getBytes(_toBytes32(address(module)), "test"), "test");
}

function test_IPAccountStorage_storeAddressArray() public {
Expand All @@ -47,10 +55,10 @@ contract IPAccountStorageTest is BaseTest {
address[] memory addresses = new address[](2);
addresses[0] = vm.addr(1);
addresses[1] = vm.addr(2);
vm.prank(vm.addr(1));
vm.prank(address(module));
ipAccount.setBytes("test", abi.encode(addresses));
vm.prank(vm.addr(2));
address[] memory result = abi.decode(ipAccount.getBytes(_toBytes32(vm.addr(1)), "test"), (address[]));
address[] memory result = abi.decode(ipAccount.getBytes(_toBytes32(address(module)), "test"), (address[]));
assertEq(result[0], vm.addr(1));
assertEq(result[1], vm.addr(2));
}
Expand All @@ -69,10 +77,10 @@ contract IPAccountStorageTest is BaseTest {
uint256[] memory uints = new uint256[](2);
uints[0] = 1;
uints[1] = 2;
vm.prank(vm.addr(1));
vm.prank(address(module));
ipAccount.setBytes("test", abi.encode(uints));
vm.prank(vm.addr(2));
uint256[] memory result = abi.decode(ipAccount.getBytes(_toBytes32(vm.addr(1)), "test"), (uint256[]));
uint256[] memory result = abi.decode(ipAccount.getBytes(_toBytes32(address(module)), "test"), (uint256[]));
assertEq(result[0], 1);
assertEq(result[1], 2);
}
Expand All @@ -91,10 +99,10 @@ contract IPAccountStorageTest is BaseTest {
string[] memory strings = new string[](2);
strings[0] = "test1";
strings[1] = "test2";
vm.prank(vm.addr(1));
vm.prank(address(module));
ipAccount.setBytes("test", abi.encode(strings));
vm.prank(vm.addr(2));
string[] memory result = abi.decode(ipAccount.getBytes(_toBytes32(vm.addr(1)), "test"), (string[]));
string[] memory result = abi.decode(ipAccount.getBytes(_toBytes32(address(module)), "test"), (string[]));
assertEq(result[0], "test1");
assertEq(result[1], "test2");
}
Expand All @@ -105,10 +113,10 @@ contract IPAccountStorageTest is BaseTest {
}

function test_IPAccountStorage_readBytes32_differentNameSpace() public {
vm.prank(vm.addr(1));
vm.prank(address(module));
ipAccount.setBytes32("test", bytes32(uint256(111)));
vm.prank(vm.addr(2));
assertEq(ipAccount.getBytes32(_toBytes32(vm.addr(1)), "test"), bytes32(uint256(111)));
assertEq(ipAccount.getBytes32(_toBytes32(address(module)), "test"), bytes32(uint256(111)));
}

function test_IPAccountStorage_storeBytes32String() public {
Expand All @@ -117,10 +125,46 @@ contract IPAccountStorageTest is BaseTest {
}

function test_IPAccountStorage_readBytes32String_differentNameSpace() public {
vm.prank(vm.addr(1));
vm.prank(address(module));
ipAccount.setBytes32("test", "testData");
vm.prank(vm.addr(2));
assertEq(ipAccount.getBytes32(_toBytes32(vm.addr(1)), "test"), "testData");
assertEq(ipAccount.getBytes32(_toBytes32(address(module)), "test"), "testData");
}

function test_IPAccountStorage_setBytes32_revert_NonRegisteredModule() public {
vm.expectRevert(abi.encodeWithSelector(Errors.IPAccountStorage__NotRegisteredModule.selector, address(0x123)));
vm.prank(address(0x123));
ipAccount.setBytes32("test", "testData");
}

function test_IPAccountStorage_setBytes_revert_NonRegisteredModule() public {
vm.expectRevert(abi.encodeWithSelector(Errors.IPAccountStorage__NotRegisteredModule.selector, address(0x123)));
vm.prank(address(0x123));
ipAccount.setBytes("test", "testData");
}

function test_IPAccountStorage_setBytes_ByIpAssetRegistry() public {
vm.prank(address(ipAssetRegistry));
ipAccount.setBytes("test", "testData");
assertEq(ipAccount.getBytes(_toBytes32(address(ipAssetRegistry)), "test"), "testData");
}

function test_IPAccountStorage_setBytes32_ByIpAssetRegistry() public {
vm.prank(address(ipAssetRegistry));
ipAccount.setBytes32("test", "testData");
assertEq(ipAccount.getBytes32(_toBytes32(address(ipAssetRegistry)), "test"), "testData");
}

function test_IPAccountStorage_setBytes_ByLicenseRegistry() public {
vm.prank(address(licenseRegistry));
ipAccount.setBytes("test", "testData");
assertEq(ipAccount.getBytes(_toBytes32(address(licenseRegistry)), "test"), "testData");
}

function test_IPAccountStorage_setBytes32_ByLicenseRegistry() public {
vm.prank(address(licenseRegistry));
ipAccount.setBytes32("test", "testData");
assertEq(ipAccount.getBytes32(_toBytes32(address(licenseRegistry)), "test"), "testData");
}

function _toBytes32(address a) internal pure returns (bytes32) {
Expand Down
Loading

0 comments on commit 6d7bed4

Please sign in to comment.