fix(world): identify modules by addresses instead of names [L-08]#2168
fix(world): identify modules by addresses instead of names [L-08]#2168
Conversation
|
|
The big change here is that we can't read data on modules unless we know the address of a specific instantiation. For example, // Require PuppetModule to be installed
if (!isInstalled(PUPPET_MODULE_NAME, new bytes(0))) {
revert Module_MissingDependency(string(bytes.concat(PUPPET_MODULE_NAME)));
}However, this is not foolproof because anything could be installed under that name. As of this PR modules are referenced by address. So, we will have to pass in the address of the puppet module when installing the ERC20 module. |
|
TODO:
|
90c7acb to
056dce2
Compare
056dce2 to
6a517cd
Compare
| */ | ||
| function registerERC20( | ||
| IBaseWorld world, | ||
| address puppetModule, |
There was a problem hiding this comment.
i feel like this additional params is making it pretty impractical to use since it's currently not easy to find the address of the puppet module. Why don't we add an overload for this function that doesn't require the puppetModule address and instead calls this overload with the CREATE2 default address of the puppet module? That way it's not a breaking change for consumers (on CREATE2 compatible chains) but still allows customisation if necessary.
There was a problem hiding this comment.
The puppet module is part of the deployment pipeline right now, so doesn't use Create2. For example Sky Strife manually deploys the puppet: https://github.com/latticexyz/skystrife-public/blob/main/packages/contracts/script/PostDeploy.s.sol#L111-L119
packages/world/mud.config.ts
Outdated
| WorldInitialized: { | ||
| keySchema: {}, | ||
| valueSchema: { | ||
| isInitialized: "bool", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
this one got pulled out to the other PR right?
There was a problem hiding this comment.
i guess we can merge the other one first and then rebase this one
There was a problem hiding this comment.
Yes exactly, this PR should really not have those changes but it only occurred to me later to break it out
Modules are now identified by address instead of a single, global name.
__selfvariable that stores their address, allowing them to check if they have been installed.