deployment_store: Remove unfail confidence check for non-deterministic errors (#3015)

Author: evaporei

store/postgres/src/deployment_store.rs

@@ -1294,41 +1294,48 @@ impl DeploymentStore {
                 // Deployment head (current_ptr) advanced more than the error.
                 // That means it's healthy, and the non-deterministic error got
                 // solved (didn't happen on another try).
+                //
+                // This should be the scenario where the unfail happens, however
+                // for now we unfail in all cases that non-deterministic errors
+                // were found and the deployment head advanced.
                 (Bound::Included(error_block_number), _)
                     if current_ptr.number >= error_block_number =>
                     {
-                        info!(
-                            self.logger,
-                            "Unfailing the deployment status";
-                            "subgraph_id" => deployment_id,
-                        );
-
-                        // Unfail the deployment.
-                        deployment::update_deployment_status(
-                            conn,
-                            deployment_id,
-                            deployment::SubgraphHealth::Healthy,
-                            None,
-                        )?;
-
-                        // Delete the fatal error.
-                        deployment::delete_error(conn, &subgraph_error.id)
                     }
-                // NOOP, the deployment head is still before where non-deterministic error happened.
+                // The deployment head is still before where non-deterministic error happened.
+                //
+                // Technically we shouldn't unfail the subgraph and delete the error
+                // until it's head actually passed the error block range. But for
+                // now we'll only log this and keep the old behavior.
                 block_range => {
                     info!(
                         self.logger,
-                        "Subgraph error is still ahead of deployment head, nothing to unfail";
+                        "Subgraph error is still ahead of deployment head";
                         "subgraph_id" => deployment_id,
                         "block_number" => format!("{}", current_ptr.number),
                         "block_hash" => format!("{}", current_ptr.hash),
                         "error_block_range" => format!("{:?}", block_range),
                         "error_block_hash" => subgraph_error.block_hash.as_ref().map(|hash| format!("0x{}", hex::encode(hash))),
                     );
-
-                    Ok(())
                 }
-            }
+            };
+
+            info!(
+                self.logger,
+                "Unfailing the deployment status";
+                "subgraph_id" => deployment_id,
+            );
+
+            // Unfail the deployment.
+            deployment::update_deployment_status(
+                conn,
+                deployment_id,
+                deployment::SubgraphHealth::Healthy,
+                None,
+            )?;
+
+            // Delete the fatal error.
+            deployment::delete_error(conn, &subgraph_error.id)
         })
     }
 

store/postgres/tests/subgraph.rs

@@ -966,14 +966,15 @@ fn fail_unfail_non_deterministic_error_noop() {
         // Fail the subgraph with a non-deterministic error, but with an advanced block.
         writable.fail_subgraph(error).await.unwrap();
 
-        // Since the block range of the block won't match the deployment head, this will be NOOP.
+        // Since the block range of the block won't match the deployment head, this would be NOOP,
+        // but we're skipping the confidence check for now.
         writable.unfail_non_deterministic_error(&BLOCKS[1]).unwrap();
 
-        // State continues the same besides a new error added to the database.
-        assert_eq!(count(), 2);
+        // Unfail ocurrs as expected.
+        assert_eq!(count(), 1);
         let vi = get_version_info(&store, NAME);
         assert_eq!(&*NAME, vi.deployment_id.as_str());
-        assert_eq!(true, vi.failed);
+        assert_eq!(false, vi.failed);
         assert_eq!(Some(1), vi.latest_ethereum_block_number);
 
         test_store::remove_subgraphs();