Add tests to decode error results

.github/workflows/clients.yml

@@ -26,7 +26,15 @@ jobs:
     - uses: ./.github/actions/setup-contracts-clients
     - run: yarn build
 
-  test:
+  test-unit:
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v3
+    - uses: ./.github/actions/setup-contracts-clients
+    - run: yarn test
+
+  test-integration:
     runs-on: ubuntu-latest
 
     steps:
@@ -35,9 +43,6 @@ jobs:
     - uses: denoland/setup-deno@v1
       with:
         deno-version: ${{ env.DENO_VERSION }}
-
-    # - name: unit tests
-    - run: yarn test
 
     # - name: run Hardhat node and deploy contracts
     - uses: ./.github/actions/local-contract-deploy

contracts/.eslintrc.js

@@ -37,6 +37,8 @@ module.exports = {
     ],
     // TODO (merge-ok) Remove and fix lint error
     "node/no-unpublished-import": ["warn"],
+    // https://github.com/typescript-eslint/typescript-eslint/blob/main/docs/linting/TROUBLESHOOTING.md#i-get-errors-from-the-no-undef-rule-about-global-variables-not-being-defined-even-though-there-are-no-typescript-errors
+    "no-undef": "off",
   },
   overrides: [
     {

contracts/clients/.gitignore

@@ -1,3 +1,4 @@
 /node_modules
 /dist
 yarn-error.log
+/.nyc_output

contracts/clients/.nycrc

@@ -0,0 +1,5 @@
+{
+  "extends": "@istanbuljs/nyc-config-typescript",
+  "all": true,
+  "include": ["src/**/*.ts"]
+}

contracts/clients/package.json

@@ -15,7 +15,7 @@
   "scripts": {
     "build": "rm -rf dist && mkdir dist && cp -rH typechain-types dist/typechain-types && find ./dist/typechain-types -type f \\! -name '*.d.ts' -name '*.ts' -delete && tsc",
     "watch": "tsc -w",
-    "test": "mocha --require ts-node/register --require source-map-support/register --require ./test/init.ts **/*.test.ts",
+    "test": "nyc --reporter=text --reporter=html mocha --require ts-node/register --require source-map-support/register --require ./test/init.ts --recursive **/*.test.ts",
     "premerge": "yarn test",
     "publish-experimental": "node scripts/showVersion.js >.version && npm version $(node scripts/showBaseVersion.js)-$(git rev-parse HEAD | head -c7) --allow-same-version && npm publish --tag experimental && npm version $(cat .version) && rm .version",
     "publish-experimental-dry-run": "node scripts/showVersion.js >.version && npm version $(node scripts/showBaseVersion.js)-$(git rev-parse HEAD | head -c7) --allow-same-version && npm publish --tag experimental --dry-run && npm version $(cat .version) && rm .version"
@@ -26,12 +26,14 @@
     "node-fetch": "2.6.7"
   },
   "devDependencies": {
+    "@istanbuljs/nyc-config-typescript": "^1.0.2",
     "@types/chai": "^4.3.4",
     "@types/chai-as-promised": "^7.1.5",
     "@types/mocha": "^10.0.1",
     "chai": "^4.3.7",
     "chai-as-promised": "^7.1.1",
     "mocha": "^10.2.0",
+    "nyc": "^15.1.0",
     "source-map-support": "^0.5.21",
     "ts-node": "^10.9.1",
     "typescript": "^4.9.4"

contracts/clients/src/OperationResults.ts

@@ -2,7 +2,7 @@ import { BigNumber, ContractReceipt, utils } from "ethers";
 import assert from "./helpers/assert";
 import { ActionData } from "./signer";
 
-const errorSelectors = {
+export const errorSelectors = {
   Error: calculateAndCheckSelector("Error(string)", "0x08c379a0"),
 
   Panic: calculateAndCheckSelector("Panic(uint256)", "0x4e487b71"),
@@ -107,19 +107,20 @@ const getError = (
   return decodeError(errorData);
 };
 
+/**
+ * Gets the results of operations (and actions) run through VerificationGateway.processBundle.
+ * Decodes unsuccessful operations into an error message and the index of the action that failed.
+ *
+ * @param transactionReceipt Transaction receipt from a VerificationGateway.processBundle transaction
+ * @returns An array of decoded operation results
+ */
 export const getOperationResults = (
   txnReceipt: ContractReceipt,
 ): OperationResult[] => {
-  if (!txnReceipt.events || !txnReceipt.events.length) {
-    throw new Error(
-      `no events found in transaction ${txnReceipt.transactionHash}`,
-    );
-  }
-
-  const walletOpProcessedEvents = txnReceipt.events.filter(
+  const walletOpProcessedEvents = txnReceipt.events?.filter(
     (e) => e.event === "WalletOperationProcessed",
   );
-  if (!walletOpProcessedEvents.length) {
+  if (!walletOpProcessedEvents?.length) {
     throw new Error(
       `no WalletOperationProcessed events found in transaction ${txnReceipt.transactionHash}`,
     );

contracts/clients/test/OperationResults.test.ts

@@ -0,0 +1,226 @@
+import { expect } from "chai";
+import { BigNumber, ContractReceipt, utils } from "ethers";
+import {
+  decodeError,
+  getOperationResults,
+  errorSelectors,
+} from "../src/OperationResults";
+
+const encodeErrorResultFromEncodedMessage = (
+  actionIndex: number,
+  actionErrorSelector: string,
+  encodedMessage: string,
+): string => {
+  const actionErrorData = `${actionErrorSelector}${encodedMessage.slice(2)}`; // remove 0x
+  const encodedErrorMessage = utils.defaultAbiCoder.encode(
+    ["uint256", "bytes"],
+    [actionIndex, actionErrorData],
+  );
+  return `${errorSelectors.ActionError}${encodedErrorMessage.slice(2)}`; // remove 0x
+};
+
+const encodeErrorResult = (
+  actionIndex: number,
+  actionErrorSelector: string,
+  message: string,
+): string => {
+  const encodedMessage = utils.defaultAbiCoder.encode(["string"], [message]);
+  return encodeErrorResultFromEncodedMessage(
+    actionIndex,
+    actionErrorSelector,
+    encodedMessage,
+  );
+};
+
+describe("OperationResults", () => {
+  describe("decodeError", () => {
+    it("fails if error data does not start with ActionError selector", () => {
+      const errorData = "does not compute";
+      expect(() => decodeError(errorData)).to.throw(
+        `errorResult does not begin with ActionError selector (${errorSelectors.ActionError}): ${errorData}`,
+      );
+    });
+
+    it("parses error data", () => {
+      const actionIndex = 43770;
+      const msg = "hello";
+      const errorData = encodeErrorResult(
+        actionIndex,
+        errorSelectors.Error,
+        msg,
+      );
+
+      const { actionIndex: actionIdxBn, message } = decodeError(errorData);
+      expect(actionIdxBn?.toNumber()).to.eql(actionIndex);
+      expect(message).to.eql(msg);
+    });
+
+    it("parses panic error data", () => {
+      const actionIndex = 1337;
+      const panicCode = BigNumber.from(42);
+      const panicActionErrorData = utils.defaultAbiCoder.encode(
+        ["uint256"],
+        [panicCode],
+      );
+      const errorData = encodeErrorResultFromEncodedMessage(
+        actionIndex,
+        errorSelectors.Panic,
+        panicActionErrorData,
+      );
+
+      const { actionIndex: actionIdxBn, message } = decodeError(errorData);
+      expect(actionIdxBn?.toNumber()).to.eql(actionIndex);
+      expect(message).to.eql(
+        `Panic: ${panicCode.toHexString()} (See Panic(uint256) in the solidity docs: https://docs.soliditylang.org/_/downloads/en/latest/pdf/)`,
+      );
+    });
+
+    it("handles unexpected error data", () => {
+      const actionIndex = 707;
+      const msg = "lol";
+      const errorData = encodeErrorResult(
+        actionIndex,
+        errorSelectors.ActionError,
+        msg,
+      );
+      const encodedMessage = utils.defaultAbiCoder.encode(["string"], [msg]);
+      const actionErrorData = `${
+        errorSelectors.ActionError
+      }${encodedMessage.slice(2)}`; // remove 0x
+
+      const { actionIndex: actionIdxBn, message } = decodeError(errorData);
+      expect(actionIdxBn?.toNumber()).to.eql(actionIndex);
+      expect(message).to.eql(
+        `Unexpected action error data: ${actionErrorData}`,
+      );
+    });
+
+    it("handles exceptions when parsing error data", () => {
+      const encodedErrorMessage = utils.defaultAbiCoder.encode(
+        ["uint256"],
+        [0],
+      );
+      const errorData = `${
+        errorSelectors.ActionError
+      }${encodedErrorMessage.slice(2)}`; // Remove 0x
+
+      expect(decodeError(errorData)).to.deep.equal({
+        actionIndex: undefined,
+        message: `Unexpected error data: ${errorData}`,
+      });
+    });
+  });
+
+  describe("getOperationResults", () => {
+    it("fails if no events are in transaction", () => {
+      const txnReceipt = {
+        transactionHash: "0x111111",
+      } as ContractReceipt;
+
+      expect(() => getOperationResults(txnReceipt)).to.throw(
+        `no WalletOperationProcessed events found in transaction ${txnReceipt.transactionHash}`,
+      );
+    });
+
+    it("fails if no WalletOperationProcessed events are in transaction", () => {
+      const event = { event: "Other" };
+      const txnReceipt = {
+        transactionHash: "0x123456",
+        events: [event],
+      } as ContractReceipt;
+
+      expect(() => getOperationResults(txnReceipt)).to.throw(
+        `no WalletOperationProcessed events found in transaction ${txnReceipt.transactionHash}`,
+      );
+    });
+
+    it("fails when WalletOperationProcessed event is missing args", () => {
+      const event = { event: "WalletOperationProcessed" };
+      const txnReceipt = {
+        events: [event],
+      } as ContractReceipt;
+
+      expect(() => getOperationResults(txnReceipt)).to.throw(
+        "WalletOperationProcessed event missing args",
+      );
+    });
+
+    it("decodes WalletOperationProcessed events", () => {
+      const otherEvent = { event: "Other" };
+
+      const errorActionIndex = 0;
+      const errorMessage = "halt and catch fire";
+      const errorResult = encodeErrorResult(
+        errorActionIndex,
+        errorSelectors.Error,
+        errorMessage,
+      );
+
+      const failedEvent = {
+        event: "WalletOperationProcessed",
+        args: {
+          wallet: "0x01",
+          nonce: BigNumber.from(0),
+          actions: [
+            {
+              ethValue: BigNumber.from(0),
+              contractAddress: "0xaabbcc",
+              encodedFunction: "0xddeeff",
+            },
+          ],
+          success: false,
+          results: [errorResult],
+        },
+      };
+
+      const successfulEvent = {
+        event: "WalletOperationProcessed",
+        args: {
+          wallet: "0x02",
+          nonce: BigNumber.from(1),
+          actions: [
+            {
+              ethValue: BigNumber.from(0),
+              contractAddress: "0xabcabc",
+              encodedFunction: "0xdefdef",
+            },
+            {
+              ethValue: BigNumber.from(42),
+              contractAddress: "0x123123",
+              encodedFunction: "0x456456",
+            },
+          ],
+          success: true,
+          results: ["0x2A", "0x539"],
+        },
+      };
+
+      const txnReceipt = {
+        events: [otherEvent, failedEvent, successfulEvent],
+      } as ContractReceipt;
+
+      const opResults = getOperationResults(txnReceipt);
+      expect(opResults).to.have.lengthOf(2);
+
+      const [r1, r2] = opResults;
+      expect(r1.walletAddress).to.eql(failedEvent.args.wallet);
+      expect(r1.nonce.toNumber()).to.eql(
+        BigNumber.from(failedEvent.args.nonce).toNumber(),
+      );
+      expect(r1.actions).to.deep.equal(failedEvent.args.actions);
+      expect(r1.success).to.eql(failedEvent.args.success);
+      expect(r1.results).to.deep.equal(failedEvent.args.results);
+      expect(r1.error?.actionIndex?.toNumber()).to.eql(errorActionIndex);
+      expect(r1.error?.message).to.eql(errorMessage);
+
+      expect(r2.walletAddress).to.eql(successfulEvent.args.wallet);
+      expect(r2.nonce.toNumber()).to.eql(
+        BigNumber.from(successfulEvent.args.nonce).toNumber(),
+      );
+      expect(r2.actions).to.deep.equal(successfulEvent.args.actions);
+      expect(r2.success).to.eql(successfulEvent.args.success);
+      expect(r2.results).to.deep.equal(successfulEvent.args.results);
+      expect(r2.error).to.eql(undefined);
+    });
+  });
+});