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

automatically convert ASCII salt to hex #3257

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
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
29 changes: 13 additions & 16 deletions packages/cli/src/runDeploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ import { configToModules } from "./deploy/configToModules";
import { findContractArtifacts } from "@latticexyz/world/node";
import { enableAutomine } from "./utils/enableAutomine";

function encodeToHex(str: string): Hex {
return ("0x" + Buffer.from(str, "utf8").toString("hex")) as Hex;
}

Comment on lines +22 to +25
Copy link
Member

Choose a reason for hiding this comment

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

there is a util from viem we can use instead: https://viem.sh/docs/utilities/toHex.html

export const deployOptions = {
configPath: { type: "string", desc: "Path to the MUD config file" },
printConfig: { type: "boolean", desc: "Print the resolved config" },
Expand Down Expand Up @@ -52,14 +56,13 @@ export const deployOptions = {

export type DeployOptions = InferredOptionTypes<typeof deployOptions>;

/**
* Given some CLI arguments, finds and resolves a MUD config, foundry profile, and runs a deploy.
* This is used by the deploy, test, and dev-contracts CLI commands.
*/
Copy link
Member

Choose a reason for hiding this comment

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

why were these comments removed?

export async function runDeploy(opts: DeployOptions): Promise<WorldDeploy> {
const salt = opts.salt;
if (salt != null && !isHex(salt)) {
throw new MUDError("Expected hex string for salt");
let salt: `0x${string}` | undefined = opts.salt as `0x${string}` | undefined;

if (salt != null) {
if (!isHex(salt)) {
salt = encodeToHex(salt) as `0x${string}`;
}
Comment on lines +60 to +65
Copy link
Member

Choose a reason for hiding this comment

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

we can simplify this to const salt = opts.salt && !isHex(opts.salt) ? toHex(opts.salt) : opts.salt;

}

const profile = opts.profile ?? process.env.FOUNDRY_PROFILE;
Expand All @@ -81,7 +84,6 @@ export async function runDeploy(opts: DeployOptions): Promise<WorldDeploy> {
),
);

// Run build
if (!opts.skipBuild) {
await build({ rootDir, config, foundryProfile: profile });
}
Expand All @@ -92,7 +94,6 @@ export async function runDeploy(opts: DeployOptions): Promise<WorldDeploy> {
forgeOutDir: outDir,
});
const artifacts = await findContractArtifacts({ forgeOutDir: outDir });
// TODO: pass artifacts into configToModules (https://github.com/latticexyz/mud/issues/3153)
const modules = await configToModules(config, outDir);

const tables = Object.values(config.namespaces)
Expand All @@ -114,8 +115,8 @@ export async function runDeploy(opts: DeployOptions): Promise<WorldDeploy> {
if (!privateKey) {
throw new MUDError(
`Missing PRIVATE_KEY environment variable.
Run 'echo "PRIVATE_KEY=0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" > .env'
in your contracts directory to use the default anvil private key.`,
Run 'echo "PRIVATE_KEY=0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" > .env'
in your contracts directory to use the default anvil private key.`,
Comment on lines -117 to +119
Copy link
Member

Choose a reason for hiding this comment

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

let's not change the formatting here

);
}

Expand All @@ -137,14 +138,13 @@ export async function runDeploy(opts: DeployOptions): Promise<WorldDeploy> {

console.log("Deploying from", client.account.address);

// Attempt to enable automine for the duration of the deploy. Noop if automine is not available.
const automine = await enableAutomine(client);

const startTime = Date.now();
const worldDeploy = await deploy({
config,
deployerAddress: opts.deployerAddress as Hex | undefined,
salt,
salt, // Use the new salt variable here
Copy link
Member

Choose a reason for hiding this comment

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

no need to add a comment here

worldAddress: opts.worldAddress as Hex | undefined,
client,
tables,
Expand All @@ -164,7 +164,6 @@ export async function runDeploy(opts: DeployOptions): Promise<WorldDeploy> {
);
}

// Reset mining mode after deploy
await automine?.reset();

console.log(chalk.green("Deployment completed in", (Date.now() - startTime) / 1000, "seconds"));
Expand All @@ -187,8 +186,6 @@ export async function runDeploy(opts: DeployOptions): Promise<WorldDeploy> {
: {};
deploys[chainId] = {
address: deploymentInfo.worldAddress,
// We expect the worlds file to be committed and since local deployments are often
// a consistent address but different block number, we'll ignore the block number.
Comment on lines -190 to -191
Copy link
Member

Choose a reason for hiding this comment

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

let's keep all these comments

blockNumber: localChains.includes(chainId) ? undefined : deploymentInfo.blockNumber,
};
writeFileSync(config.deploy.worldsFile, JSON.stringify(deploys, null, 2));
Expand Down
Loading