Skip to content

Commit

Permalink
refactor(world): use IErrors instead of Error library
Browse files Browse the repository at this point in the history
  • Loading branch information
alvrs committed Mar 14, 2023
1 parent 31e1d7f commit 4232dce
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 27 deletions.
6 changes: 3 additions & 3 deletions packages/world/src/AccessControl.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity >=0.8.0;

import { ResourceSelector } from "./ResourceSelector.sol";
import { Errors } from "./Errors.sol";
import { IErrors } from "./interfaces/IErrors.sol";

import { ResourceAccess } from "./tables/ResourceAccess.sol";
import { NamespaceOwner } from "./tables/NamespaceOwner.sol";
Expand Down Expand Up @@ -33,7 +33,7 @@ library AccessControl {

// Check if the given caller has access to the given namespace or file
if (!hasAccess(namespace, file, msg.sender)) {
revert Errors.AccessDenied(resourceSelector.toString(), caller);
revert IErrors.AccessDenied(resourceSelector.toString(), caller);
}
}

Expand All @@ -45,7 +45,7 @@ library AccessControl {
resourceSelector = ResourceSelector.from(namespace, file);

if (NamespaceOwner.get(namespace) != msg.sender) {
revert Errors.AccessDenied(resourceSelector.toString(), caller);
revert IErrors.AccessDenied(resourceSelector.toString(), caller);
}
}
}
8 changes: 4 additions & 4 deletions packages/world/src/World.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { System } from "./System.sol";
import { ResourceSelector } from "./ResourceSelector.sol";
import { Resource } from "./types.sol";
import { ROOT_NAMESPACE, ROOT_FILE } from "./constants.sol";
import { Errors } from "./Errors.sol";
import { AccessControl } from "./AccessControl.sol";

import { NamespaceOwner } from "./tables/NamespaceOwner.sol";
Expand All @@ -24,8 +23,9 @@ import { RegistrationSystem } from "./systems/RegistrationSystem.sol";

import { IWorldCore } from "./interfaces/IWorldCore.sol";
import { IWorld } from "./interfaces/IWorld.sol";
import { IErrors } from "./interfaces/IErrors.sol";

contract World is Store, IWorldCore {
contract World is Store, IWorldCore, IErrors {
using ResourceSelector for bytes32;

// IWorld includes interfaces for dynamically registered systems (e.g. IRegistrationSystem)
Expand Down Expand Up @@ -318,7 +318,7 @@ contract World is Store, IWorldCore {
(address systemAddress, bool publicAccess) = Systems.get(resourceSelector);

// Check if the system exists
if (systemAddress == address(0)) revert Errors.ResourceNotFound(resourceSelector.toString());
if (systemAddress == address(0)) revert ResourceNotFound(resourceSelector.toString());

// Allow access if the system is public or the caller has access to the namespace or file
if (!publicAccess) AccessControl.requireAccess(namespace, file, msg.sender);
Expand All @@ -345,7 +345,7 @@ contract World is Store, IWorldCore {
fallback() external {
(bytes16 namespace, bytes16 file, bytes4 systemFunctionSelector) = FunctionSelectors.get(msg.sig);

if (namespace == 0 && file == 0) revert Errors.FunctionSelectorNotFound(msg.sig);
if (namespace == 0 && file == 0) revert FunctionSelectorNotFound(msg.sig);

// Replace function selector in the calldata with the system function selector
bytes memory callData = Bytes.setBytes4(msg.data, 0, systemFunctionSelector);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: MIT
pragma solidity >=0.8.0;

library Errors {
interface IErrors {
error ResourceExists(string resource);
error ResourceNotFound(string resource);
error AccessDenied(string resource, address caller);
Expand Down
22 changes: 11 additions & 11 deletions packages/world/src/systems/RegistrationSystem.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { System } from "../System.sol";
import { ResourceSelector } from "../ResourceSelector.sol";
import { Resource } from "../types.sol";
import { ROOT_NAMESPACE, ROOT_FILE } from "../constants.sol";
import { Errors } from "../Errors.sol";
import { AccessControl } from "../AccessControl.sol";

import { NamespaceOwner } from "../tables/NamespaceOwner.sol";
Expand All @@ -21,8 +20,9 @@ import { FunctionSelectors } from "../tables/FunctionSelectors.sol";

import { ISystemHook } from "../interfaces/ISystemHook.sol";
import { IRegistrationSystem } from "../interfaces/systems/IRegistrationSystem.sol";
import { IErrors } from "../interfaces/IErrors.sol";

contract RegistrationSystem is System, IRegistrationSystem {
contract RegistrationSystem is System, IRegistrationSystem, IErrors {
using ResourceSelector for bytes32;

/**
Expand All @@ -32,7 +32,7 @@ contract RegistrationSystem is System, IRegistrationSystem {
bytes32 resourceSelector = ResourceSelector.from(namespace);

// Require namespace to not exist yet
if (ResourceType.get(namespace) != Resource.NONE) revert Errors.ResourceExists(resourceSelector.toString());
if (ResourceType.get(namespace) != Resource.NONE) revert ResourceExists(resourceSelector.toString());

// Register namespace resource
ResourceType.set(namespace, Resource.NAMESPACE);
Expand All @@ -56,7 +56,7 @@ contract RegistrationSystem is System, IRegistrationSystem {
resourceSelector = ResourceSelector.from(namespace, file);

// Require the file selector to not be the namespace's root file
if (file == ROOT_FILE) revert Errors.InvalidSelector(resourceSelector.toString());
if (file == ROOT_FILE) revert InvalidSelector(resourceSelector.toString());

// If the namespace doesn't exist yet, register it
// otherwise require caller to own the namespace
Expand All @@ -65,7 +65,7 @@ contract RegistrationSystem is System, IRegistrationSystem {

// Require no resource to exist at this selector yet
if (ResourceType.get(resourceSelector) != Resource.NONE) {
revert Errors.ResourceExists(resourceSelector.toString());
revert ResourceExists(resourceSelector.toString());
}

// Store the table resource type
Expand Down Expand Up @@ -108,7 +108,7 @@ contract RegistrationSystem is System, IRegistrationSystem {
return registerSystemHook(namespace, file, ISystemHook(hook));
}

revert Errors.InvalidSelector(ResourceSelector.from(namespace, file).toString());
revert InvalidSelector(ResourceSelector.from(namespace, file).toString());
}

/**
Expand Down Expand Up @@ -145,10 +145,10 @@ contract RegistrationSystem is System, IRegistrationSystem {
resourceSelector = ResourceSelector.from(namespace, file);

// Require the file selector to not be the namespace's root file
if (file == ROOT_FILE) revert Errors.InvalidSelector(resourceSelector.toString());
if (file == ROOT_FILE) revert InvalidSelector(resourceSelector.toString());

// Require the system to not exist yet
if (SystemRegistry.get(address(system)) != 0) revert Errors.SystemExists(address(system));
if (SystemRegistry.get(address(system)) != 0) revert SystemExists(address(system));

// If the namespace doesn't exist yet, register it
// otherwise require caller to own the namespace
Expand All @@ -157,7 +157,7 @@ contract RegistrationSystem is System, IRegistrationSystem {

// Require no resource to exist at this selector yet
if (ResourceType.get(resourceSelector) != Resource.NONE) {
revert Errors.ResourceExists(resourceSelector.toString());
revert ResourceExists(resourceSelector.toString());
}

// Store the system resource type
Expand Down Expand Up @@ -199,7 +199,7 @@ contract RegistrationSystem is System, IRegistrationSystem {
bytes16 existingNamespace = FunctionSelectors.getNamespace(worldFunctionSelector);
bytes16 existingFile = FunctionSelectors.getFile(worldFunctionSelector);

if (existingNamespace != 0 || existingFile != 0) revert Errors.FunctionSelectorExists(worldFunctionSelector);
if (existingNamespace != 0 || existingFile != 0) revert FunctionSelectorExists(worldFunctionSelector);

// Register the function selector
bytes memory systemFunctionSignature = abi.encodePacked(systemFunctionName, systemFunctionArguments);
Expand Down Expand Up @@ -229,7 +229,7 @@ contract RegistrationSystem is System, IRegistrationSystem {
bytes16 existingNamespace = FunctionSelectors.getNamespace(worldFunctionSelector);
bytes16 existingFile = FunctionSelectors.getFile(worldFunctionSelector);

if (!(existingNamespace == 0 && existingFile == 0)) revert Errors.FunctionSelectorExists(worldFunctionSelector);
if (!(existingNamespace == 0 && existingFile == 0)) revert FunctionSelectorExists(worldFunctionSelector);

// Register the function selector
FunctionSelectors.set(worldFunctionSelector, namespace, file, systemFunctionSelector);
Expand Down
18 changes: 10 additions & 8 deletions packages/world/test/World.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { EncodeArray } from "@latticexyz/store/src/tightcoder/EncodeArray.sol";
import { World } from "../src/World.sol";
import { System } from "../src/System.sol";
import { ResourceSelector } from "../src/ResourceSelector.sol";
import { Errors } from "../src/Errors.sol";
import { ROOT_NAMESPACE, ROOT_FILE } from "../src/constants.sol";

import { NamespaceOwner } from "../src/tables/NamespaceOwner.sol";
Expand All @@ -24,6 +23,7 @@ import { Bool } from "../src/tables/Bool.sol";
import { AddressArray } from "../src/tables/AddressArray.sol";

import { IWorld } from "../src/interfaces/IWorld.sol";
import { IErrors } from "../src/interfaces/IErrors.sol";

struct WorldTestSystemReturn {
address sender;
Expand Down Expand Up @@ -125,7 +125,7 @@ contract WorldTest is Test {
function _expectAccessDenied(address caller, bytes16 namespace, bytes16 file) internal {
vm.prank(caller);
vm.expectRevert(
abi.encodeWithSelector(Errors.AccessDenied.selector, ResourceSelector.from(namespace, file).toString(), caller)
abi.encodeWithSelector(IErrors.AccessDenied.selector, ResourceSelector.from(namespace, file).toString(), caller)
);
}

Expand Down Expand Up @@ -153,7 +153,9 @@ contract WorldTest is Test {
assertEq(ResourceAccess.get(world, "test", address(this)), true, "caller should have access");

// Expect an error when registering an existing namespace
vm.expectRevert(abi.encodeWithSelector(Errors.ResourceExists.selector, ResourceSelector.toString(bytes16("test"))));
vm.expectRevert(
abi.encodeWithSelector(IErrors.ResourceExists.selector, ResourceSelector.toString(bytes16("test")))
);
world.registerNamespace("test");
}

Expand All @@ -173,7 +175,7 @@ contract WorldTest is Test {
assertEq(world.getSchema(uint256(tableSelector)).unwrap(), schema.unwrap(), "schema should be registered");

// Expect an error when registering an existing table
vm.expectRevert(abi.encodeWithSelector(Errors.ResourceExists.selector, tableSelector.toString()));
vm.expectRevert(abi.encodeWithSelector(IErrors.ResourceExists.selector, tableSelector.toString()));
world.registerTable(namespace, table, schema, defaultKeySchema);

// Expect an error when registering a table in a namespace that is not owned by the caller
Expand Down Expand Up @@ -253,14 +255,14 @@ contract WorldTest is Test {
assertEq(NamespaceOwner.get(world, "newNamespace"), address(this));

// Expect an error when registering an existing system
vm.expectRevert(abi.encodeWithSelector(Errors.SystemExists.selector, address(system)));
vm.expectRevert(abi.encodeWithSelector(IErrors.SystemExists.selector, address(system)));
world.registerSystem("", "newSystem", system, true);

// Expect an error when registering a system at an existing resource selector
System newSystem = new System();

// Expect an error when registering a system at an existing resource selector
vm.expectRevert(abi.encodeWithSelector(Errors.ResourceExists.selector, resourceSelector.toString()));
vm.expectRevert(abi.encodeWithSelector(IErrors.ResourceExists.selector, resourceSelector.toString()));
resourceSelector = world.registerSystem("", "testSystem", newSystem, true);

// Expect an error when registering a system in a namespace is not owned by the caller
Expand All @@ -277,14 +279,14 @@ contract WorldTest is Test {
System system = new System();

// Expect an error when trying to register a system at the same selector
vm.expectRevert(abi.encodeWithSelector(Errors.ResourceExists.selector, resourceSelector.toString()));
vm.expectRevert(abi.encodeWithSelector(IErrors.ResourceExists.selector, resourceSelector.toString()));
world.registerSystem("namespace", "file", system, false);

// Register a new system
resourceSelector = world.registerSystem("namespace2", "file", new System(), false);

// Expect an error when trying to register a table at the same selector
vm.expectRevert(abi.encodeWithSelector(Errors.ResourceExists.selector, resourceSelector.toString()));
vm.expectRevert(abi.encodeWithSelector(IErrors.ResourceExists.selector, resourceSelector.toString()));
world.registerTable("namespace2", "file", Bool.getSchema(), defaultKeySchema);
}

Expand Down

0 comments on commit 4232dce

Please sign in to comment.