Skip to content

Commit

Permalink
refactor(world): make AccessControl lib usable outside of world packa…
Browse files Browse the repository at this point in the history
…ge (#3034)
  • Loading branch information
holic authored Aug 15, 2024
1 parent 91028bf commit 6a66f57
Show file tree
Hide file tree
Showing 18 changed files with 200 additions and 177 deletions.
7 changes: 7 additions & 0 deletions .changeset/quick-fans-jog.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@latticexyz/world-module-metadata": patch
"@latticexyz/world-modules": patch
"@latticexyz/world": patch
---

Refactored `AccessControl` library exported from `@latticexyz/world` to be usable outside of the world package and updated module packages to use it.
79 changes: 79 additions & 0 deletions docs/pages/world/reference/internal/access-control.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,29 @@ function hasAccess(ResourceId resourceId, address caller) internal view returns
| -------- | ------ | ----------------------------------------------- |
| `<none>` | `bool` | true if the caller has access, false otherwise. |

#### \_hasAccess

Checks if the caller has access to the given resource ID or its namespace.

_This bypasses StoreSwitch and assumes its being called within the Store, saving gas for known call contexts._

```solidity
function _hasAccess(ResourceId resourceId, address caller) internal view returns (bool);
```

**Parameters**

| Name | Type | Description |
| ------------ | ------------ | ------------------------------------ |
| `resourceId` | `ResourceId` | The resource ID to check access for. |
| `caller` | `address` | The address of the caller. |

**Returns**

| Name | Type | Description |
| -------- | ------ | ----------------------------------------------- |
| `<none>` | `bool` | true if the caller has access, false otherwise. |

#### requireAccess

Check for access at the given namespace or resource.
Expand All @@ -46,6 +69,25 @@ function requireAccess(ResourceId resourceId, address caller) internal view;
| `resourceId` | `ResourceId` | The resource ID to check access for. |
| `caller` | `address` | The address of the caller. |

#### \_requireAccess

Check for access at the given namespace or resource.

_Reverts with IWorldErrors.World_AccessDenied if access is denied._

_This bypasses StoreSwitch and assumes its being called within the Store, saving gas for known call contexts._

```solidity
function _requireAccess(ResourceId resourceId, address caller) internal view;
```

**Parameters**

| Name | Type | Description |
| ------------ | ------------ | ------------------------------------ |
| `resourceId` | `ResourceId` | The resource ID to check access for. |
| `caller` | `address` | The address of the caller. |

#### requireOwner

Check for ownership of the namespace of the given resource ID.
Expand All @@ -63,6 +105,25 @@ function requireOwner(ResourceId resourceId, address caller) internal view;
| `resourceId` | `ResourceId` | The resource ID to check ownership for. |
| `caller` | `address` | The address of the caller. |

#### \_requireOwner

Check for ownership of the namespace of the given resource ID.

_Reverts with IWorldErrors.World_AccessDenied if caller is not owner of the namespace of the resource._

_This bypasses StoreSwitch and assumes its being called within the Store, saving gas for known call contexts._

```solidity
function _requireOwner(ResourceId resourceId, address caller) internal view;
```

**Parameters**

| Name | Type | Description |
| ------------ | ------------ | --------------------------------------- |
| `resourceId` | `ResourceId` | The resource ID to check ownership for. |
| `caller` | `address` | The address of the caller. |

#### requireExistence

Check for existence of the given resource ID.
Expand All @@ -78,3 +139,21 @@ function requireExistence(ResourceId resourceId) internal view;
| Name | Type | Description |
| ------------ | ------------ | --------------------------------------- |
| `resourceId` | `ResourceId` | The resource ID to check existence for. |

#### \_requireExistence

Check for existence of the given resource ID.

_Reverts with IWorldErrors.World_ResourceNotFound if the resource does not exist._

_This bypasses StoreSwitch and assumes its being called within the Store, saving gas for known call contexts._

```solidity
function _requireExistence(ResourceId resourceId) internal view;
```

**Parameters**

| Name | Type | Description |
| ------------ | ------------ | --------------------------------------- |
| `resourceId` | `ResourceId` | The resource ID to check existence for. |
4 changes: 2 additions & 2 deletions packages/world-module-metadata/src/MetadataModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pragma solidity >=0.8.24;

import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol";
import { Module } from "@latticexyz/world/src/Module.sol";
import { requireOwner } from "./common.sol";
import { AccessControl } from "@latticexyz/world/src/AccessControl.sol";
import { ResourceId, WorldResourceIdLib, WorldResourceIdInstance } from "@latticexyz/world/src/WorldResourceId.sol";
import { ResourceIds } from "@latticexyz/store/src/codegen/tables/ResourceIds.sol";
import { RESOURCE_SYSTEM } from "@latticexyz/world/src/worldResourceTypes.sol";
Expand Down Expand Up @@ -33,7 +33,7 @@ contract MetadataModule is Module {
if (!ResourceIds.getExists(namespace)) {
world.registerNamespace(namespace);
}
requireOwner(namespace, address(this));
AccessControl.requireOwner(namespace, address(this));

if (!ResourceIds.getExists(ResourceTag._tableId)) {
ResourceTag.register();
Expand Down
11 changes: 6 additions & 5 deletions packages/world-module-metadata/src/MetadataSystem.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ pragma solidity >=0.8.24;

import { ResourceId } from "@latticexyz/store/src/ResourceId.sol";
import { System } from "@latticexyz/world/src/System.sol";
import { requireExistence, requireOwner } from "./common.sol";
import { AccessControl } from "@latticexyz/world/src/AccessControl.sol";

import { ResourceTag } from "./codegen/tables/ResourceTag.sol";

contract MetadataSystem is System {
Expand All @@ -12,14 +13,14 @@ contract MetadataSystem is System {
}

function setResourceTag(ResourceId resource, bytes32 tag, bytes memory value) public {
requireExistence(resource);
requireOwner(resource, _msgSender());
AccessControl.requireExistence(resource);
AccessControl.requireOwner(resource, _msgSender());
ResourceTag.set(resource, tag, value);
}

function deleteResourceTag(ResourceId resource, bytes32 tag) public {
requireExistence(resource);
requireOwner(resource, _msgSender());
AccessControl.requireExistence(resource);
AccessControl.requireOwner(resource, _msgSender());
ResourceTag.deleteRecord(resource, tag);
}
}
60 changes: 0 additions & 60 deletions packages/world-module-metadata/src/common.sol

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import { System } from "@latticexyz/world/src/System.sol";
import { WorldResourceIdInstance } from "@latticexyz/world/src/WorldResourceId.sol";
import { NamespaceOwner } from "@latticexyz/world/src/codegen/tables/NamespaceOwner.sol";
import { SystemRegistry } from "@latticexyz/world/src/codegen/tables/SystemRegistry.sol";
import { AccessControl } from "@latticexyz/world/src/AccessControl.sol";

import { AccessControlLib } from "../../utils/AccessControlLib.sol";
import { PuppetMaster } from "../puppet/PuppetMaster.sol";
import { toTopic } from "../puppet/utils.sol";
import { Balances } from "../tokens/tables/Balances.sol";
Expand Down Expand Up @@ -281,6 +281,6 @@ contract ERC20System is System, IERC20Mintable, PuppetMaster {
}

function _requireOwner() internal view {
AccessControlLib.requireOwner(SystemRegistry.get(address(this)), _msgSender());
AccessControl.requireOwner(SystemRegistry.get(address(this)), _msgSender());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import { ResourceId } from "@latticexyz/store/src/ResourceId.sol";
import { System } from "@latticexyz/world/src/System.sol";
import { WorldResourceIdInstance } from "@latticexyz/world/src/WorldResourceId.sol";
import { SystemRegistry } from "@latticexyz/world/src/codegen/tables/SystemRegistry.sol";
import { AccessControl } from "@latticexyz/world/src/AccessControl.sol";

import { AccessControlLib } from "../../utils/AccessControlLib.sol";
import { PuppetMaster } from "../puppet/PuppetMaster.sol";
import { toTopic } from "../puppet/utils.sol";
import { Balances } from "../tokens/tables/Balances.sol";
Expand Down Expand Up @@ -526,6 +526,6 @@ contract ERC721System is IERC721Mintable, System, PuppetMaster {
}

function _requireOwner() internal view {
AccessControlLib.requireOwner(SystemRegistry.get(address(this)), _msgSender());
AccessControl.requireOwner(SystemRegistry.get(address(this)), _msgSender());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ pragma solidity >=0.8.24;
import { ResourceId } from "@latticexyz/store/src/ResourceId.sol";
import { System } from "@latticexyz/world/src/System.sol";
import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol";

import { AccessControlLib } from "../../utils/AccessControlLib.sol";
import { AccessControl } from "@latticexyz/world/src/AccessControl.sol";

import { PuppetRegistry } from "./tables/PuppetRegistry.sol";
import { Puppet } from "./Puppet.sol";
Expand All @@ -14,7 +13,7 @@ import { PUPPET_TABLE_ID } from "./constants.sol";
contract PuppetFactorySystem is System {
function createPuppet(ResourceId systemId) public returns (address puppet) {
// Only the owner of a system can create a puppet for it
AccessControlLib.requireOwner(systemId, _msgSender());
AccessControl.requireOwner(systemId, _msgSender());

// Deploy a new puppet contract
puppet = address(new Puppet(IBaseWorld(_world()), systemId));
Expand Down
55 changes: 0 additions & 55 deletions packages/world-modules/src/utils/AccessControlLib.sol

This file was deleted.

14 changes: 7 additions & 7 deletions packages/world/gas-report.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,43 +3,43 @@
"file": "test/AccessControl.t.sol",
"test": "testAccessControl",
"name": "AccessControl: hasAccess (cold)",
"gasUsed": 6521
"gasUsed": 9111
},
{
"file": "test/AccessControl.t.sol",
"test": "testAccessControl",
"name": "AccessControl: hasAccess (warm, namespace only)",
"gasUsed": 1300
"gasUsed": 1595
},
{
"file": "test/AccessControl.t.sol",
"test": "testAccessControl",
"name": "AccessControl: hasAccess (warm)",
"gasUsed": 2527
"gasUsed": 3117
},
{
"file": "test/AccessControl.t.sol",
"test": "testRequireAccess",
"name": "AccessControl: requireAccess (cold)",
"gasUsed": 6564
"gasUsed": 9154
},
{
"file": "test/AccessControl.t.sol",
"test": "testRequireAccess",
"name": "AccessControl: requireAccess (warm)",
"gasUsed": 2566
"gasUsed": 3156
},
{
"file": "test/AccessControl.t.sol",
"test": "testRequireOwner",
"name": "AccessControl: requireOwner (cold)",
"gasUsed": 3106
"gasUsed": 5401
},
{
"file": "test/AccessControl.t.sol",
"test": "testRequireOwner",
"name": "AccessControl: requireOwner (warm)",
"gasUsed": 1107
"gasUsed": 1402
},
{
"file": "test/BatchCall.t.sol",
Expand Down
Loading

0 comments on commit 6a66f57

Please sign in to comment.