-
Notifications
You must be signed in to change notification settings - Fork 7
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
Issue #14: smart character refactor into non root module #16
Closed
MerkleBoy
wants to merge
15
commits into
develop
from
ccp-merkleboy/14-smart-character-refactor-into-non-root-module
Closed
Issue #14: smart character refactor into non root module #16
MerkleBoy
wants to merge
15
commits into
develop
from
ccp-merkleboy/14-smart-character-refactor-into-non-root-module
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
it is now possible to install a module through a Solidity script containing two lines: ``` SmartObjectFrameworkModule module = new SmartObjectFrameworkModule(); IBaseWorld(worldAddress).installModule(module, abi.encode(namespace)); ``` `Deploying with namespace==bytes14(0)` is forbidden for security reasons; the fact that _namespace() has been added to `EveSystem` means it will probably break the current Access Control PR since there's systems there that also have their own implementation of it, but since the PR has not been merged yet it's okay.
There's a misconfig somewhere and it's driving me insane idk if it's, like, a wrong tableID or some registration tomfoolery at play, or just some general deployment shennanigan but there's *something* happening with the Test smart-deployable-system Anyways, I know the structure is (for the most part,) fine, so it's just a matter of debugging this.
This is a dirty fix, where we need to manually make sure that the namespace used to access tables in EveSystem matches the one given during the Smart Object Framework deployment; because we don't know in advance where that module is going to be deployed exactly, and because the goal is for contracts to inherit `EveSystem` methods everywhere, we can't just use whatever _namespace() returns because it would only work for core contracts and nothing else e.g. builders trying to deploy new systems, _namespace() method inside EveSystem will fetch the namespace they deployed their stuff to, and that's bad anyways. There might be a clean and efficient way to do this that doesnt require to use hard-coded values, but I don;'t see it for now. It's not meant for us to deploy a bunch of core contract suites anywaym it only needs to be done once, so hardcoding stuff is fine and gas-efficient, at the cost of potentially breaking stuff if we need to re-deploy a new suite in the future. If anything, it shouldnt be too hard to catch that
same as smart-object-framework, added unit tests
Co-authored-by: skygirl <[email protected]>
as 0xxlegolas said it's defaulted to true so there's no need to explicitly add this flag
This reverts commit ffbea18.
creating a `Modules` contract so that we can easily control in which namespace that module will be deployed to, and applying changes that goes with it
…-into-non-root-module
…-into-non-root-module
MerkleBoy
deleted the
ccp-merkleboy/14-smart-character-refactor-into-non-root-module
branch
April 3, 2024 13:51
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
following discussion from #10
#10 (comment)