fix: archive debug bundle on invalid vote and added unit tests (#112)

KYVENetwork · Dec 29, 2023 · 800d4a1 · 800d4a1
1 parent 40180ad
commit 800d4a1
Show file tree

Hide file tree

Showing 5 changed files with 181 additions and 90 deletions.
diff --git a/common/protocol/src/methods/helpers/archiveDebugBundle.ts b/common/protocol/src/methods/helpers/archiveDebugBundle.ts
@@ -25,6 +25,8 @@ export function archiveDebugBundle(
   metadata: object
 ): void {
   try {
+    this.logger.info("Archiving debug bundle");
+
     // if "debug" folder under target path does not exist create it
     if (!existsSync(path.join(this.home, `debug`))) {
       mkdirSync(path.join(this.home, `debug`), { recursive: true });
@@ -89,10 +91,9 @@ export function archiveDebugBundle(
     const zipPath = path.join(
       this.home,
       `debug`,
-      `${voteType}_${Math.floor(Date.now() / 1000)}_${storageId.slice(
-        0,
-        6
-      )}.zip`
+      `${voteType}_${this.pool.id}_${Math.floor(
+        Date.now() / 1000
+      )}_${storageId.slice(0, 6)}.zip`
     );
 
     // save zip file
@@ -101,6 +102,10 @@ export function archiveDebugBundle(
       .pipe(createWriteStream(zipPath))
       .on("finish", () => {
         this.logger.info("Successfully saved debug information");
+      })
+      .on("error", (err) => {
+        this.logger.error("Failed to save debug information");
+        this.logger.error(standardizeError(err));
       });
   } catch (err) {
     this.logger.error("Failed to save debug information");

diff --git a/common/protocol/src/methods/validate/validateBundleProposal.ts b/common/protocol/src/methods/validate/validateBundleProposal.ts
@@ -237,14 +237,50 @@ export async function validateBundleProposal(
           this.logger.debug(
             `this.runtime.validateDataItem($THIS, $PROPOSED_DATA_ITEM, $VALIDATION_DATA_ITEM)`
           );
-          const vote = await this.runtime.validateDataItem(
-            this,
-            proposedBundle[i],
-            validationBundle[i]
-          );
+          const vote = await this.runtime
+            .validateDataItem(this, proposedBundle[i], validationBundle[i])
+            .catch((err) => {
+              this.logger.error(
+                `Unexpected error validating data item with runtime. Voting abstain ...`
+              );
+              this.logger.error(standardizeError(err));
+
+              this.archiveDebugBundle(
+                VoteType.VOTE_TYPE_ABSTAIN,
+                proposedBundle,
+                validationBundle,
+                {
+                  reason: "unexpected error validating data item with runtime",
+                  err: err,
+                  proposed: proposedBundle[i],
+                  validation: validationBundle[i],
+                }
+              );
+
+              return null;
+            });
+
+          // vote abstain if vote is null
+          if (vote === null) {
+            const success = await this.voteBundleProposal(
+              this.pool.bundle_proposal!.storage_id,
+              VoteType.VOTE_TYPE_ABSTAIN
+            );
+            return success;
+          }
 
           // vote abstain if data item validation returned abstain
           if (vote === VoteType.VOTE_TYPE_ABSTAIN) {
+            // archive local abstain bundle for debug purposes
+            this.archiveDebugBundle(
+              VoteType.VOTE_TYPE_ABSTAIN,
+              proposedBundle,
+              validationBundle,
+              {
+                reason: "custom runtime validation returned abstain",
+              }
+            );
+
             const success = await this.voteBundleProposal(
               this.pool.bundle_proposal!.storage_id,
               VoteType.VOTE_TYPE_ABSTAIN
@@ -275,20 +311,32 @@ export async function validateBundleProposal(
         `Finished validating bundle by custom runtime validation. Result = ${valid}`
       );
 
+      let bundleSummary;
+
       // only continue with bundle summary validation if proposed bundle is valid
       if (valid) {
         // vote invalid if bundle summary does not match with proposed summary
         this.logger.debug(`Validating bundle proposal by bundle summary`);
         this.logger.debug(`this.runtime.summarizeDataBundle($PROPOSED_BUNDLE)`);
 
-        const bundleSummary = await this.runtime
+        bundleSummary = await this.runtime
           .summarizeDataBundle(this, proposedBundle)
           .catch((err) => {
             this.logger.error(
               `Unexpected error summarizing bundle with runtime. Voting abstain ...`
             );
             this.logger.error(standardizeError(err));
 
+            this.archiveDebugBundle(
+              VoteType.VOTE_TYPE_ABSTAIN,
+              proposedBundle,
+              validationBundle,
+              {
+                reason: "unexpected error summarizing bundle with runtime",
+                err: err,
+              }
+            );
+
             return null;
           });
 
@@ -312,21 +360,20 @@ export async function validateBundleProposal(
         this.logger.debug(
           `Finished validating bundle by bundle summary. Result = ${valid}`
         );
+      }
 
-        if (!valid) {
-          // archive local invalid bundle for debug purposes
-          this.archiveDebugBundle(
-            VoteType.VOTE_TYPE_INVALID,
-            proposedBundle,
-            validationBundle,
-            {
-              reason:
-                "custom data item validation failed or bundle summary mismatches",
-              proposed: this.pool.bundle_proposal!.bundle_summary,
-              validation: bundleSummary,
-            }
-          );
-        }
+      if (!valid) {
+        // archive local invalid bundle for debug purposes
+        this.archiveDebugBundle(
+          VoteType.VOTE_TYPE_INVALID,
+          proposedBundle,
+          validationBundle,
+          {
+            reason: "custom runtime validation returned invalid",
+            proposed: this.pool.bundle_proposal!.bundle_summary,
+            validation: bundleSummary,
+          }
+        );
       }
 
       // vote with either valid or invalid

diff --git a/common/protocol/test/vote_abstain.test.ts b/common/protocol/test/vote_abstain.test.ts
@@ -106,6 +106,8 @@ describe("vote abstain tests", () => {
 
     v["waitForNextBundleProposal"] = jest.fn();
 
+    v["archiveDebugBundle"] = jest.fn();
+
     v["continueRound"] = jest
       .fn()
       .mockReturnValueOnce(true)
@@ -277,10 +279,11 @@ describe("vote abstain tests", () => {
     // ASSERT NODEJS INTERFACES
     // ========================
 
+    // assert that debug bundle was saved
+    expect(v["archiveDebugBundle"]).toHaveBeenCalledTimes(0);
+
     // assert that only one round ran
     expect(v["waitForNextBundleProposal"]).toHaveBeenCalledTimes(1);
-
-    // TODO: assert timeouts
   });
 
   test("vote abstain because local bundle could only be loaded partially", async () => {
@@ -447,10 +450,11 @@ describe("vote abstain tests", () => {
     // ASSERT NODEJS INTERFACES
     // ========================
 
+    // assert that debug bundle was saved
+    expect(v["archiveDebugBundle"]).toHaveBeenCalledTimes(0);
+
     // assert that only one round ran
     expect(v["waitForNextBundleProposal"]).toHaveBeenCalledTimes(1);
-
-    // TODO: assert timeouts
   });
 
   test("vote abstain because local bundle could only be loaded partially multiple times", async () => {
@@ -618,10 +622,11 @@ describe("vote abstain tests", () => {
     // ASSERT NODEJS INTERFACES
     // ========================
 
+    // assert that debug bundle was saved
+    expect(v["archiveDebugBundle"]).toHaveBeenCalledTimes(0);
+
     // assert that only one round ran
     expect(v["waitForNextBundleProposal"]).toHaveBeenCalledTimes(1);
-
-    // TODO: assert timeouts
   });
 
   test("vote abstain because bundle from storage provider could not be loaded", async () => {
@@ -794,10 +799,11 @@ describe("vote abstain tests", () => {
     // ASSERT NODEJS INTERFACES
     // ========================
 
+    // assert that debug bundle was saved
+    expect(v["archiveDebugBundle"]).toHaveBeenCalledTimes(0);
+
     // assert that only one round ran
     expect(v["waitForNextBundleProposal"]).toHaveBeenCalledTimes(1);
-
-    // TODO: assert timeouts
   });
 
   test("vote abstain because bundle from storage provider could not be loaded multiple times", async () => {
@@ -977,10 +983,11 @@ describe("vote abstain tests", () => {
     // ASSERT NODEJS INTERFACES
     // ========================
 
+    // assert that debug bundle was saved
+    expect(v["archiveDebugBundle"]).toHaveBeenCalledTimes(0);
+
     // assert that only one round ran
     expect(v["waitForNextBundleProposal"]).toHaveBeenCalledTimes(1);
-
-    // TODO: assert timeouts
   });
 
   test("vote abstain because local and storage provider bundle could not be loaded", async () => {
@@ -1155,10 +1162,11 @@ describe("vote abstain tests", () => {
     // ASSERT NODEJS INTERFACES
     // ========================
 
+    // assert that debug bundle was saved
+    expect(v["archiveDebugBundle"]).toHaveBeenCalledTimes(0);
+
     // assert that only one round ran
     expect(v["waitForNextBundleProposal"]).toHaveBeenCalledTimes(1);
-
-    // TODO: assert timeouts
   });
 
   test("try to vote abstain after validator has already voted abstain", async () => {
@@ -1307,10 +1315,11 @@ describe("vote abstain tests", () => {
     // ASSERT NODEJS INTERFACES
     // ========================
 
+    // assert that debug bundle was saved
+    expect(v["archiveDebugBundle"]).toHaveBeenCalledTimes(0);
+
     // assert that only one round ran
     expect(v["waitForNextBundleProposal"]).toHaveBeenCalledTimes(1);
-
-    // TODO: assert timeouts
   });
 
   test("try to vote abstain after validator has already voted valid", async () => {
@@ -1433,10 +1442,11 @@ describe("vote abstain tests", () => {
     // ASSERT NODEJS INTERFACES
     // ========================
 
+    // assert that debug bundle was saved
+    expect(v["archiveDebugBundle"]).toHaveBeenCalledTimes(0);
+
     // assert that only one round ran
     expect(v["waitForNextBundleProposal"]).toHaveBeenCalledTimes(1);
-
-    // TODO: assert timeouts
   });
 
   test("try to vote abstain after validator has already voted invalid", async () => {
@@ -1560,10 +1570,11 @@ describe("vote abstain tests", () => {
     // ASSERT NODEJS INTERFACES
     // ========================
 
+    // assert that debug bundle was saved
+    expect(v["archiveDebugBundle"]).toHaveBeenCalledTimes(0);
+
     // assert that only one round ran
     expect(v["waitForNextBundleProposal"]).toHaveBeenCalledTimes(1);
-
-    // TODO: assert timeouts
   });
 
   test("try to vote abstain where voteBundleProposal fails", async () => {
@@ -1734,10 +1745,11 @@ describe("vote abstain tests", () => {
     // ASSERT NODEJS INTERFACES
     // ========================
 
+    // assert that debug bundle was saved
+    expect(v["archiveDebugBundle"]).toHaveBeenCalledTimes(0);
+
     // assert that only one round ran
     expect(v["waitForNextBundleProposal"]).toHaveBeenCalledTimes(1);
-
-    // TODO: assert timeouts
   });
 
   test("try to validate bundle but summarizeDataBundle fails", async () => {
@@ -1889,10 +1901,11 @@ describe("vote abstain tests", () => {
     // ASSERT NODEJS INTERFACES
     // ========================
 
+    // assert that debug bundle was saved
+    expect(v["archiveDebugBundle"]).toHaveBeenCalledTimes(1);
+
     // assert that only one round ran
     expect(v["waitForNextBundleProposal"]).toHaveBeenCalledTimes(1);
-
-    // TODO: assert timeouts
   });
 
   test("try to validate bundle but validateDataItem fails", async () => {
@@ -2032,10 +2045,11 @@ describe("vote abstain tests", () => {
     // ASSERT NODEJS INTERFACES
     // ========================
 
+    // assert that debug bundle was saved
+    expect(v["archiveDebugBundle"]).toHaveBeenCalledTimes(1);
+
     // assert that only one round ran
     expect(v["waitForNextBundleProposal"]).toHaveBeenCalledTimes(1);
-
-    // TODO: assert timeouts
   });
 
   test("vote abstain because decompression fails", async () => {
@@ -2170,9 +2184,10 @@ describe("vote abstain tests", () => {
     // ASSERT NODEJS INTERFACES
     // ========================
 
+    // assert that debug bundle was saved
+    expect(v["archiveDebugBundle"]).toHaveBeenCalledTimes(1);
+
     // assert that only one round ran
     expect(v["waitForNextBundleProposal"]).toHaveBeenCalledTimes(1);
-
-    // TODO: assert timeouts
   });
 });