Skip to content

Commit

Permalink
refactor(world): encapsulate functionality [N-01] (#2157)
Browse files Browse the repository at this point in the history
Co-authored-by: Kevin Ingersoll <[email protected]>
  • Loading branch information
yonadaa and holic authored Jan 19, 2024
1 parent 5be712f commit 61985bb
Show file tree
Hide file tree
Showing 8 changed files with 187 additions and 30 deletions.
28 changes: 14 additions & 14 deletions packages/world-modules/gas-report.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,13 @@
"file": "test/KeysInTableModule.t.sol",
"test": "testInstallComposite",
"name": "install keys in table module",
"gasUsed": 1438969
"gasUsed": 1439079
},
{
"file": "test/KeysInTableModule.t.sol",
"test": "testInstallGas",
"name": "install keys in table module",
"gasUsed": 1438969
"gasUsed": 1439079
},
{
"file": "test/KeysInTableModule.t.sol",
Expand All @@ -93,13 +93,13 @@
"file": "test/KeysInTableModule.t.sol",
"test": "testInstallSingleton",
"name": "install keys in table module",
"gasUsed": 1438969
"gasUsed": 1439079
},
{
"file": "test/KeysInTableModule.t.sol",
"test": "testSetAndDeleteRecordHookCompositeGas",
"name": "install keys in table module",
"gasUsed": 1438969
"gasUsed": 1439079
},
{
"file": "test/KeysInTableModule.t.sol",
Expand All @@ -117,7 +117,7 @@
"file": "test/KeysInTableModule.t.sol",
"test": "testSetAndDeleteRecordHookGas",
"name": "install keys in table module",
"gasUsed": 1438969
"gasUsed": 1439079
},
{
"file": "test/KeysInTableModule.t.sol",
Expand All @@ -135,7 +135,7 @@
"file": "test/KeysWithValueModule.t.sol",
"test": "testGetKeysWithValueGas",
"name": "install keys with value module",
"gasUsed": 684371
"gasUsed": 684459
},
{
"file": "test/KeysWithValueModule.t.sol",
Expand All @@ -153,7 +153,7 @@
"file": "test/KeysWithValueModule.t.sol",
"test": "testInstall",
"name": "install keys with value module",
"gasUsed": 684371
"gasUsed": 684459
},
{
"file": "test/KeysWithValueModule.t.sol",
Expand All @@ -165,7 +165,7 @@
"file": "test/KeysWithValueModule.t.sol",
"test": "testSetAndDeleteRecordHook",
"name": "install keys with value module",
"gasUsed": 684371
"gasUsed": 684459
},
{
"file": "test/KeysWithValueModule.t.sol",
Expand All @@ -183,7 +183,7 @@
"file": "test/KeysWithValueModule.t.sol",
"test": "testSetField",
"name": "install keys with value module",
"gasUsed": 684371
"gasUsed": 684459
},
{
"file": "test/KeysWithValueModule.t.sol",
Expand Down Expand Up @@ -267,7 +267,7 @@
"file": "test/StandardDelegationsModule.t.sol",
"test": "testCallFromCallboundDelegation",
"name": "register a callbound delegation",
"gasUsed": 118175
"gasUsed": 118198
},
{
"file": "test/StandardDelegationsModule.t.sol",
Expand All @@ -279,7 +279,7 @@
"file": "test/StandardDelegationsModule.t.sol",
"test": "testCallFromSystemDelegation",
"name": "register a systembound delegation",
"gasUsed": 115728
"gasUsed": 115751
},
{
"file": "test/StandardDelegationsModule.t.sol",
Expand All @@ -291,7 +291,7 @@
"file": "test/StandardDelegationsModule.t.sol",
"test": "testCallFromTimeboundDelegation",
"name": "register a timebound delegation",
"gasUsed": 112651
"gasUsed": 112674
},
{
"file": "test/StandardDelegationsModule.t.sol",
Expand All @@ -303,7 +303,7 @@
"file": "test/UniqueEntityModule.t.sol",
"test": "testInstall",
"name": "install unique entity module",
"gasUsed": 704240
"gasUsed": 704219
},
{
"file": "test/UniqueEntityModule.t.sol",
Expand All @@ -315,7 +315,7 @@
"file": "test/UniqueEntityModule.t.sol",
"test": "testInstallRoot",
"name": "installRoot unique entity module",
"gasUsed": 673144
"gasUsed": 673145
},
{
"file": "test/UniqueEntityModule.t.sol",
Expand Down
24 changes: 18 additions & 6 deletions packages/world/gas-report.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
"file": "test/Factories.t.sol",
"test": "testWorldFactory",
"name": "deploy world via WorldFactory",
"gasUsed": 12512902
"gasUsed": 12642949
},
{
"file": "test/World.t.sol",
Expand All @@ -81,7 +81,7 @@
"file": "test/World.t.sol",
"test": "testCallFromUnlimitedDelegation",
"name": "register an unlimited delegation",
"gasUsed": 47584
"gasUsed": 47607
},
{
"file": "test/World.t.sol",
Expand All @@ -105,7 +105,7 @@
"file": "test/World.t.sol",
"test": "testRegisterFunctionSelector",
"name": "Register a function selector",
"gasUsed": 84519
"gasUsed": 84542
},
{
"file": "test/World.t.sol",
Expand All @@ -117,20 +117,26 @@
"file": "test/World.t.sol",
"test": "testRegisterRootFunctionSelector",
"name": "Register a root function selector",
"gasUsed": 80467
"gasUsed": 80445
},
{
"file": "test/World.t.sol",
"test": "testRegisterSystem",
"name": "register a system",
"gasUsed": 164385
"gasUsed": 164363
},
{
"file": "test/World.t.sol",
"test": "testRegisterTable",
"name": "Register a new table in the namespace",
"gasUsed": 536881
},
{
"file": "test/World.t.sol",
"test": "testRenounceNamespace",
"name": "Renounce namespace ownership",
"gasUsed": 36773
},
{
"file": "test/World.t.sol",
"test": "testSetField",
Expand All @@ -143,11 +149,17 @@
"name": "Write data to the table",
"gasUsed": 39047
},
{
"file": "test/World.t.sol",
"test": "testUnregisterNamespaceDelegation",
"name": "unregister a namespace delegation",
"gasUsed": 28360
},
{
"file": "test/World.t.sol",
"test": "testUnregisterUnlimitedDelegation",
"name": "unregister an unlimited delegation",
"gasUsed": 26173
"gasUsed": 26255
},
{
"file": "test/WorldDynamicUpdate.t.sol",
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 6 additions & 4 deletions packages/world/src/modules/core/CoreModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,12 @@ contract CoreModule is Module {
* @dev Iterates through known function signatures and registers them.
*/
function _registerFunctionSelectors() internal {
string[3] memory functionSignaturesAccessManagement = [
string[4] memory functionSignaturesAccessManagement = [
// --- AccessManagementSystem ---
"grantAccess(bytes32,address)",
"revokeAccess(bytes32,address)",
"transferOwnership(bytes32,address)"
"transferOwnership(bytes32,address)",
"renounceOwnership(bytes32)"
];
for (uint256 i = 0; i < functionSignaturesAccessManagement.length; i++) {
_registerRootFunctionSelector(ACCESS_MANAGEMENT_SYSTEM_ID, functionSignaturesAccessManagement[i]);
Expand All @@ -169,7 +170,7 @@ contract CoreModule is Module {
_registerRootFunctionSelector(BATCH_CALL_SYSTEM_ID, functionSignaturesBatchCall[i]);
}

string[13] memory functionSignaturesCoreRegistration = [
string[14] memory functionSignaturesCoreRegistration = [
// --- ModuleInstallationSystem ---
"installModule(address,bytes)",
// --- StoreRegistrationSystem ---
Expand All @@ -185,7 +186,8 @@ contract CoreModule is Module {
"registerRootFunctionSelector(bytes32,string,bytes4)",
"registerDelegation(address,bytes32,bytes)",
"unregisterDelegation(address)",
"registerNamespaceDelegation(bytes32,bytes32,bytes)"
"registerNamespaceDelegation(bytes32,bytes32,bytes)",
"unregisterNamespaceDelegation(bytes32)"
];
for (uint256 i = 0; i < functionSignaturesCoreRegistration.length; i++) {
_registerRootFunctionSelector(CORE_REGISTRATION_SYSTEM_ID, functionSignaturesCoreRegistration[i]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ contract AccessManagementSystem is System, LimitedCallContext {
* @param newOwner The address to which ownership should be transferred.
*/
function transferOwnership(ResourceId namespaceId, address newOwner) public virtual onlyDelegatecall {
// Require the namespace to be a valid namespace ID
// Require the namespace ID to be a valid namespace
requireNamespace(namespaceId);

// Require the namespace to exist
Expand All @@ -71,4 +71,26 @@ contract AccessManagementSystem is System, LimitedCallContext {
// Grant access to new owner
ResourceAccess._set(namespaceId, newOwner, true);
}

/**
* @notice Renounces ownership of the given namespace
* @dev Requires the caller to own the namespace. Revoke ResourceAccess for previous owner
* @param namespaceId The ID of the namespace to transfer ownership.
*/
function renounceOwnership(ResourceId namespaceId) public virtual onlyDelegatecall {
// Require the namespace ID to be a valid namespace
requireNamespace(namespaceId);

// Require the namespace to exist
AccessControl.requireExistence(namespaceId);

// Require the caller to own the namespace
AccessControl.requireOwner(namespaceId, _msgSender());

// Delete namespace owner
NamespaceOwner._deleteRecord(namespaceId);

// Revoke access from old owner
ResourceAccess._deleteRecord(namespaceId, _msgSender());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ contract WorldRegistrationSystem is System, IWorldErrors, LimitedCallContext {
* @param namespaceId The unique identifier for the new namespace
*/
function registerNamespace(ResourceId namespaceId) public virtual onlyDelegatecall {
// Require namespace to be a valid namespace ID
// Require namespace ID to be a valid namespace
requireNamespace(namespaceId);

// Require namespace to not exist yet
Expand Down Expand Up @@ -284,7 +284,12 @@ contract WorldRegistrationSystem is System, IWorldErrors, LimitedCallContext {
}
}

function unregisterDelegation(address delegatee) public {
/**
* @notice Unregisters a delegation
* @dev Deletes the new delegation from the caller to the specified delegatee
* @param delegatee The address of the delegatee
*/
function unregisterDelegation(address delegatee) public onlyDelegatecall {
// Delete the delegation control contract address
UserDelegationControl.deleteRecord({ delegator: _msgSender(), delegatee: delegatee });
}
Expand All @@ -301,7 +306,7 @@ contract WorldRegistrationSystem is System, IWorldErrors, LimitedCallContext {
ResourceId delegationControlId,
bytes memory initCallData
) public onlyDelegatecall {
// Require namespace to be a valid namespace ID
// Require namespace ID to be a valid namespace
requireNamespace(namespaceId);

// Require the delegation to not be unlimited
Expand Down Expand Up @@ -329,4 +334,23 @@ contract WorldRegistrationSystem is System, IWorldErrors, LimitedCallContext {
});
}
}

/**
* @notice Unregisters a delegation for a namespace
* @dev Deletes the delegation control for a specific namespace
* @param namespaceId The ID of the namespace
*/
function unregisterNamespaceDelegation(ResourceId namespaceId) public onlyDelegatecall {
// Require namespace ID to be a valid namespace
requireNamespace(namespaceId);

// Require the namespace to exist
AccessControl.requireExistence(namespaceId);

// Require the caller to own the namespace
AccessControl.requireOwner(namespaceId, _msgSender());

// Delete the delegation control
NamespaceDelegationControl.deleteRecord(namespaceId);
}
}
Loading

0 comments on commit 61985bb

Please sign in to comment.