Handle empty output in ABI for /write (#707)

* Handle empty output in ABI for /write * fix write tests * fix: sdk generation failing when using type.merge * minor test improvements --------- Co-authored-by: Prithvish Baidya <[email protected]>
thirdweb-dev · Oct 5, 2024 · 921449d · 921449d
1 parent e12dcaf
commit 921449d
Show file tree

Hide file tree

Showing 12 changed files with 126 additions and 56 deletions.
diff --git a/src/scripts/generate-sdk.ts b/src/scripts/generate-sdk.ts
@@ -1,8 +1,9 @@
-import { execSync } from "child_process";
-import fs from "fs";
-import { kill } from "process";
+import { execSync } from "node:child_process";
+import fs from "node:fs";
+import { kill } from "node:process";
 
-const ENGINE_OPENAPI_URL = "https://demo.web3api.thirdweb.com/json";
+// requires engine to be running locally
+const ENGINE_OPENAPI_URL = "http://localhost:3005/json";
 
 async function main() {
   try {
@@ -22,7 +23,8 @@ async function main() {
 
     const code = fs
       .readFileSync("./sdk/src/Engine.ts", "utf-8")
-      .replace(`export class Engine`, `class EngineLogic`).concat(`
+      .replace("export class Engine", "class EngineLogic")
+      .concat(`
 export class Engine extends EngineLogic {
   constructor(config: { url: string; accessToken: string; }) {
     super({ BASE: config.url, TOKEN: config.accessToken });

diff --git a/src/server/middleware/error.ts b/src/server/middleware/error.ts
@@ -1,5 +1,6 @@
 import type { FastifyInstance } from "fastify";
 import { ReasonPhrases, StatusCodes } from "http-status-codes";
+import { stringify } from "thirdweb/utils";
 import { ZodError } from "zod";
 import { env } from "../../utils/env";
 import { parseEthersError } from "../../utils/ethers";
@@ -22,6 +23,13 @@ export const createCustomError = (
   code,
 });
 
+export function formatError(error: unknown) {
+  if (error instanceof Error) {
+    return error.message;
+  }
+  return stringify(error);
+}
+
 export const customDateTimestampError = (date: string): CustomError =>
   createCustomError(
     `Invalid date: ${date}. Needs to new Date() / new Date().toISOstring() / new Date().getTime() / Unix Epoch`,

diff --git a/src/server/routes/contract/write/write.ts b/src/server/routes/contract/write/write.ts
@@ -1,12 +1,12 @@
-import { Type, type Static } from "@sinclair/typebox";
+import { type Static, Type } from "@sinclair/typebox";
 import type { FastifyInstance } from "fastify";
 import { StatusCodes } from "http-status-codes";
 import { prepareContractCall, resolveMethod } from "thirdweb";
-import { stringify, type AbiFunction } from "thirdweb/utils";
+import type { AbiFunction } from "thirdweb/utils";
 import { getContractV5 } from "../../../../utils/cache/getContractv5";
 import { queueTransaction } from "../../../../utils/transaction/queueTransation";
-import { createCustomError } from "../../../middleware/error";
-import { abiSchema } from "../../../schemas/contract";
+import { createCustomError, formatError } from "../../../middleware/error";
+import { abiArraySchema } from "../../../schemas/contract";
 import {
   contractParamSchema,
   requestQuerystringSchema,
@@ -18,6 +18,7 @@ import {
   requiredAddress,
   walletWithAAHeaderSchema,
 } from "../../../schemas/wallet";
+import { sanitizeAbi } from "../../../utils/abi";
 import { getChainIdFromChain } from "../../../utils/chain";
 import { parseTransactionOverrides } from "../../../utils/transactionOverrides";
 
@@ -30,7 +31,7 @@ const writeRequestBodySchema = Type.Object({
     description: "The arguments to call on the function",
   }),
   ...txOverridesWithValueSchema.properties,
-  abi: Type.Optional(Type.Array(abiSchema)),
+  abi: Type.Optional(abiArraySchema),
 });
 
 // LOGIC
@@ -71,7 +72,7 @@ export async function writeToContract(fastify: FastifyInstance) {
       const contract = await getContractV5({
         chainId,
         contractAddress,
-        abi,
+        abi: sanitizeAbi(abi),
       });
 
       // 3 possible ways to get function from abi:
@@ -82,9 +83,9 @@ export async function writeToContract(fastify: FastifyInstance) {
       let method: AbiFunction;
       try {
         method = await resolveMethod(functionName)(contract);
-      } catch (e: any) {
+      } catch (e) {
         throw createCustomError(
-          stringify(e),
+          formatError(e),
           StatusCodes.BAD_REQUEST,
           "BAD_REQUEST",
         );

diff --git a/src/server/routes/transaction/blockchain/getLogs.ts b/src/server/routes/transaction/blockchain/getLogs.ts
@@ -54,29 +54,31 @@ const LogSchema = Type.Object({
   blockHash: Type.String(),
   logIndex: Type.Number(),
   removed: Type.Boolean(),
-});
 
-const ParsedLogSchema = Type.Object({
-  ...LogSchema.properties,
-  eventName: Type.String(),
-  args: Type.Unknown({
-    description: "Event arguments.",
-    examples: [
-      {
-        from: "0xdeadbeeefdeadbeeefdeadbeeefdeadbeeefdead",
-        to: "0xdeadbeeefdeadbeeefdeadbeeefdeadbeeefdead",
-        value: "1000000000000000000n",
-      },
-    ],
-  }),
+  // Additional properties only for parsed logs
+  eventName: Type.Optional(
+    Type.String({
+      description: "Event name, only returned when `parseLogs` is true",
+    }),
+  ),
+  args: Type.Optional(
+    Type.Unknown({
+      description: "Event arguments. Only returned when `parseLogs` is true",
+      examples: [
+        {
+          from: "0xdeadbeeefdeadbeeefdeadbeeefdeadbeeefdead",
+          to: "0xdeadbeeefdeadbeeefdeadbeeefdeadbeeefdead",
+          value: "1000000000000000000n",
+        },
+      ],
+    }),
+  ),
 });
 
+// DO NOT USE type.union
+// this is known to cause issues with the generated types
 export const responseBodySchema = Type.Object({
-  result: Type.Union([
-    // ParsedLogSchema is listed before LogSchema because it is more specific.
-    Type.Array(ParsedLogSchema),
-    Type.Array(LogSchema),
-  ]),
+  result: Type.Array(LogSchema),
 });
 
 responseBodySchema.example = {
@@ -221,7 +223,7 @@ export async function getTransactionLogs(fastify: FastifyInstance) {
 
       reply.status(StatusCodes.OK).send({
         result: superjson.serialize(parsedLogs).json as Static<
-          typeof ParsedLogSchema
+          typeof LogSchema
         >[],
       });
     },

diff --git a/src/server/schemas/contract/index.ts b/src/server/schemas/contract/index.ts
@@ -1,4 +1,4 @@
-import { Type, type Static } from "@sinclair/typebox";
+import { type Static, Type } from "@sinclair/typebox";
 import type { contractSchemaTypes } from "../sharedApiSchemas";
 
 /**
@@ -61,6 +61,9 @@ export const abiSchema = Type.Object({
   stateMutability: Type.Optional(Type.String()),
 });
 
+export const abiArraySchema = Type.Array(abiSchema);
+export type AbiSchemaType = Static<typeof abiArraySchema>;
+
 export const contractEventSchema = Type.Record(Type.String(), Type.Any());
 
 export const rolesResponseSchema = Type.Object({

diff --git a/src/server/utils/abi.ts b/src/server/utils/abi.ts
@@ -0,0 +1,17 @@
+import type { Abi } from "thirdweb/utils";
+import type { AbiSchemaType } from "../schemas/contract";
+
+export function sanitizeAbi(abi: AbiSchemaType | undefined): Abi | undefined {
+  if (!abi) return undefined;
+  return abi.map((item) => {
+    if (item.type === "function") {
+      return {
+        ...item,
+        // older versions of engine allowed passing in empty inputs/outputs, but necesasry for abi validation
+        inputs: item.inputs || [],
+        outputs: item.outputs || [],
+      };
+    }
+    return item;
+  }) as Abi;
+}
diff --git a/src/utils/cache/getContractv5.ts b/src/utils/cache/getContractv5.ts
@@ -1,19 +1,20 @@
-import { getContract } from "thirdweb";
+import { type ThirdwebContract, getContract } from "thirdweb";
+import type { Abi } from "thirdweb/utils";
 import { thirdwebClient } from "../../utils/sdk";
 import { getChain } from "../chain";
 
 interface GetContractParams {
   chainId: number;
   contractAddress: string;
-  abi?: any;
+  abi?: Abi;
 }
 
 // Using new v5 SDK
 export const getContractV5 = async ({
   chainId,
   contractAddress,
   abi,
-}: GetContractParams) => {
+}: GetContractParams): Promise<ThirdwebContract> => {
   const definedChain = await getChain(chainId);
 
   // get a contract
@@ -25,5 +26,5 @@ export const getContractV5 = async ({
     // the chain the contract is deployed on
     chain: definedChain,
     abi,
-  });
+  }) as ThirdwebContract; // not using type inference here;
 };
diff --git a/src/utils/transaction/queueTransation.ts b/src/utils/transaction/queueTransation.ts
@@ -1,10 +1,10 @@
 import type { Static } from "@sinclair/typebox";
 import { StatusCodes } from "http-status-codes";
 import {
-  encode,
   type Address,
   type Hex,
   type PreparedTransaction,
+  encode,
 } from "thirdweb";
 import { stringify } from "thirdweb/utils";
 import { createCustomError } from "../../server/middleware/error";

diff --git a/test/e2e/tests/smoke.test.ts b/test/e2e/tests/smoke.test.ts
@@ -12,16 +12,13 @@ describe("Smoke Test", () => {
       backendWallet,
       {
         amount: "0",
-        currencyAddress: "0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee",
         to: backendWallet,
       },
     );
 
-    expect(res.result.queueId).toBeDefined();
-
     const transactionStatus = await pollTransactionStatus(
       engine,
-      res.result.queueId!,
+      res.result.queueId,
       true,
     );
 

diff --git a/test/e2e/tests/write.test.ts b/test/e2e/tests/write.test.ts
@@ -1,6 +1,8 @@
 import { beforeAll, describe, expect, test } from "bun:test";
+import assert from "node:assert";
 import type { Address } from "thirdweb";
 import { zeroAddress } from "viem";
+import type { ApiError } from "../../../sdk/dist/thirdweb-dev-engine.cjs";
 import { CONFIG } from "../config";
 import type { setupEngine } from "../utils/engine";
 import { pollTransactionStatus } from "../utils/transactions";
@@ -14,7 +16,7 @@ describe("Write Tests", () => {
   beforeAll(async () => {
     const { engine: _engine, backendWallet: _backendWallet } = await setup();
     engine = _engine;
-    backendWallet = _backendWallet;
+    backendWallet = _backendWallet as Address;
 
     const res = await engine.deploy.deployToken(
       CONFIG.CHAIN.id.toString(),
@@ -31,16 +33,18 @@ describe("Write Tests", () => {
     );
 
     expect(res.result.queueId).toBeDefined();
+    assert(res.result.queueId, "queueId must be defined");
     expect(res.result.deployedAddress).toBeDefined();
 
     const transactionStatus = await pollTransactionStatus(
       engine,
-      res.result.queueId!,
+      res.result.queueId,
       true,
     );
 
     expect(transactionStatus.minedAt).toBeDefined();
-    tokenContractAddress = res.result.deployedAddress!;
+    assert(res.result.deployedAddress, "deployedAddress must be defined");
+    tokenContractAddress = res.result.deployedAddress;
     console.log("tokenContractAddress", tokenContractAddress);
   });
 
@@ -59,7 +63,7 @@ describe("Write Tests", () => {
 
     const writeTransactionStatus = await pollTransactionStatus(
       engine,
-      writeRes.result.queueId!,
+      writeRes.result.queueId,
       true,
     );
 
@@ -81,7 +85,7 @@ describe("Write Tests", () => {
 
     const writeTransactionStatus = await pollTransactionStatus(
       engine,
-      writeRes.result.queueId!,
+      writeRes.result.queueId,
       true,
     );
 
@@ -107,7 +111,7 @@ describe("Write Tests", () => {
             name: "setContractURI",
             stateMutability: "nonpayable",
             type: "function",
-            // outputs: [],
+            outputs: [],
           },
         ],
       },
@@ -117,14 +121,49 @@ describe("Write Tests", () => {
 
     const writeTransactionStatus = await pollTransactionStatus(
       engine,
-      writeRes.result.queueId!,
+      writeRes.result.queueId,
       true,
     );
 
     expect(writeTransactionStatus.minedAt).toBeDefined();
   });
 
-  test.only("Should throw error if function name is not found", async () => {
+  test("Write to a contract with non-standard abi", async () => {
+    const writeRes = await engine.contract.write(
+      CONFIG.CHAIN.id.toString(),
+      tokenContractAddress,
+      backendWallet,
+      {
+        functionName: "setContractURI",
+        args: ["https://abi-test.com"],
+        abi: [
+          {
+            inputs: [
+              {
+                name: "uri",
+                type: "string",
+              },
+            ],
+            name: "setContractURI",
+            stateMutability: "nonpayable",
+            type: "function",
+          },
+        ],
+      },
+    );
+
+    expect(writeRes.result.queueId).toBeDefined();
+
+    const writeTransactionStatus = await pollTransactionStatus(
+      engine,
+      writeRes.result.queueId,
+      true,
+    );
+
+    expect(writeTransactionStatus.minedAt).toBeDefined();
+  });
+
+  test("Should throw error if function name is not found", async () => {
     try {
       await engine.contract.write(
         CONFIG.CHAIN.id.toString(),
@@ -135,8 +174,8 @@ describe("Write Tests", () => {
           args: [""],
         },
       );
-    } catch (e: any) {
-      expect(e.message).toBe(
+    } catch (e) {
+      expect((e as ApiError).body?.error?.message).toBe(
         `could not find function with name "nonExistentFunction" in abi`,
       );
     }

diff --git a/test/e2e/utils/engine.ts b/test/e2e/utils/engine.ts
@@ -16,8 +16,8 @@ export const createChain = async (engine: Engine) => {
   const chains = await engine.configuration.getChainsConfiguration();
 
   if (chains.result) {
-    const parsedChains = JSON.parse(chains.result);
-    if (parsedChains.find((chain: any) => chain.chainId === CONFIG.CHAIN.id)) {
+    const parsedChains = chains.result;
+    if (parsedChains.find((chain) => chain.chainId === CONFIG.CHAIN.id)) {
       console.log("Anvil chain already exists in engine");
       return;
     }