Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(world): add CallBatchSystem to core module #1500

Merged
merged 25 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
f2d79e0
feat(world): add BatchCallModule
yonadaa Sep 15, 2023
e5bba67
fix: remove comment
yonadaa Sep 15, 2023
ca27801
Merge remote-tracking branch 'origin/main' into batch-call
yonadaa Sep 18, 2023
8ab8d04
chore: run gas report, rename to callDatas
yonadaa Sep 18, 2023
4f00381
feat: add batch call from root module
yonadaa Sep 18, 2023
c3c3d00
chore: add batch call to default module contracts
yonadaa Sep 18, 2023
ad2bebb
test: do not hardcode system address
yonadaa Sep 18, 2023
f7c9bc9
chore: run gas report
yonadaa Sep 18, 2023
7cd8904
Merge remote-tracking branch 'origin/main' into batch-call
yonadaa Sep 18, 2023
c2a9869
chore: delete non-root batch call module
yonadaa Sep 18, 2023
295ed8c
chore: remove mud config change
yonadaa Sep 18, 2023
7159e83
feat: rename to callBatch, add to core module
yonadaa Sep 18, 2023
312dba9
chore: rename gas report
yonadaa Sep 18, 2023
f27d7a2
feat: add return data to callBatch
yonadaa Sep 18, 2023
94cfee6
Merge remote-tracking branch 'origin/main' into batch-call
yonadaa Sep 18, 2023
2218493
chore: move comment
yonadaa Sep 18, 2023
059b0e6
chore: modify comment
yonadaa Sep 19, 2023
5cf3012
feat: use struct for callBatch arguments
yonadaa Sep 19, 2023
8300aa0
test: add access control tests for callBatch
yonadaa Sep 19, 2023
480b721
chore: tweak comment
yonadaa Sep 19, 2023
1affa39
refactor: move test constants
yonadaa Sep 19, 2023
e8144b7
Merge remote-tracking branch 'origin/main' into batch-call
yonadaa Sep 19, 2023
56f1094
Merge branch 'main' into batch-call
alvrs Sep 21, 2023
9f8a2a2
gas report
alvrs Sep 21, 2023
4ce27d2
Create wild-nails-wonder.md
alvrs Sep 21, 2023
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
1 change: 1 addition & 0 deletions packages/world/mud.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ export default mudConfig({
// TODO: Move optional modules into a separate package
// (see https://github.com/latticexyz/mud/pull/584)
"UniqueEntitySystem",
"BatchCallSystem",

// Worldgen currently does not support systems inheriting logic
// from other contracts, so all parts of CoreSystem are named
Expand Down
6 changes: 6 additions & 0 deletions packages/world/src/interfaces/IBatchCallSystem.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// SPDX-License-Identifier: MIT
pragma solidity >=0.8.0;

interface IBatchCallSystem {
function batchCall_system_batchCall(bytes32[] memory resourceSelectors, bytes[] memory funcSelectorAndArgs) external;
}
31 changes: 31 additions & 0 deletions packages/world/src/modules/batchcall/BatchCallModule.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// SPDX-License-Identifier: MIT
pragma solidity >=0.8.0;

import { IBaseWorld } from "../../interfaces/IBaseWorld.sol";

import { Module } from "../../Module.sol";
import { ResourceSelector } from "../../ResourceSelector.sol";

import { BatchCallSystem } from "./BatchCallSystem.sol";

import { NAMESPACE, MODULE_NAME, SYSTEM_NAME } from "./constants.sol";

contract BatchCallModule is Module {
BatchCallSystem private immutable batchCallSystem = new BatchCallSystem();

function getName() public pure returns (bytes16) {
return MODULE_NAME;
}

function installRoot(bytes memory args) public {
install(args);
}

function install(bytes memory) public {
IBaseWorld world = IBaseWorld(_world());

world.registerSystem(ResourceSelector.from(NAMESPACE, SYSTEM_NAME), batchCallSystem, true);

world.registerFunctionSelector(ResourceSelector.from(NAMESPACE, SYSTEM_NAME), "batchCall", "(bytes32[],bytes[])");
}
}
14 changes: 14 additions & 0 deletions packages/world/src/modules/batchcall/BatchCallSystem.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// SPDX-License-Identifier: MIT
pragma solidity >=0.8.0;
import { System } from "../../System.sol";
import { IBaseWorld } from "../../interfaces/IBaseWorld.sol";

contract BatchCallSystem is System {
function batchCall(bytes32[] memory resourceSelectors, bytes[] memory funcSelectorAndArgss) public {
Copy link
Member

@holic holic Sep 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be worse on calldata length if this was something like

struct SystemCall {
  bytes32 systemId;
  bytes callData;
}

function batchCall(SystemCall[] memory systemCalls) public {

(this feels cleaner but I would prob optimize for calldata/gas here, since these calls will be abstracted away anyway)

IBaseWorld world = IBaseWorld(_world());

for (uint256 i; i < resourceSelectors.length; i++) {
world.callFrom(_msgSender(), resourceSelectors[i], funcSelectorAndArgss[i]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we make this a root system and then do this so the users don't need to create a delegation?

(bool success, bytes memory returnData) = address(world).delegatecall(abi.encodeCall(
  world.call,
  (
    _msgSender(),
    resourceSelector[i],
    funcSelectorAndArgs[i]
  )
));
if(!success) revertWithBytes(returnData);

We can actually do both, have a BatchCallSystem and a BatchCallRootSystem, and install then in installModule and installRootModule respectively

Copy link
Member

@holic holic Sep 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this to be a module? if we intend to use this for deploys anyway, would it make sense to just have this under core/implementations as a plain root system? I also can't imagine you'd wanna install this more than once anyway?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it can also be part of the core module

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this approach without the delegation. I'm going to add the root version to the core module.

I'll also remove the non-root version, because the root version will be installed on worlds by default.

}
}
}
6 changes: 6 additions & 0 deletions packages/world/src/modules/batchcall/constants.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// SPDX-License-Identifier: MIT
pragma solidity >=0.8.0;

bytes16 constant NAMESPACE = bytes16("batchCall");
bytes16 constant MODULE_NAME = bytes16("batchCall.m");
bytes16 constant SYSTEM_NAME = bytes16("system");
89 changes: 89 additions & 0 deletions packages/world/test/BatchCallModule.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
// SPDX-License-Identifier: MIT
pragma solidity >=0.8.0;

import { Test } from "forge-std/Test.sol";
import { GasReporter } from "@latticexyz/gas-report/src/GasReporter.sol";

import { ROOT_NAMESPACE, UNLIMITED_DELEGATION } from "../src/constants.sol";
import { World } from "../src/World.sol";
import { IBaseWorld } from "../src/interfaces/IBaseWorld.sol";
import { IWorldErrors } from "../src/interfaces/IWorldErrors.sol";

import { CoreModule } from "../src/modules/core/CoreModule.sol";
import { BatchCallModule } from "../src/modules/batchcall/BatchCallModule.sol";
import { IBatchCallSystem } from "../src/interfaces/IBatchCallSystem.sol";

import { NAMESPACE } from "../src/modules/batchcall/constants.sol";
import { ResourceSelector } from "../src/ResourceSelector.sol";

import { WorldTestSystem } from "./World.t.sol";

contract BatchCallModuleTest is Test, GasReporter {
using ResourceSelector for bytes32;

IBaseWorld world;
BatchCallModule batchCallModule = new BatchCallModule();

function setUp() public {
world = IBaseWorld(address(new World()));
world.initialize(new CoreModule());
}

function testInstall() public {
startGasReport("install batch call module");
world.installModule(batchCallModule, new bytes(0));
endGasReport();

// Register a new system
WorldTestSystem system = new WorldTestSystem();
bytes32 resourceSelector = ResourceSelector.from("namespace", "testSystem");

world.registerSystem(resourceSelector, system, false);

bytes32[] memory resourceSelectors = new bytes32[](1);
bytes[] memory funcSelectorAndArgss = new bytes[](1);

resourceSelectors[0] = resourceSelector;
funcSelectorAndArgss[0] = abi.encodeWithSelector(WorldTestSystem.getStoreAddress.selector);

// TODO: do not hardcode this
address delegatee = 0x104fBc016F4bb334D775a19E8A6510109AC63E00;

vm.expectRevert(abi.encodeWithSelector(IWorldErrors.DelegationNotFound.selector, address(this), delegatee));
IBatchCallSystem(address(world)).batchCall_system_batchCall(resourceSelectors, funcSelectorAndArgss);

// Register an unlimited delegation
world.registerDelegation(delegatee, UNLIMITED_DELEGATION, new bytes(0));

IBatchCallSystem(address(world)).batchCall_system_batchCall(resourceSelectors, funcSelectorAndArgss);
}

function testInstallRoot() public {
startGasReport("installRoot batch call module");
world.installRootModule(batchCallModule, new bytes(0));
endGasReport();

// Register a new system
WorldTestSystem system = new WorldTestSystem();
bytes32 resourceSelector = ResourceSelector.from("namespace", "testSystem");

world.registerSystem(resourceSelector, system, false);

bytes32[] memory resourceSelectors = new bytes32[](1);
bytes[] memory funcSelectorAndArgss = new bytes[](1);

resourceSelectors[0] = resourceSelector;
funcSelectorAndArgss[0] = abi.encodeWithSelector(WorldTestSystem.getStoreAddress.selector);

// TODO: do not hardcode this
address delegatee = 0x104fBc016F4bb334D775a19E8A6510109AC63E00;

vm.expectRevert(abi.encodeWithSelector(IWorldErrors.DelegationNotFound.selector, address(this), delegatee));
IBatchCallSystem(address(world)).batchCall_system_batchCall(resourceSelectors, funcSelectorAndArgss);

// Register an unlimited delegation
world.registerDelegation(delegatee, UNLIMITED_DELEGATION, new bytes(0));

IBatchCallSystem(address(world)).batchCall_system_batchCall(resourceSelectors, funcSelectorAndArgss);
}
}