fix(cli): support public library methods in modules (latticexyz#3308)

Author: vdrg

.changeset/gentle-carrots-kneel.md

@@ -0,0 +1,5 @@
+---
+"@latticexyz/cli": patch
+---
+
+Added support for deploying public libraries used within modules.

.changeset/little-ways-change.md

@@ -0,0 +1,6 @@
+---
+"@latticexyz/world-module-erc20": patch
+"@latticexyz/world-modules": patch
+---
+
+Changed ERC20 and ERC721 related modules to use public library methods instead of manual `delegatecall`s.

e2e/pnpm-lock.yaml

@@ -7,7 +7,7 @@
   },
   "type": "module",
   "scripts": {
-    "all-build": "for dir in packages/store packages/world packages/world-modules packages/cli test/mock-game-contracts e2e/packages/contracts examples/*/packages/contracts examples/multiple-namespaces templates/*/packages/contracts; do (cd \"$dir\" && pwd && pnpm build); done",
+    "all-build": "for dir in packages/store packages/world packages/world-modules packages/cli test/mock-game-contracts test/puppet-modules e2e/packages/contracts examples/*/packages/contracts examples/multiple-namespaces templates/*/packages/contracts; do (cd \"$dir\" && pwd && pnpm build); done",
     "all-install": "for dir in . docs e2e examples/* templates/*; do (cd \"$dir\" && pwd && pnpm install); done",
     "bench": "pnpm run --recursive bench",
     "build": "turbo run build --concurrency=100%",

package.json

@@ -7,6 +7,8 @@ import { groupBy } from "@latticexyz/common/utils";
 import { findLibraries } from "./findLibraries";
 import { createPrepareDeploy } from "./createPrepareDeploy";
 import { World } from "@latticexyz/world";
+import { findUp } from "find-up";
+import { createRequire } from "node:module";
 
 // TODO: replace this with a manifest/combined config output
 
@@ -22,18 +24,34 @@ export async function resolveConfig({
   readonly systems: readonly System[];
   readonly libraries: readonly Library[];
 }> {
-  const libraries = findLibraries(forgeOutDir).map((library): Library => {
-    // foundry/solc flattens artifacts, so we just use the path basename
-    const contractData = getContractData(path.basename(library.path), library.name, forgeOutDir);
-    return {
-      path: library.path,
-      name: library.name,
-      abi: contractData.abi,
-      prepareDeploy: createPrepareDeploy(contractData.bytecode, contractData.placeholders),
-      deployedBytecodeSize: contractData.deployedBytecodeSize,
-    };
+  const requirePath = await findUp("package.json");
+  if (!requirePath) throw new Error("Could not find package.json to import relative to.");
+  const require = createRequire(requirePath);
+
+  const moduleOutDirs = config.modules.flatMap((mod) => {
+    if (mod.artifactPath == undefined) {
+      return [];
+    }
+
+    // Navigate up two dirs to get the contract output directory
+    const moduleOutDir = path.join(require.resolve(mod.artifactPath), "../../");
+    return [moduleOutDir];
   });
 
+  const libraries = [forgeOutDir, ...moduleOutDirs].flatMap((outDir) =>
+    findLibraries(outDir).map((library): Library => {
+      // foundry/solc flattens artifacts, so we just use the path basename
+      const contractData = getContractData(path.basename(library.path), library.name, outDir);
+      return {
+        path: library.path,
+        name: library.name,
+        abi: contractData.abi,
+        prepareDeploy: createPrepareDeploy(contractData.bytecode, contractData.placeholders),
+        deployedBytecodeSize: contractData.deployedBytecodeSize,
+      };
+    }),
+  );
+
   const baseSystemContractData = getContractData("System.sol", "System", forgeOutDir);
   const baseSystemFunctions = baseSystemContractData.abi
     .filter((item): item is typeof item & { type: "function" } => item.type === "function")

packages/cli/src/deploy/resolveConfig.ts

@@ -91,6 +91,7 @@ export async function runDeploy(opts: DeployOptions): Promise<WorldDeploy> {
     config,
     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);

packages/cli/src/runDeploy.ts

@@ -11,18 +11,24 @@ export type GetContractArtifactResult = {
   deployedBytecodeSize: number;
 };
 
+function isBytecode(value: string): value is Hex {
+  return isHex(value, { strict: false });
+}
+
 const bytecodeSchema = z.object({
-  object: z.string().refine(isHex),
-  linkReferences: z.record(
-    z.record(
-      z.array(
-        z.object({
-          start: z.number(),
-          length: z.number(),
-        }),
+  object: z.string().refine(isBytecode),
+  linkReferences: z
+    .record(
+      z.record(
+        z.array(
+          z.object({
+            start: z.number(),
+            length: z.number(),
+          }),
+        ),
       ),
-    ),
-  ),
+    )
+    .optional(),
 });
 
 const artifactSchema = z.object({
@@ -34,7 +40,7 @@ const artifactSchema = z.object({
 export function getContractArtifact(artifactJson: unknown): GetContractArtifactResult {
   // TODO: improve errors or replace with arktype?
   const artifact = artifactSchema.parse(artifactJson);
-  const placeholders = findPlaceholders(artifact.bytecode.linkReferences);
+  const placeholders = findPlaceholders(artifact.bytecode.linkReferences ?? {});
 
   return {
     abi: artifact.abi,

packages/cli/src/utils/getContractArtifact.ts

@@ -135,7 +135,7 @@
     "file": "test/ERC20Module.t.sol:ERC20ModuleTest",
     "test": "testInstall",
     "name": "install erc20 module",
-    "gasUsed": 4524262
+    "gasUsed": 4523802
   },
   {
     "file": "test/ERC20Pausable.t.sol:ERC20PausableWithInternalStoreTest",

packages/world-module-erc20/gas-report.json

@@ -17,8 +17,6 @@ contract ERC20Module is Module {
   error ERC20Module_InvalidNamespace(bytes14 namespace);
   error ERC20Module_NamespaceAlreadyExists(bytes14 namespace);
 
-  ERC20RegistryLib public immutable registryLib = new ERC20RegistryLib();
-
   function install(bytes memory encodedArgs) public {
     // TODO: we should probably check just for namespace, not for all args
     requireNotInstalled(__self, encodedArgs);
@@ -44,15 +42,15 @@ contract ERC20Module is Module {
     // The token should have transferred the namespace ownership to this module in its constructor
     world.transferOwnership(namespaceId, _msgSender());
 
-    registryLib.delegateRegister(world, namespaceId, address(token));
+    ERC20RegistryLib.register(world, namespaceId, address(token));
   }
 
   function installRoot(bytes memory) public pure {
     revert Module_RootInstallNotSupported();
   }
 }
 
-contract ERC20RegistryLib {
+library ERC20RegistryLib {
   function register(IBaseWorld world, ResourceId namespaceId, address token) public {
     ResourceId erc20RegistryTableId = ModuleConstants.registryTableId();
     if (!ResourceIds.getExists(erc20RegistryTableId)) {
@@ -63,12 +61,3 @@ contract ERC20RegistryLib {
     ERC20Registry.set(erc20RegistryTableId, namespaceId, address(token));
   }
 }
-
-function delegateRegister(ERC20RegistryLib lib, IBaseWorld world, ResourceId namespaceId, address token) {
-  (bool success, bytes memory returnData) = address(lib).delegatecall(
-    abi.encodeCall(ERC20RegistryLib.register, (world, namespaceId, token))
-  );
-  if (!success) revertWithBytes(returnData);
-}
-
-using { delegateRegister } for ERC20RegistryLib;

packages/world-module-erc20/src/ERC20Module.sol

@@ -46,10 +46,7 @@ contract ERC20ModuleTest is Test, GasReporter {
     ResourceId erc20NamespaceId = WorldResourceIdLib.encodeNamespace(TestConstants.ERC20_NAMESPACE);
 
     // Token should retain access to the namespace
-    address token = ERC20Registry.get(
-      erc20RegistryTableId,
-      WorldResourceIdLib.encodeNamespace(TestConstants.ERC20_NAMESPACE)
-    );
+    address token = ERC20Registry.get(erc20RegistryTableId, erc20NamespaceId);
 
     // Module should grant the token access to the token namespace
     assertTrue(ResourceAccess.get(erc20NamespaceId, token), "Token does not have access to its namespace");

packages/world-module-erc20/test/ERC20Module.t.sol

@@ -24,8 +24,6 @@ import { ERC20Metadata, ERC20MetadataData } from "./tables/ERC20Metadata.sol";
 contract ERC20Module is Module {
   error ERC20Module_InvalidNamespace(bytes14 namespace);
 
-  address immutable registrationLibrary = address(new ERC20ModuleRegistrationLibrary());
-
   function install(bytes memory encodedArgs) public {
     // Require the module to not be installed with these args yet
     requireNotInstalled(__self, encodedArgs);
@@ -40,10 +38,7 @@ contract ERC20Module is Module {
 
     // Register the ERC20 tables and system
     IBaseWorld world = IBaseWorld(_world());
-    (bool success, bytes memory returnData) = registrationLibrary.delegatecall(
-      abi.encodeCall(ERC20ModuleRegistrationLibrary.register, (world, namespace))
-    );
-    if (!success) revertWithBytes(returnData);
+    ERC20ModuleRegistrationLib.register(world, namespace);
 
     // Initialize the Metadata
     ERC20Metadata.set(_metadataTableId(namespace), metadata);
@@ -69,7 +64,7 @@ contract ERC20Module is Module {
   }
 }
 
-contract ERC20ModuleRegistrationLibrary {
+library ERC20ModuleRegistrationLib {
   /**
    * Register systems and tables for a new ERC20 token in a given namespace
    */

packages/world-modules/src/modules/erc20-puppet/ERC20Module.sol

@@ -27,8 +27,6 @@ import { ERC721Metadata, ERC721MetadataData } from "./tables/ERC721Metadata.sol"
 contract ERC721Module is Module {
   error ERC721Module_InvalidNamespace(bytes14 namespace);
 
-  address immutable registrationLibrary = address(new ERC721ModuleRegistrationLibrary());
-
   function install(bytes memory encodedArgs) public {
     // Require the module to not be installed with these args yet
     requireNotInstalled(__self, encodedArgs);
@@ -43,10 +41,7 @@ contract ERC721Module is Module {
 
     // Register the ERC721 tables and system
     IBaseWorld world = IBaseWorld(_world());
-    (bool success, bytes memory returnData) = registrationLibrary.delegatecall(
-      abi.encodeCall(ERC721ModuleRegistrationLibrary.register, (world, namespace))
-    );
-    if (!success) revertWithBytes(returnData);
+    ERC721ModuleRegistrationLib.register(world, namespace);
 
     // Initialize the Metadata
     ERC721Metadata.set(_metadataTableId(namespace), metadata);
@@ -72,7 +67,7 @@ contract ERC721Module is Module {
   }
 }
 
-contract ERC721ModuleRegistrationLibrary {
+library ERC721ModuleRegistrationLib {
   /**
    * Register systems and tables for a new ERC721 token in a given namespace
    */