Skip to content
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

feat(cli,world): add user defined salt in WorldFactory.deployWorld() #2219

Merged
merged 23 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/happy-snails-sleep.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@latticexyz/cli": minor
"@latticexyz/world": major
---

`WorldFactory` now expects a user-provided `salt` when calling `deployWorld(...)` (instead of the previous globally incrementing counter). This enables deterministic world addresses across different chains.

When using `mud deploy`, you can provide a `bytes32` hex-encoded salt using the `--salt` option, otherwise it defaults to a random hex value.
1 change: 1 addition & 0 deletions packages/cli/src/commands/dev-contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ const commandModule: CommandModule<typeof devOptions, InferredOptionTypes<typeof
saveDeployment: true,
worldAddress,
srcDir,
salt: undefined,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be nice to do our own internally incrementing counter for this process here, so that restarting the dev process always results in the same contract address.

Copy link
Member

@holic holic Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, this might be harder than I initially thought it would be. Since we already pass the worldAddress in to upgrade worlds, this should be fine.

Just a note that this means that our tests that generate data, snapshots, etc. will be non-deterministic for now (unless we update them to pass in a consistent salt)

});
worldAddress = deploy.address;
// if there were changes while we were deploying, trigger it again
Expand Down
7 changes: 5 additions & 2 deletions packages/cli/src/deploy/deploy.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Account, Address, Chain, Client, Transport, getAddress } from "viem";
import { Account, Address, Chain, Client, Hex, Transport, getAddress } from "viem";
import { ensureDeployer } from "./ensureDeployer";
import { deployWorld } from "./deployWorld";
import { ensureTables } from "./ensureTables";
Expand All @@ -15,10 +15,12 @@ import { resourceLabel } from "./resourceLabel";
import { uniqueBy } from "@latticexyz/common/utils";
import { ensureContractsDeployed } from "./ensureContractsDeployed";
import { worldFactoryContracts } from "./ensureWorldFactory";
import { randomBytes } from "crypto";

type DeployOptions<configInput extends ConfigInput> = {
client: Client<Transport, Chain | undefined, Account>;
config: Config<configInput>;
salt?: Hex;
worldAddress?: Address;
};

Expand All @@ -31,6 +33,7 @@ type DeployOptions<configInput extends ConfigInput> = {
export async function deploy<configInput extends ConfigInput>({
client,
config,
salt,
worldAddress: existingWorldAddress,
}: DeployOptions<configInput>): Promise<WorldDeploy> {
const tables = Object.values(config.tables) as Table[];
Expand Down Expand Up @@ -58,7 +61,7 @@ export async function deploy<configInput extends ConfigInput>({

const worldDeploy = existingWorldAddress
? await getWorldDeploy(client, existingWorldAddress)
: await deployWorld(client);
: await deployWorld(client, salt ? salt : `0x${randomBytes(32).toString("hex")}`);

Copy link
Member

@holic holic Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want to check isHex here (edit: need to check where this is called, not here)

if (!supportedStoreVersions.includes(worldDeploy.storeVersion)) {
throw new Error(`Unsupported Store version: ${worldDeploy.storeVersion}`);
Expand Down
8 changes: 6 additions & 2 deletions packages/cli/src/deploy/deployWorld.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Account, Chain, Client, Log, Transport } from "viem";
import { Account, Chain, Client, Hex, Log, Transport } from "viem";
import { waitForTransactionReceipt } from "viem/actions";
import { ensureWorldFactory, worldFactory } from "./ensureWorldFactory";
import WorldFactoryAbi from "@latticexyz/world/out/WorldFactory.sol/WorldFactory.abi.json" assert { type: "json" };
Expand All @@ -7,7 +7,10 @@ import { debug } from "./debug";
import { logsToWorldDeploy } from "./logsToWorldDeploy";
import { WorldDeploy } from "./common";

export async function deployWorld(client: Client<Transport, Chain | undefined, Account>): Promise<WorldDeploy> {
export async function deployWorld(
client: Client<Transport, Chain | undefined, Account>,
salt: Hex
): Promise<WorldDeploy> {
await ensureWorldFactory(client);

debug("deploying world");
Expand All @@ -16,6 +19,7 @@ export async function deployWorld(client: Client<Transport, Chain | undefined, A
address: worldFactory,
abi: WorldFactoryAbi,
functionName: "deployWorld",
args: [salt],
});

debug("waiting for world deploy");
Expand Down
5 changes: 5 additions & 0 deletions packages/cli/src/runDeploy.ts
Copy link
Contributor

@yonadaa yonadaa Feb 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alvrs @holic should/do we test CLI commands?

For example, we have contract tests for this salt but that doesn't tell us if the CLI option actually works.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we sort of do in that our e2e tests lean on deploying a world with the mud cli (I think)

Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ export const deployOptions = {
type: "boolean",
desc: "Always run PostDeploy.s.sol after each deploy (including during upgrades). By default, PostDeploy.s.sol is only run once after a new world is deployed.",
},
salt: {
type: "string",
desc: "The deployment salt to use. Defaults to a random salt.",
},
} as const satisfies Record<string, Options>;

export type DeployOptions = InferredOptionTypes<typeof deployOptions>;
Expand Down Expand Up @@ -79,6 +83,7 @@ in your contracts directory to use the default anvil private key.`

const startTime = Date.now();
const worldDeploy = await deploy({
salt: opts.salt as Hex | undefined,
worldAddress: opts.worldAddress as Hex | undefined,
client,
config: resolvedConfig,
Expand Down
4 changes: 2 additions & 2 deletions packages/world/gas-report.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,13 @@
"file": "test/Factories.t.sol",
"test": "testCreate2Factory",
"name": "deploy contract via Create2",
"gasUsed": 4586875
"gasUsed": 4609895
},
{
"file": "test/Factories.t.sol",
"test": "testWorldFactoryGas",
"name": "deploy world via WorldFactory",
"gasUsed": 12716027
"gasUsed": 12694691
},
{
"file": "test/World.t.sol",
Expand Down
9 changes: 1 addition & 8 deletions packages/world/src/IWorldFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,10 @@ interface IWorldFactory {
*/
event WorldDeployed(address indexed newContract);

/**
* @notice Returns the total count of deployed World contracts per account.
* @param account The account.
* @return The total number of World contracts deployed by this factory per account.
*/
function worldCounts(address account) external view returns (uint256);

/**
* @notice Deploys a new World contract.
* @dev The deployment of the World contract will result in the `WorldDeployed` event being emitted.
* @return worldAddress The address of the newly deployed World contract.
*/
function deployWorld() external returns (address worldAddress);
function deployWorld(bytes memory salt) external returns (address worldAddress);
}
10 changes: 4 additions & 6 deletions packages/world/src/WorldFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ contract WorldFactory is IWorldFactory {
/// @notice Address of the init module to be set in the World instances.
IModule public immutable initModule;

/// @notice Counters to keep track of the number of World instances deployed per address.
mapping(address creator => uint256 worldCount) public worldCounts;

/// @param _initModule The address of the init module.
constructor(IModule _initModule) {
initModule = _initModule;
Expand All @@ -28,13 +25,14 @@ contract WorldFactory is IWorldFactory {
/**
* @notice Deploys a new World instance, installs the InitModule and transfers ownership to the caller.
* @dev Uses the Create2 for deterministic deployment.
* @param salt User defined salt for deterministic world addresses across chains
* @return worldAddress The address of the newly deployed World contract.
*/
function deployWorld() public returns (address worldAddress) {
function deployWorld(bytes memory salt) public returns (address worldAddress) {
// Deploy a new World and increase the WorldCount
bytes memory bytecode = type(World).creationCode;
uint256 salt = uint256(keccak256(abi.encode(msg.sender, worldCounts[msg.sender]++)));
worldAddress = Create2.deploy(bytecode, salt);
uint256 _salt = uint256(keccak256(abi.encode(msg.sender, salt)));
worldAddress = Create2.deploy(bytecode, _salt);
IBaseWorld world = IBaseWorld(worldAddress);

// Initialize the World and transfer ownership to the caller
Expand Down
40 changes: 21 additions & 19 deletions packages/world/test/Factories.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -48,24 +48,24 @@ contract FactoriesTest is Test, GasReporter {
startGasReport("deploy contract via Create2");
create2Factory.deployContract(combinedBytes, uint256(0));
endGasReport();

// Confirm worldFactory was deployed correctly
IWorldFactory worldFactory = IWorldFactory(calculatedAddress);
assertEq(uint256(worldFactory.worldCounts(address(0))), uint256(0));
}

function testWorldFactory(address account) public {
function testWorldFactory(address account, uint256 salt1, uint256 salt2) public {
vm.assume(salt1 != salt2);
vm.startPrank(account);

// Deploy WorldFactory with current InitModule
InitModule initModule = createInitModule();
address worldFactoryAddress = address(new WorldFactory(initModule));
IWorldFactory worldFactory = IWorldFactory(worldFactoryAddress);

// Address we expect for World
// User defined bytes for create2
bytes memory _salt1 = abi.encode(salt1);

// Address we expect for first World
address calculatedAddress = calculateAddress(
worldFactoryAddress,
keccak256(abi.encode(account, 0)),
keccak256(abi.encode(account, _salt1)),
type(World).creationCode
);

Expand All @@ -77,27 +77,28 @@ contract FactoriesTest is Test, GasReporter {
vm.expectEmit(true, false, false, false);
emit WorldDeployed(calculatedAddress);
startGasReport("deploy world via WorldFactory");
worldFactory.deployWorld();
worldFactory.deployWorld(_salt1);
endGasReport();

// Set the store address manually
StoreSwitch.setStoreAddress(calculatedAddress);

// Confirm accountCount (which is salt) has incremented
assertEq(uint256(worldFactory.worldCounts(account)), uint256(1));

// Confirm correct Core is installed
assertTrue(InstalledModules.get(address(initModule), keccak256(new bytes(0))));

// Confirm the msg.sender is owner of the root namespace of the new world
assertEq(NamespaceOwner.get(ROOT_NAMESPACE_ID), account);

// Deploy another world
// Deploy a second world

// Address we expect for World
// User defined bytes for create2
// unchecked for the fuzzing test
bytes memory _salt2 = abi.encode(salt2);

// Address we expect for second World
calculatedAddress = calculateAddress(
worldFactoryAddress,
keccak256(abi.encode(account, 1)),
keccak256(abi.encode(account, _salt2)),
type(World).creationCode
);

Expand All @@ -108,10 +109,7 @@ contract FactoriesTest is Test, GasReporter {
// Check for WorldDeployed event from Factory
vm.expectEmit(true, false, false, false);
emit WorldDeployed(calculatedAddress);
worldFactory.deployWorld();

// Confirm accountCount (which is salt) has incremented
assertEq(uint256(worldFactory.worldCounts(account)), uint256(2));
worldFactory.deployWorld(_salt2);

// Set the store address manually
StoreSwitch.setStoreAddress(calculatedAddress);
Expand All @@ -121,9 +119,13 @@ contract FactoriesTest is Test, GasReporter {

// Confirm the msg.sender is owner of the root namespace of the new world
assertEq(NamespaceOwner.get(ROOT_NAMESPACE_ID), account);

// Expect revert when deploying world with same bytes salt as already deployed world
vm.expectRevert();
worldFactory.deployWorld(_salt1);
}

function testWorldFactoryGas() public {
testWorldFactory(address(this));
testWorldFactory(address(this), 0, 1);
}
}
Loading