fix(world): identify modules by addresses instead of names [L-08]

[yonadaaa]

Modules are now identified by address instead of a single, global name.

• Modules have a __self variable that stores their address, allowing them to check if they have been installed.
• The ERC20 and ERC721 modules store the address of their puppet module in an immutable variable. This is passed into the registration helpers so will require a change for consumers (see the Sky Strife PostDeploy script)

[changeset-bot]

⚠️ No Changeset found

Latest commit: 8ce1239

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[yonadaaa]

The big change here is that we can't read data on modules unless we know the address of a specific instantiation.

For example, ERC20Module needs to know whether PuppetModule is installed, so expects it to be installed under "puppet":

// 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.

[yonadaaa]

TODO:

• Add a isInitialized table the World to prevent initialising the world multiple times.
• Declare the address of a puppet module as an immutable variable in the constructor of the ERC20/ERC721 module

[alvrs]

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.

[yonadaaa]

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

[alvrs]

this one got pulled out to the other PR right?

[alvrs]

i guess we can merge the other one first and then rebase this one

[yonadaaa]

Yes exactly, this PR should really not have those changes but it only occurred to me later to break it out