-
Notifications
You must be signed in to change notification settings - Fork 196
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(cli,world,world-modules): split and separately deploy core systems #2128
Changes from 5 commits
091e988
8605d63
15d4041
599b685
3683ba6
0449f6e
263e8c7
42def3d
d7004a2
c498bf9
88ce864
5849b21
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,5 @@ | ||
--- | ||
"@latticexyz/cli": major | ||
--- | ||
|
||
Separated core systems deployment from `CoreModule`, and added the systems as arguments to `CoreModule` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
--- | ||
"@latticexyz/world": major | ||
--- | ||
|
||
- Split `CoreSystem` into `AccessManagementSystem`, `BalanceTransferSystem`, `BatchCallSystem`, `CoreRegistrationSystem` | ||
- Changed `CoreModule` to receive the addresses of these systems as arguments, instead of deploying them | ||
- Replaced `CORE_SYSTEM_ID` constant with `ACCESS_MANAGEMENT_SYSTEM_ID`, `BALANCE_TRANSFER_SYSTEM_ID`, `BATCH_CALL_SYSTEM_ID`, `CORE_REGISTRATION_SYSTEM_ID`, for each respective system | ||
|
||
These changes separate the initcode of `CoreModule` from the bytecode of core systems, which effectively removes a limit on the total bytecode of all core systems. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
import accessManagementSystemBuild from "@latticexyz/world/out/AccessManagementSystem.sol/AccessManagementSystem.json" assert { type: "json" }; | ||
import balanceTransferSystemBuild from "@latticexyz/world/out/BalanceTransferSystem.sol/BalanceTransferSystem.json" assert { type: "json" }; | ||
import batchCallSystemBuild from "@latticexyz/world/out/BatchCallSystem.sol/BatchCallSystem.json" assert { type: "json" }; | ||
import coreRegistrationSystemBuild from "@latticexyz/world/out/CoreRegistrationSystem.sol/CoreRegistrationSystem.json" assert { type: "json" }; | ||
import coreModuleBuild from "@latticexyz/world/out/CoreModule.sol/CoreModule.json" assert { type: "json" }; | ||
import worldFactoryBuild from "@latticexyz/world/out/WorldFactory.sol/WorldFactory.json" assert { type: "json" }; | ||
import { Client, Transport, Chain, Account, Hex, parseAbi, getCreate2Address, encodeDeployData, size } from "viem"; | ||
|
@@ -6,10 +10,57 @@ import { salt } from "./common"; | |
import { ensureContractsDeployed } from "./ensureContractsDeployed"; | ||
import { Contract } from "./ensureContract"; | ||
|
||
export const accessManagementSystemDeployedBytecodeSize = size( | ||
accessManagementSystemBuild.deployedBytecode.object as Hex | ||
); | ||
export const accessManagementSystemBytecode = encodeDeployData({ | ||
bytecode: accessManagementSystemBuild.bytecode.object as Hex, | ||
abi: [], | ||
}); | ||
export const accessManagementSystem = getCreate2Address({ | ||
from: deployer, | ||
bytecode: accessManagementSystemBytecode, | ||
salt, | ||
}); | ||
|
||
export const balanceTransferSystemDeployedBytecodeSize = size( | ||
balanceTransferSystemBuild.deployedBytecode.object as Hex | ||
); | ||
export const balanceTransferSystemBytecode = encodeDeployData({ | ||
bytecode: balanceTransferSystemBuild.bytecode.object as Hex, | ||
abi: [], | ||
}); | ||
export const balanceTransferSystem = getCreate2Address({ | ||
from: deployer, | ||
bytecode: balanceTransferSystemBytecode, | ||
salt, | ||
}); | ||
|
||
export const batchCallSystemDeployedBytecodeSize = size(batchCallSystemBuild.deployedBytecode.object as Hex); | ||
export const batchCallSystemBytecode = encodeDeployData({ | ||
bytecode: batchCallSystemBuild.bytecode.object as Hex, | ||
abi: [], | ||
}); | ||
export const batchCallSystem = getCreate2Address({ from: deployer, bytecode: batchCallSystemBytecode, salt }); | ||
|
||
export const coreRegistrationSystemDeployedBytecodeSize = size( | ||
coreRegistrationSystemBuild.deployedBytecode.object as Hex | ||
); | ||
export const coreRegistrationSystemBytecode = encodeDeployData({ | ||
bytecode: coreRegistrationSystemBuild.bytecode.object as Hex, | ||
abi: [], | ||
}); | ||
export const coreRegistrationSystem = getCreate2Address({ | ||
from: deployer, | ||
bytecode: coreRegistrationSystemBytecode, | ||
salt, | ||
}); | ||
|
||
export const coreModuleDeployedBytecodeSize = size(coreModuleBuild.deployedBytecode.object as Hex); | ||
export const coreModuleBytecode = encodeDeployData({ | ||
bytecode: coreModuleBuild.bytecode.object as Hex, | ||
abi: [], | ||
abi: parseAbi(["constructor(address,address,address,address)"]), | ||
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. is it possible to get this form an import of the code module? so this can throw a type error when it changes instead of a runtime error if the abi doesn't match 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. Maybe with abitype (we don't have it in cli package yet) 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. I think what he means is importing the typed ABI, i.e. import coreModuleAbi from "@latticexyz/world/out/CoreModule.sol/CoreModule.abi.json" assert { type: "json" };
export const coreModuleBytecode = encodeDeployData({
bytecode: coreModuleBuild.bytecode.object as Hex,
abi: coreModuleAbi,
...
}); 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. Oh I see, well yeah point is I remember next to no context for this |
||
args: [accessManagementSystem, balanceTransferSystem, batchCallSystem, coreRegistrationSystem], | ||
}); | ||
|
||
export const coreModule = getCreate2Address({ from: deployer, bytecode: coreModuleBytecode, salt }); | ||
|
@@ -24,6 +75,26 @@ export const worldFactoryBytecode = encodeDeployData({ | |
export const worldFactory = getCreate2Address({ from: deployer, bytecode: worldFactoryBytecode, salt }); | ||
|
||
export const worldFactoryContracts: readonly Contract[] = [ | ||
{ | ||
bytecode: accessManagementSystemBytecode, | ||
deployedBytecodeSize: accessManagementSystemDeployedBytecodeSize, | ||
label: "access management system", | ||
}, | ||
{ | ||
bytecode: balanceTransferSystemBytecode, | ||
deployedBytecodeSize: balanceTransferSystemDeployedBytecodeSize, | ||
label: "balance transfer system", | ||
}, | ||
{ | ||
bytecode: batchCallSystemBytecode, | ||
deployedBytecodeSize: batchCallSystemDeployedBytecodeSize, | ||
label: "batch call system", | ||
}, | ||
{ | ||
bytecode: coreRegistrationSystemBytecode, | ||
deployedBytecodeSize: coreRegistrationSystemDeployedBytecodeSize, | ||
label: "core registration system", | ||
}, | ||
{ | ||
bytecode: coreModuleBytecode, | ||
deployedBytecodeSize: coreModuleDeployedBytecodeSize, | ||
|
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.
Thoughts on
CoreRegistrationSystem
vs justRegistrationSystem
?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.
CoreRegistrationSystem
is less generic, easier to search for, fitsCoreModule
a bit betterNot a strong opinion
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.
none of the other systems are called "core" though 🤷
(I think this is fine either way and easy enough to rename later if we decide to)
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.
I guess I see
Core
as some aggregation prefixagree that it's fine either way