Skip to content

Commit

Permalink
refactor(store,world): prefix errors with library/contract name (#1568)
Browse files Browse the repository at this point in the history
  • Loading branch information
alvrs authored Sep 22, 2023
1 parent 748f458 commit d087892
Show file tree
Hide file tree
Showing 23 changed files with 152 additions and 128 deletions.
6 changes: 6 additions & 0 deletions .changeset/large-drinks-sell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@latticexyz/store": patch
"@latticexyz/world": patch
---

Prefixed all errors with their respective library/contract for improved debugging.
23 changes: 11 additions & 12 deletions packages/store/src/IStoreErrors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,16 @@ import { ResourceId } from "./ResourceId.sol";

interface IStoreErrors {
// Errors include a stringified version of the tableId for easier debugging if cleartext tableIds are used
error StoreCore_TableAlreadyExists(ResourceId tableId, string tableIdString);
error StoreCore_TableNotFound(ResourceId tableId, string tableIdString);
error StoreCore_InvalidResourceType(bytes2 expected, ResourceId resourceId, string resourceIdString);
error Store_TableAlreadyExists(ResourceId tableId, string tableIdString);
error Store_TableNotFound(ResourceId tableId, string tableIdString);
error Store_InvalidResourceType(bytes2 expected, ResourceId resourceId, string resourceIdString);

error StoreCore_NotImplemented();
error StoreCore_NotDynamicField();
error StoreCore_InvalidStaticDataLength(uint256 expected, uint256 received);
error StoreCore_InvalidDynamicDataLength(uint256 expected, uint256 received);
error StoreCore_InvalidKeyNamesLength(uint256 expected, uint256 received);
error StoreCore_InvalidFieldNamesLength(uint256 expected, uint256 received);
error StoreCore_InvalidValueSchemaLength(uint256 expected, uint256 received);
error StoreCore_DataIndexOverflow(uint256 length, uint256 received);
error StoreCore_InvalidSplice(uint40 startWithinField, uint40 deleteCount, uint40 fieldLength);
error Store_NotDynamicField();
error Store_InvalidStaticDataLength(uint256 expected, uint256 received);
error Store_InvalidDynamicDataLength(uint256 expected, uint256 received);
error Store_InvalidKeyNamesLength(uint256 expected, uint256 received);
error Store_InvalidFieldNamesLength(uint256 expected, uint256 received);
error Store_InvalidValueSchemaLength(uint256 expected, uint256 received);
error Store_DataIndexOverflow(uint256 length, uint256 received);
error Store_InvalidSplice(uint40 startWithinField, uint40 deleteCount, uint40 fieldLength);
}
36 changes: 18 additions & 18 deletions packages/store/src/StoreCore.sol
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ library StoreCore {
function getFieldLayout(ResourceId tableId) internal view returns (FieldLayout fieldLayout) {
fieldLayout = FieldLayout.wrap(Tables._getFieldLayout(ResourceId.unwrap(tableId)));
if (fieldLayout.isEmpty()) {
revert IStoreErrors.StoreCore_TableNotFound(tableId, string(abi.encodePacked(tableId)));
revert IStoreErrors.Store_TableNotFound(tableId, string(abi.encodePacked(tableId)));
}
}

Expand All @@ -99,7 +99,7 @@ library StoreCore {
keySchema = Schema.wrap(Tables._getKeySchema(ResourceId.unwrap(tableId)));
// key schemas can be empty for singleton tables, so we can't depend on key schema for table check
if (!ResourceIds._getExists(ResourceId.unwrap(tableId))) {
revert IStoreErrors.StoreCore_TableNotFound(tableId, string(abi.encodePacked(tableId)));
revert IStoreErrors.Store_TableNotFound(tableId, string(abi.encodePacked(tableId)));
}
}

Expand All @@ -109,7 +109,7 @@ library StoreCore {
function getValueSchema(ResourceId tableId) internal view returns (Schema valueSchema) {
valueSchema = Schema.wrap(Tables._getValueSchema(ResourceId.unwrap(tableId)));
if (valueSchema.isEmpty()) {
revert IStoreErrors.StoreCore_TableNotFound(tableId, string(abi.encodePacked(tableId)));
revert IStoreErrors.Store_TableNotFound(tableId, string(abi.encodePacked(tableId)));
}
}

Expand All @@ -126,7 +126,7 @@ library StoreCore {
) internal {
// Verify the table ID is of type RESOURCE_TABLE
if (tableId.getType() != RESOURCE_TABLE && tableId.getType() != RESOURCE_OFFCHAIN_TABLE) {
revert IStoreErrors.StoreCore_InvalidResourceType(RESOURCE_TABLE, tableId, string(abi.encodePacked(tableId)));
revert IStoreErrors.Store_InvalidResourceType(RESOURCE_TABLE, tableId, string(abi.encodePacked(tableId)));
}

// Verify the field layout is valid
Expand All @@ -138,22 +138,22 @@ library StoreCore {

// Verify the number of key names matches the number of key schema types
if (keyNames.length != keySchema.numFields()) {
revert IStoreErrors.StoreCore_InvalidKeyNamesLength(keySchema.numFields(), keyNames.length);
revert IStoreErrors.Store_InvalidKeyNamesLength(keySchema.numFields(), keyNames.length);
}

// Verify the number of value names
if (fieldNames.length != fieldLayout.numFields()) {
revert IStoreErrors.StoreCore_InvalidFieldNamesLength(fieldLayout.numFields(), fieldNames.length);
revert IStoreErrors.Store_InvalidFieldNamesLength(fieldLayout.numFields(), fieldNames.length);
}

// Verify the number of value schema types
if (valueSchema.numFields() != fieldLayout.numFields()) {
revert IStoreErrors.StoreCore_InvalidValueSchemaLength(fieldLayout.numFields(), valueSchema.numFields());
revert IStoreErrors.Store_InvalidValueSchemaLength(fieldLayout.numFields(), valueSchema.numFields());
}

// Verify there is no resource with this ID yet
if (ResourceIds._getExists(ResourceId.unwrap(tableId))) {
revert IStoreErrors.StoreCore_TableAlreadyExists(tableId, string(abi.encodePacked(tableId)));
revert IStoreErrors.Store_TableAlreadyExists(tableId, string(abi.encodePacked(tableId)));
}

// Register the table metadata
Expand Down Expand Up @@ -182,7 +182,7 @@ library StoreCore {
function registerStoreHook(ResourceId tableId, IStoreHook hookAddress, uint8 enabledHooksBitmap) internal {
// Hooks are only supported for tables, not for offchain tables
if (tableId.getType() != RESOURCE_TABLE) {
revert IStoreErrors.StoreCore_InvalidResourceType(RESOURCE_TABLE, tableId, string(abi.encodePacked(tableId)));
revert IStoreErrors.Store_InvalidResourceType(RESOURCE_TABLE, tableId, string(abi.encodePacked(tableId)));
}

StoreHooks.push(ResourceId.unwrap(tableId), Hook.unwrap(HookLib.encode(address(hookAddress), enabledHooksBitmap)));
Expand Down Expand Up @@ -476,7 +476,7 @@ library StoreCore {
FieldLayout fieldLayout
) internal {
if (fieldIndex < fieldLayout.numStaticFields()) {
revert IStoreErrors.StoreCore_NotDynamicField();
revert IStoreErrors.Store_NotDynamicField();
}

StoreCoreInternal._pushToDynamicField(tableId, keyTuple, fieldLayout, fieldIndex, dataToPush);
Expand All @@ -493,7 +493,7 @@ library StoreCore {
FieldLayout fieldLayout
) internal {
if (fieldIndex < fieldLayout.numStaticFields()) {
revert IStoreErrors.StoreCore_NotDynamicField();
revert IStoreErrors.Store_NotDynamicField();
}

StoreCoreInternal._popFromDynamicField(tableId, keyTuple, fieldLayout, fieldIndex, byteLengthToPop);
Expand All @@ -511,13 +511,13 @@ library StoreCore {
FieldLayout fieldLayout
) internal {
if (fieldIndex < fieldLayout.numStaticFields()) {
revert IStoreErrors.StoreCore_NotDynamicField();
revert IStoreErrors.Store_NotDynamicField();
}

// index must be checked because it could be arbitrarily large
// (but dataToSet.length can be unchecked - it won't overflow into another slot due to gas costs and hashed slots)
if (startByteIndex > type(uint40).max) {
revert IStoreErrors.StoreCore_DataIndexOverflow(type(uint40).max, startByteIndex);
revert IStoreErrors.Store_DataIndexOverflow(type(uint40).max, startByteIndex);
}

StoreCoreInternal._setDynamicFieldItem(tableId, keyTuple, fieldLayout, fieldIndex, startByteIndex, dataToSet);
Expand Down Expand Up @@ -651,7 +651,7 @@ library StoreCore {
) internal view returns (bytes memory) {
uint8 numStaticFields = uint8(fieldLayout.numStaticFields());
if (fieldIndex < fieldLayout.numStaticFields()) {
revert IStoreErrors.StoreCore_NotDynamicField();
revert IStoreErrors.Store_NotDynamicField();
}

// Get the length and storage location of the dynamic field
Expand Down Expand Up @@ -687,7 +687,7 @@ library StoreCoreInternal {
// Splicing dynamic data is not supported for offchain tables, because it
// requires reading the previous encoded lengths from storage
if (tableId.getType() != RESOURCE_TABLE) {
revert IStoreErrors.StoreCore_InvalidResourceType(RESOURCE_TABLE, tableId, string(abi.encodePacked(tableId)));
revert IStoreErrors.Store_InvalidResourceType(RESOURCE_TABLE, tableId, string(abi.encodePacked(tableId)));
}

uint256 previousFieldLength = previousEncodedLengths.atIndex(dynamicFieldIndex);
Expand All @@ -696,7 +696,7 @@ library StoreCoreInternal {
// If the total length of the field is changed, the data has to be appended/removed at the end of the field.
// Otherwise offchain indexers would shift the data after inserted data, while onchain the data is truncated at the end.
if (previousFieldLength != updatedFieldLength && startWithinField + deleteCount != previousFieldLength) {
revert IStoreErrors.StoreCore_InvalidSplice(startWithinField, deleteCount, uint40(previousFieldLength));
revert IStoreErrors.Store_InvalidSplice(startWithinField, deleteCount, uint40(previousFieldLength));
}

// Compute start index for the splice
Expand Down Expand Up @@ -894,10 +894,10 @@ library StoreCoreInternal {
bytes memory dynamicData
) internal pure {
if (fieldLayout.staticDataLength() != staticData.length) {
revert IStoreErrors.StoreCore_InvalidStaticDataLength(fieldLayout.staticDataLength(), staticData.length);
revert IStoreErrors.Store_InvalidStaticDataLength(fieldLayout.staticDataLength(), staticData.length);
}
if (encodedLengths.total() != dynamicData.length) {
revert IStoreErrors.StoreCore_InvalidDynamicDataLength(encodedLengths.total(), dynamicData.length);
revert IStoreErrors.Store_InvalidDynamicDataLength(encodedLengths.total(), dynamicData.length);
}
}

Expand Down
26 changes: 9 additions & 17 deletions packages/store/test/StoreCore.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ contract StoreCoreTest is Test, StoreMock {
// Expect a revert when registering a table that already exists
vm.expectRevert(
abi.encodeWithSelector(
IStoreErrors.StoreCore_TableAlreadyExists.selector,
IStoreErrors.Store_TableAlreadyExists.selector,
ResourceId.unwrap(tableId),
string(bytes.concat(ResourceId.unwrap(tableId)))
)
Expand Down Expand Up @@ -142,7 +142,7 @@ contract StoreCoreTest is Test, StoreMock {

vm.expectRevert(
abi.encodeWithSelector(
IStoreErrors.StoreCore_InvalidResourceType.selector,
IStoreErrors.Store_InvalidResourceType.selector,
RESOURCE_TABLE,
invalidTableId,
string(abi.encodePacked(invalidTableId))
Expand Down Expand Up @@ -182,28 +182,20 @@ contract StoreCoreTest is Test, StoreMock {

vm.expectRevert(
abi.encodeWithSelector(
IStoreErrors.StoreCore_TableNotFound.selector,
IStoreErrors.Store_TableNotFound.selector,
tableId2,
string(abi.encodePacked(ResourceId.unwrap(tableId2)))
)
);
IStore(this).getFieldLayout(tableId2);

vm.expectRevert(
abi.encodeWithSelector(
IStoreErrors.StoreCore_TableNotFound.selector,
tableId2,
string(abi.encodePacked(tableId2))
)
abi.encodeWithSelector(IStoreErrors.Store_TableNotFound.selector, tableId2, string(abi.encodePacked(tableId2)))
);
IStore(this).getValueSchema(tableId2);

vm.expectRevert(
abi.encodeWithSelector(
IStoreErrors.StoreCore_TableNotFound.selector,
tableId2,
string(abi.encodePacked(tableId2))
)
abi.encodeWithSelector(IStoreErrors.Store_TableNotFound.selector, tableId2, string(abi.encodePacked(tableId2)))
);
IStore(this).getKeySchema(tableId2);
}
Expand All @@ -223,11 +215,11 @@ contract StoreCoreTest is Test, StoreMock {
string[] memory oneName = new string[](1);

// Register table with invalid key names
vm.expectRevert(abi.encodeWithSelector(IStoreErrors.StoreCore_InvalidKeyNamesLength.selector, 4, 1));
vm.expectRevert(abi.encodeWithSelector(IStoreErrors.Store_InvalidKeyNamesLength.selector, 4, 1));
IStore(this).registerTable(tableId, fieldLayout, keySchema, valueSchema, oneName, oneName);

// Register table with invalid value names
vm.expectRevert(abi.encodeWithSelector(IStoreErrors.StoreCore_InvalidFieldNamesLength.selector, 1, 4));
vm.expectRevert(abi.encodeWithSelector(IStoreErrors.Store_InvalidFieldNamesLength.selector, 1, 4));
IStore(this).registerTable(tableId, fieldLayout, keySchema, valueSchema, fourNames, fourNames);
}

Expand Down Expand Up @@ -333,7 +325,7 @@ contract StoreCoreTest is Test, StoreMock {
keyTuple[0] = "some key";

// This should fail because the data is not 6 bytes long
vm.expectRevert(abi.encodeWithSelector(IStoreErrors.StoreCore_InvalidStaticDataLength.selector, 6, 4));
vm.expectRevert(abi.encodeWithSelector(IStoreErrors.Store_InvalidStaticDataLength.selector, 6, 4));
IStore(this).setRecord(tableId, keyTuple, staticData, PackedCounter.wrap(bytes32(0)), new bytes(0), fieldLayout);
}

Expand Down Expand Up @@ -1051,7 +1043,7 @@ contract StoreCoreTest is Test, StoreMock {

// startByteIndex must not overflow
vm.expectRevert(
abi.encodeWithSelector(IStoreErrors.StoreCore_DataIndexOverflow.selector, type(uint40).max, type(uint56).max)
abi.encodeWithSelector(IStoreErrors.Store_DataIndexOverflow.selector, type(uint40).max, type(uint56).max)
);
IStore(this).updateInField(data.tableId, data.keyTuple, 2, type(uint56).max, data.thirdDataForUpdate, fieldLayout);
}
Expand Down
8 changes: 4 additions & 4 deletions packages/world/src/AccessControl.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,22 @@ library AccessControl {

/**
* Check for access at the given namespace or name.
* Reverts with AccessDenied if the caller has no access.
* Reverts with World_AccessDenied if the caller has no access.
*/
function requireAccess(ResourceId resourceId, address caller) internal view {
// Check if the given caller has access to the given namespace or name
if (!hasAccess(resourceId, caller)) {
revert IWorldErrors.AccessDenied(resourceId.toString(), caller);
revert IWorldErrors.World_AccessDenied(resourceId.toString(), caller);
}
}

/**
* Check for ownership of the namespace of the given resource ID.
* Reverts with AccessDenied if the check fails.
* Reverts with World_AccessDenied if the check fails.
*/
function requireOwner(ResourceId resourceId, address caller) internal view {
if (NamespaceOwner._get(ResourceId.unwrap(resourceId.getNamespaceId())) != caller) {
revert IWorldErrors.AccessDenied(resourceId.toString(), caller);
revert IWorldErrors.World_AccessDenied(resourceId.toString(), caller);
}
}
}
2 changes: 1 addition & 1 deletion packages/world/src/SystemCall.sol
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ library SystemCall {
(address systemAddress, bool publicAccess) = Systems._get(ResourceId.unwrap(systemId));

// Check if the system exists
if (systemAddress == address(0)) revert IWorldErrors.ResourceNotFound(systemId, systemId.toString());
if (systemAddress == address(0)) revert IWorldErrors.World_ResourceNotFound(systemId, systemId.toString());

// Allow access if the system is public or the caller has access to the namespace or name
if (!publicAccess) AccessControl.requireAccess(systemId, caller);
Expand Down
10 changes: 5 additions & 5 deletions packages/world/src/World.sol
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ contract World is StoreRead, IStoreData, IWorldKernel {
*/
modifier requireNoCallback() {
if (msg.sender == address(this)) {
revert WorldCallbackNotAllowed(msg.sig);
revert World_CallbackNotAllowed(msg.sig);
}
_;
}
Expand All @@ -65,12 +65,12 @@ contract World is StoreRead, IStoreData, IWorldKernel {
function initialize(IModule coreModule) public requireNoCallback {
// Only the initial creator of the World can initialize it
if (msg.sender != creator) {
revert AccessDenied(ROOT_NAMESPACE_ID.toString(), msg.sender);
revert World_AccessDenied(ROOT_NAMESPACE_ID.toString(), msg.sender);
}

// The World can only be initialized once
if (InstalledModules._get(CORE_MODULE_NAME, keccak256("")) != address(0)) {
revert WorldAlreadyInitialized();
revert World_AlreadyInitialized();
}

// Initialize the World by installing the core module
Expand Down Expand Up @@ -291,7 +291,7 @@ contract World is StoreRead, IStoreData, IWorldKernel {
return SystemCall.callWithHooksOrRevert(delegator, systemId, callData, msg.value);
}

revert DelegationNotFound(delegator, msg.sender);
revert World_DelegationNotFound(delegator, msg.sender);
}

/************************************************************************
Expand All @@ -314,7 +314,7 @@ contract World is StoreRead, IStoreData, IWorldKernel {
fallback() external payable requireNoCallback {
(bytes32 systemId, bytes4 systemFunctionSelector) = FunctionSelectors._get(msg.sig);

if (systemId == 0) revert FunctionSelectorNotFound(msg.sig);
if (systemId == 0) revert World_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
4 changes: 2 additions & 2 deletions packages/world/src/interfaces/IModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ bytes4 constant MODULE_INTERFACE_ID = IModule.getName.selector ^
ERC165_INTERFACE_ID;

interface IModule is IERC165 {
error RootInstallModeNotSupported();
error NonRootInstallNotSupported();
error Module_RootInstallNotSupported();
error Module_NonRootInstallNotSupported();

/**
* Return the module name as a bytes16.
Expand Down
26 changes: 13 additions & 13 deletions packages/world/src/interfaces/IWorldErrors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@ pragma solidity >=0.8.21;
import { ResourceId } from "@latticexyz/store/src/ResourceId.sol";

interface IWorldErrors {
error WorldAlreadyInitialized();
error ResourceExists(ResourceId resourceId, string resourceIdString);
error ResourceNotFound(ResourceId resourceId, string resourceIdString);
error AccessDenied(string resource, address caller);
error InvalidResourceId(ResourceId resourceId, string resourceIdString);
error SystemExists(address system);
error FunctionSelectorExists(bytes4 functionSelector);
error FunctionSelectorNotFound(bytes4 functionSelector);
error DelegationNotFound(address delegator, address delegatee);
error InsufficientBalance(uint256 balance, uint256 amount);
error InterfaceNotSupported(address contractAddress, bytes4 interfaceId);
error InvalidResourceType(bytes2 expected, ResourceId resourceId, string resourceIdString);
error WorldCallbackNotAllowed(bytes4 functionSelector);
error World_AlreadyInitialized();
error World_ResourceAlreadyExists(ResourceId resourceId, string resourceIdString);
error World_ResourceNotFound(ResourceId resourceId, string resourceIdString);
error World_AccessDenied(string resource, address caller);
error World_InvalidResourceId(ResourceId resourceId, string resourceIdString);
error World_SystemAlreadyExists(address system);
error World_FunctionSelectorAlreadyExists(bytes4 functionSelector);
error World_FunctionSelectorNotFound(bytes4 functionSelector);
error World_DelegationNotFound(address delegator, address delegatee);
error World_InsufficientBalance(uint256 balance, uint256 amount);
error World_InterfaceNotSupported(address contractAddress, bytes4 interfaceId);
error World_InvalidResourceType(bytes2 expected, ResourceId resourceId, string resourceIdString);
error World_CallbackNotAllowed(bytes4 functionSelector);
}
2 changes: 1 addition & 1 deletion packages/world/src/modules/core/CoreModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ contract CoreModule is Module {
}

function install(bytes memory) public pure {
revert NonRootInstallNotSupported();
revert Module_NonRootInstallNotSupported();
}

/**
Expand Down
Loading

0 comments on commit d087892

Please sign in to comment.