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

Cleanup and Minimize Base Module Attributes #81

Merged
merged 3 commits into from
Feb 7, 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
41 changes: 6 additions & 35 deletions contracts/modules/BaseModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,50 +3,21 @@
pragma solidity ^0.8.23;

import { IModule } from "../interfaces/modules/base/IModule.sol";
import { IAccessController } from "../interfaces/IAccessController.sol";
import { AccessControlled } from "../access/AccessControlled.sol";
import { IPAssetRegistry } from "../registries/IPAssetRegistry.sol";
import { LicenseRegistry } from "../registries/LicenseRegistry.sol";
import { Errors } from "../lib/Errors.sol";

/// @title BaseModule
/// @notice Base implementation for all modules in Story Protocol. This is to
/// ensure all modules share the same authorization through the access
/// controll manager.
abstract contract BaseModule is IModule {
/// @notice Gets the protocol-wide module access controller.
IAccessController public immutable ACCESS_CONTROLLER;

abstract contract BaseModule is IModule, AccessControlled {
/// @notice Gets the protocol-wide IP asset registry.
IPAssetRegistry public immutable IP_ASSET_REGISTRY;

/// @notice Gets the protocol-wide license registry.
LicenseRegistry public immutable LICENSE_REGISTRY;

/// @notice Modifier for authorizing the calling entity.
modifier onlyAuthorized(address ipId) {
_authenticate(ipId);
_;
}

/// @notice Initializes the base module contract.
/// @param controller The access controller used for IP authorization.
/// @param assetRegistry The address of the IP asset registry.
/// @param licenseRegistry The address of the license registry.
constructor(address controller, address assetRegistry, address licenseRegistry) {
// TODO: Add checks for interface support or at least zero address
ACCESS_CONTROLLER = IAccessController(controller);
IP_ASSET_REGISTRY = IPAssetRegistry(assetRegistry);
LICENSE_REGISTRY = LicenseRegistry(licenseRegistry);
}

/// @notice Gets the protocol string identifier associated with the module.
/// @return The string identifier of the module.
function name() public pure virtual override returns (string memory);

/// @notice Authenticates the caller entity through the access controller.
function _authenticate(address ipId) internal view {
try ACCESS_CONTROLLER.checkPermission(ipId, msg.sender, address(this), msg.sig) {} catch {
revert Errors.Module_Unauthorized();
}
/// @param accessController The access controller used for IP authorization.
/// @param ipAssetRegistry The address of the IP asset registry.
constructor(address accessController, address ipAssetRegistry) AccessControlled(accessController, ipAssetRegistry) {
IP_ASSET_REGISTRY = IPAssetRegistry(ipAssetRegistry);
}
}
4 changes: 1 addition & 3 deletions contracts/modules/RegistrationModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,12 @@ contract RegistrationModule is BaseModule, IRegistrationModule {
/// @notice Initializes the registration module contract.
/// @param controller The access controller used for IP authorization.
/// @param assetRegistry The address of the IP asset registry.
/// @param licenseRegistry The address of the license module.
constructor(
address controller,
address assetRegistry,
address licenseRegistry,
address licensingModule,
address resolverAddr
) BaseModule(controller, assetRegistry, licenseRegistry) {
) BaseModule(controller, assetRegistry) {
resolver = IPResolver(resolverAddr);
_LICENSING_MODULE = ILicensingModule(licensingModule);
}
Expand Down
7 changes: 2 additions & 5 deletions contracts/modules/dispute-module/DisputeModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,12 @@ contract DisputeModule is IDisputeModule, BaseModule, Governable, ReentrancyGuar
/// @notice Initializes the registration module contract
/// @param _controller The access controller used for IP authorization
/// @param _assetRegistry The address of the IP asset registry
/// @param _licenseRegistry The address of the license registry
/// @param _governance The address of the governance contract
constructor(
address _controller,
address _assetRegistry,
address _licenseRegistry,
address _governance
) BaseModule(_controller, _assetRegistry, _licenseRegistry) Governable(_governance) {}
) BaseModule(_controller, _assetRegistry) Governable(_governance) {}

/// @notice Whitelists a dispute tag
/// @param _tag The dispute tag
Expand Down Expand Up @@ -114,8 +112,7 @@ contract DisputeModule is IDisputeModule, BaseModule, Governable, ReentrancyGuar
/// @notice Sets the arbitration policy for an ipId
/// @param _ipId The ipId
/// @param _arbitrationPolicy The address of the arbitration policy
function setArbitrationPolicy(address _ipId, address _arbitrationPolicy) external {
if (_ipId != msg.sender) _authenticate(_ipId);
function setArbitrationPolicy(address _ipId, address _arbitrationPolicy) external verifyPermission(_ipId) {
if (!isWhitelistedArbitrationPolicy[_arbitrationPolicy]) revert Errors.DisputeModule__NotWhitelistedArbitrationPolicy();

arbitrationPolicies[_ipId] = _arbitrationPolicy;
Expand Down
10 changes: 2 additions & 8 deletions contracts/resolvers/IPResolver.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
pragma solidity ^0.8.23;

import { ResolverBase } from "./ResolverBase.sol";
import { BaseModule } from "../modules/BaseModule.sol";
import { IModule } from "../interfaces/modules/base/IModule.sol";
import { KeyValueResolver } from "../resolvers/KeyValueResolver.sol";
import { IP_RESOLVER_MODULE_KEY } from "../lib/modules/Module.sol";
Expand All @@ -17,12 +16,7 @@ contract IPResolver is KeyValueResolver {
/// @notice Initializes the IP metadata resolver.
/// @param accessController The access controller used for IP authorization.
/// @param ipAssetRegistry The address of the IP record registry.
/// @param licenseRegistry The address of the license registry.
constructor(
address accessController,
address ipAssetRegistry,
address licenseRegistry
) ResolverBase(accessController, ipAssetRegistry, licenseRegistry) {}
constructor(address accessController, address ipAssetRegistry) ResolverBase(accessController, ipAssetRegistry) {}

/// @notice Checks whether the resolver interface is supported.
/// @param id The resolver interface identifier.
Expand All @@ -32,7 +26,7 @@ contract IPResolver is KeyValueResolver {
}

/// @notice Gets the protocol-wide module identifier for this module.
function name() public pure override(BaseModule, IModule) returns (string memory) {
function name() public pure override(IModule) returns (string memory) {
return IP_RESOLVER_MODULE_KEY;
}
}
2 changes: 1 addition & 1 deletion contracts/resolvers/KeyValueResolver.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ abstract contract KeyValueResolver is IKeyValueResolver, ResolverBase {
/// @param ipId The canonical identifier of the IP asset.
/// @param k The string parameter key to update.
/// @param v The value to set for the specified key.
function setValue(address ipId, string calldata k, string calldata v) external virtual onlyAuthorized(ipId) {
function setValue(address ipId, string calldata k, string calldata v) external virtual verifyPermission(ipId) {
_values[ipId][k] = v;
emit KeyValueSet(ipId, k, v);
}
Expand Down
7 changes: 1 addition & 6 deletions contracts/resolvers/ResolverBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,7 @@ abstract contract ResolverBase is IResolver, BaseModule {
/// @notice Initializes the base module contract.
/// @param controller The access controller used for IP authorization.
/// @param assetRegistry The address of the IP record registry.
/// @param licenseRegistry The address of the license registry.
constructor(
address controller,
address assetRegistry,
address licenseRegistry
) BaseModule(controller, assetRegistry, licenseRegistry) {}
constructor(address controller, address assetRegistry) BaseModule(controller, assetRegistry) {}

/// @notice Checks whether the resolver interface is supported.
/// @param id The resolver interface identifier.
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 @@ -204,15 +204,14 @@ contract Main is Script, BroadcastManager, JsonDeploymentHandler {

contractKey = "IPResolver";
_predeploy(contractKey);
ipResolver = new IPResolver(address(accessController), address(ipAssetRegistry), address(licenseRegistry));
ipResolver = new IPResolver(address(accessController), address(ipAssetRegistry));
_postdeploy(contractKey, address(ipResolver));

contractKey = "RegistrationModule";
_predeploy(contractKey);
registrationModule = new RegistrationModule(
address(accessController),
address(ipAssetRegistry),
address(licenseRegistry),
address(licensingModule),
address(ipResolver)
);
Expand All @@ -223,7 +222,6 @@ contract Main is Script, BroadcastManager, JsonDeploymentHandler {
disputeModule = new DisputeModule(
address(accessController),
address(ipAssetRegistry),
address(licenseRegistry),
address(governance)
);
_postdeploy(contractKey, address(disputeModule));
Expand Down
4 changes: 1 addition & 3 deletions test/foundry/integration/BaseIntegration.sol
Original file line number Diff line number Diff line change
Expand Up @@ -137,19 +137,17 @@ contract BaseIntegration is Test {
);
licenseRegistry.setLicensingModule(address(licensingModule));
ipMetadataProvider = new IPMetadataProvider(address(moduleRegistry));
ipResolver = new IPResolver(address(accessController), address(ipAssetRegistry), address(licenseRegistry));
ipResolver = new IPResolver(address(accessController), address(ipAssetRegistry));
registrationModule = new RegistrationModule(
address(accessController),
address(ipAssetRegistry),
address(licenseRegistry),
address(licensingModule),
address(ipResolver)
);
taggingModule = new TaggingModule();
disputeModule = new DisputeModule(
address(accessController),
address(ipAssetRegistry),
address(licenseRegistry),
address(governance)
);
ipAssetRenderer = new IPAssetRenderer(
Expand Down
4 changes: 1 addition & 3 deletions test/foundry/modules/ModuleBase.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,11 @@ abstract contract ModuleBaseTest is BaseTest {
licenseRegistry.setLicensingModule(address(licensingModule));
IPResolver ipResolver = new IPResolver(
address(accessController),
address(ipAssetRegistry),
address(licenseRegistry)
address(ipAssetRegistry)
);
RegistrationModule registrationModule = new RegistrationModule(
address(accessController),
address(ipAssetRegistry),
address(licenseRegistry),
address(licensingModule),
address(ipResolver)
);
Expand Down
10 changes: 9 additions & 1 deletion test/foundry/modules/dispute/DisputeModule.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,15 @@ contract TestDisputeModule is TestHelper {
}

function test_DisputeModule_setArbitrationPolicy_revert_UnauthorizedAccess() public {
vm.expectRevert(Errors.Module_Unauthorized.selector);
vm.expectRevert(
abi.encodeWithSelector(
Errors.AccessController__PermissionDenied.selector,
ipAddr,
address(this),
address(disputeModule),
disputeModule.setArbitrationPolicy.selector
)
);
disputeModule.setArbitrationPolicy(ipAddr, address(arbitrationPolicySP2));
}

Expand Down
4 changes: 1 addition & 3 deletions test/foundry/registries/metadata/IPAssetRenderer.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ contract IPAssetRendererTest is BaseTest {
licenseRegistry.setLicensingModule(address(licensingModule));
resolver = new IPResolver(
address(accessController),
address(ipAssetRegistry),
address(licenseRegistry)
address(ipAssetRegistry)
);
renderer = new IPAssetRenderer(
address(ipAssetRegistry),
Expand All @@ -131,7 +130,6 @@ contract IPAssetRendererTest is BaseTest {
registrationModule = new RegistrationModule(
address(accessController),
address(ipAssetRegistry),
address(licenseRegistry),
address(licensingModule),
address(resolver)
);
Expand Down
3 changes: 1 addition & 2 deletions test/foundry/registries/metadata/MetadataProvider.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,7 @@ contract MetadataProviderTest is BaseTest {
uint256 tokenId = erc721.mintId(alice, 99);
IPResolver resolver = new IPResolver(
address(accessController),
address(registry),
address(licenseRegistry)
address(registry)
);
ipId = registry.register(
block.chainid,
Expand Down
3 changes: 1 addition & 2 deletions test/foundry/resolvers/IPResolver.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ contract IPResolverTest is ResolverBaseTest {
return address(
new IPResolver(
address(accessController),
address(ipAssetRegistry),
address(licenseRegistry)
address(ipAssetRegistry)
)
);
}
Expand Down
4 changes: 1 addition & 3 deletions test/utils/DeployHelper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -143,19 +143,17 @@ contract DeployHelper is Test {
);
licenseRegistry.setLicensingModule(address(licensingModule));
ipMetadataProvider = new IPMetadataProvider(address(moduleRegistry));
ipResolver = new IPResolver(address(accessController), address(ipAssetRegistry), address(licenseRegistry));
ipResolver = new IPResolver(address(accessController), address(ipAssetRegistry));
registrationModule = new RegistrationModule(
address(accessController),
address(ipAssetRegistry),
address(licenseRegistry),
address(licensingModule),
address(ipResolver)
);
taggingModule = new TaggingModule();
disputeModule = new DisputeModule(
address(accessController),
address(ipAssetRegistry),
address(licenseRegistry),
address(governance)
);
ipAssetRenderer = new IPAssetRenderer(
Expand Down
Loading