-
Notifications
You must be signed in to change notification settings - Fork 203
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(store-consumer): adapt WithWorld to be a System #3546
Changes from all commits
f61ca11
e02e718
77e7c77
4fcb9fd
8709d40
5baa242
f7d8eb0
c862636
6403c30
46645e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
--- | ||
"@latticexyz/store-consumer": patch | ||
"@latticexyz/store": patch | ||
"@latticexyz/world-module-erc20": patch | ||
"@latticexyz/world": patch | ||
--- | ||
|
||
Updates `WithWorld` to be a `System`, so that functions in child contracts that use the `onlyWorld` or `onlyNamespace` modifiers must be called through the world in order to safely support calls from systems. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,21 +6,32 @@ import { ResourceId } from "@latticexyz/store/src/ResourceId.sol"; | |
import { ResourceAccess } from "@latticexyz/world/src/codegen/tables/ResourceAccess.sol"; | ||
import { IBaseWorld } from "@latticexyz/world/src/codegen/interfaces/IBaseWorld.sol"; | ||
import { WorldResourceIdLib } from "@latticexyz/world/src/WorldResourceId.sol"; | ||
import { WorldContextConsumer } from "@latticexyz/world/src/WorldContext.sol"; | ||
import { System } from "@latticexyz/world/src/System.sol"; | ||
|
||
import { Context } from "./Context.sol"; | ||
import { WithStore } from "./WithStore.sol"; | ||
|
||
abstract contract WithWorld is WithStore { | ||
abstract contract WithWorld is WithStore, System { | ||
bytes14 public immutable namespace; | ||
|
||
error WithWorld_RootNamespaceNotAllowed(); | ||
error WithWorld_NamespaceAlreadyExists(); | ||
error WithWorld_NamespaceDoesNotExists(); | ||
error WithWorld_CallerHasNoNamespaceAccess(); | ||
error WithWorld_NamespaceAlreadyExists(bytes14 namespace); | ||
error WithWorld_NamespaceDoesNotExists(bytes14 namespace); | ||
error WithWorld_CallerHasNoNamespaceAccess(bytes14 namespace, address caller); | ||
error WithWorld_CallerIsNotWorld(address caller); | ||
|
||
modifier onlyWorld() { | ||
if (!_callerIsWorld()) { | ||
revert WithWorld_CallerIsNotWorld(msg.sender); | ||
} | ||
_; | ||
} | ||
|
||
modifier onlyNamespace() { | ||
address sender = _msgSender(); | ||
if (!ResourceAccess.get(getNamespaceId(), sender)) { | ||
revert WithWorld_CallerHasNoNamespaceAccess(); | ||
// We use WorldContextConsumer directly as we already know the world is the caller | ||
if (!_callerIsWorld() || !ResourceAccess.get(getNamespaceId(), WorldContextConsumer._msgSender())) { | ||
Comment on lines
+32
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so if my EOA has access to the namespace I still have to call the token system through the world correct? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, I thought about checking for supporting it for both EOAs and world, but I felt it was more consistent this way. |
||
revert WithWorld_CallerHasNoNamespaceAccess(namespace, _msgSender()); | ||
} | ||
_; | ||
} | ||
|
@@ -38,12 +49,12 @@ abstract contract WithWorld is WithStore { | |
|
||
if (registerNamespace) { | ||
if (namespaceExists) { | ||
revert WithWorld_NamespaceAlreadyExists(); | ||
revert WithWorld_NamespaceAlreadyExists(_namespace); | ||
} | ||
|
||
_world.registerNamespace(namespaceId); | ||
} else if (!namespaceExists) { | ||
revert WithWorld_NamespaceDoesNotExists(); | ||
revert WithWorld_NamespaceDoesNotExists(_namespace); | ||
} | ||
} | ||
|
||
|
@@ -55,6 +66,14 @@ abstract contract WithWorld is WithStore { | |
return IBaseWorld(getStore()); | ||
} | ||
|
||
function _msgSender() public view virtual override(Context, WorldContextConsumer) returns (address sender) { | ||
return _callerIsWorld() ? WorldContextConsumer._msgSender() : Context._msgSender(); | ||
} | ||
|
||
function _callerIsWorld() internal view returns (bool) { | ||
return msg.sender == address(getWorld()); | ||
} | ||
|
||
function _encodeResourceId(bytes2 typeId, bytes16 name) internal view virtual override returns (ResourceId) { | ||
return WorldResourceIdLib.encode(typeId, namespace, name); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooc why public?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because
_msgSender
inWorldContext.sol
is public so we need to be able to overrideThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for context: it's public to have a unique interface id we can check for in
supportsInterface
so only contracts that are intended to be used as systems can be registered as systems