feat(world-module-metadata): add metadata module #3026
Conversation
🦋 Changeset detectedLatest commit: 43dba06 The changes in this PR will be included in the next version bump. This PR includes changesets to release 25 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
holic
force-pushed
the
holic/metadata-module
branch
from
21b7c00 to
e27c5a7
Compare
This reverts commit 83b32f5.
holic
force-pushed
the
holic/metadata-module
branch
from
7640a80 to
9866564
Compare
| world.registerFunctionSelector(metadataSystemId, "setResource(bytes32,bytes32,string)"); | ||
| } | ||
|
|
||
| world.transferOwnership(namespace, _msgSender()); |
There was a problem hiding this comment.
this pattern feels a little weird to me, but curious to hear what you think:
on first install, metadata module creates metadata namespace, registers tables/systems, hands ownership back to original caller
on second install (idempotent, would upgrade systems or add tables), you'd first need to hand ownership of the namespace to the module, then it hands it back after install
should the world or something else own the metadata namespace? if so, how would you perform an upgrade to the metadata module?
or maybe we should consider the metadata module more of a "locked in" thing, rename it to something more specific like "resource tag module" and if we want more metadata, we add as another module and namespace rather than trying to figure out how to upgrade this one?
There was a problem hiding this comment.
i think this pattern is fine! makes it possible to upgrade, which is nice
holic
changed the title
| world.installModule(metadataModule, new bytes(0)); | ||
|
|
||
| // Transferring the namespace to the module and installing should be a no-op/idempotent and the module will return namespace ownership. | ||
| // TODO: is this a security concern? could someone frontrun by putting a tx in between the two calls below? |
There was a problem hiding this comment.
you can always make it atomic by doing both via a batchcall. But even without i don't think there is a security concern here, the module would just own the namespace, but unless the module has functions to do something malicious with or is upgradable, another tx in between couldn't do anything else with it
| * To avoid circular dependencies, we run a very similar `build` step as `cli` package here. | ||
| */ | ||
|
|
||
| // TODO: move tablegen/worldgen to CLI commands from store/world we can run in package.json instead of a custom script |
There was a problem hiding this comment.
wanna open an issue for this? seems like a "good first issue"
part of #2981