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

refactor(world): separate call utils into WorldContextProvider and SystemCall #1370

Merged
merged 9 commits into from
Aug 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
116 changes: 116 additions & 0 deletions .changeset/quiet-squids-share.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
---
"@latticexyz/world": major
---

- The previous `Call.withSender` util is replaced with `WorldContextProvider`, since the usecase of appending the `msg.sender` to the calldata is tightly coupled with `WorldContextConsumer` (which extracts the appended context from the calldata).

The previous `Call.withSender` utility reverted if the call failed and only returned the returndata on success. This is replaced with `callWithContextOrRevert`/`delegatecallWithContextOrRevert`

```diff
-import { Call } from "@latticexyz/world/src/Call.sol";
+import { WorldContextProvider } from "@latticexyz/world/src/WorldContext.sol";

-Call.withSender({
- delegate: false,
- value: 0,
- ...
-});
+WorldContextProvider.callWithContextOrRevert({
+ value: 0,
+ ...
+});

-Call.withSender({
- delegate: true,
- value: 0,
- ...
-});
+WorldContextProvider.delegatecallWithContextOrRevert({
+ ...
+});
```

In addition there are utils that return a `bool success` flag instead of reverting on errors. This mirrors the behavior of Solidity's low level `call`/`delegatecall` functions and is useful in situations where additional logic should be executed in case of a reverting external call.

```solidity
library WorldContextProvider {
function callWithContext(
address target, // Address to call
bytes memory funcSelectorAndArgs, // Abi encoded function selector and arguments to pass to pass to the contract
address msgSender, // Address to append to the calldata as context for msgSender
uint256 value // Value to pass with the call
) internal returns (bool success, bytes memory data);

function delegatecallWithContext(
address target, // Address to call
bytes memory funcSelectorAndArgs, // Abi encoded function selector and arguments to pass to pass to the contract
address msgSender // Address to append to the calldata as context for msgSender
) internal returns (bool success, bytes memory data);
}
```

- `WorldContext` is renamed to `WorldContextConsumer` to clarify the relationship between `WorldContextProvider` (appending context to the calldata) and `WorldContextConsumer` (extracting context from the calldata)

```diff
-import { WorldContext } from "@latticexyz/world/src/WorldContext.sol";
-import { WorldContextConsumer } from "@latticexyz/world/src/WorldContext.sol";
```

- The `World` contract previously had a `_call` method to handle calling systems via their resource selector, performing accesss control checks and call hooks registered for the system.

```solidity
library SystemCall {
/**
* Calls a system via its resource selector and perform access control checks.
* Does not revert if the call fails, but returns a `success` flag along with the returndata.
*/
function call(
Copy link
Member

Choose a reason for hiding this comment

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

should this have a corresponding callOrRevert?

address caller,
bytes32 resourceSelector,
bytes memory funcSelectorAndArgs,
uint256 value
) internal returns (bool success, bytes memory data);

/**
* Calls a system via its resource selector, perform access control checks and trigger hooks registered for the system.
* Does not revert if the call fails, but returns a `success` flag along with the returndata.
Copy link
Member

Choose a reason for hiding this comment

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

I assume hooks are only called on success?

*/
function callWithHooks(
address caller,
bytes32 resourceSelector,
bytes memory funcSelectorAndArgs,
uint256 value
) internal returns (bool success, bytes memory data);

/**
* Calls a system via its resource selector, perform access control checks and trigger hooks registered for the system.
* Reverts if the call fails.
*/
function callWithHooksOrRevert(
address caller,
bytes32 resourceSelector,
bytes memory funcSelectorAndArgs,
uint256 value
) internal returns (bytes memory data);
}
```

- System hooks now are called with the system's resource selector instead of its address. The system's address can still easily obtained within the hook via `Systems.get(resourceSelector)` if necessary.

```diff
interface ISystemHook {
function onBeforeCallSystem(
address msgSender,
- address systemAddress,
+ bytes32 resourceSelector,
bytes memory funcSelectorAndArgs
) external;

function onAfterCallSystem(
address msgSender,
- address systemAddress,
+ bytes32 resourceSelector,
bytes memory funcSelectorAndArgs
) external;
}
```
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,7 @@ const _abi = [
type: "bytes32",
},
{
internalType: "contract System",
internalType: "contract WorldContextConsumer",
name: "system",
type: "address",
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,7 @@ const _abi = [
type: "bytes32",
},
{
internalType: "contract System",
internalType: "contract WorldContextConsumer",
name: "system",
type: "address",
},
Expand Down
2 changes: 1 addition & 1 deletion packages/world/abi/CoreSystem.sol/CoreSystem.abi.json
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@
"type": "bytes32"
},
{
"internalType": "contract System",
"internalType": "contract WorldContextConsumer",
"name": "system",
"type": "address"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/world/abi/IBaseWorld.sol/IBaseWorld.abi.json
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,7 @@
"type": "bytes32"
},
{
"internalType": "contract System",
"internalType": "contract WorldContextConsumer",
"name": "system",
"type": "address"
},
Expand Down
12 changes: 6 additions & 6 deletions packages/world/abi/ISystemHook.sol/ISystemHook.abi.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
"type": "address"
},
{
"internalType": "address",
"name": "systemAddress",
"type": "address"
"internalType": "bytes32",
"name": "resourceSelector",
"type": "bytes32"
},
{
"internalType": "bytes",
Expand All @@ -30,9 +30,9 @@
"type": "address"
},
{
"internalType": "address",
"name": "systemAddress",
"type": "address"
"internalType": "bytes32",
"name": "resourceSelector",
"type": "bytes32"
},
{
"internalType": "bytes",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
"type": "bytes32"
},
{
"internalType": "contract System",
"internalType": "contract WorldContextConsumer",
"name": "system",
"type": "address"
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[]
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@
"type": "bytes32"
},
{
"internalType": "contract System",
"internalType": "contract WorldContextConsumer",
"name": "system",
"type": "address"
},
Expand Down
Loading