feat: send useful error messages in batcher responses to the client (#635)

Co-authored-by: NicolasRampoldi <[email protected]>
Co-authored-by: Mariano A. Nicolini <[email protected]>
Co-authored-by: Tatu <[email protected]>

Author: 4 people

batcher/aligned-batcher/src/lib.rs

@@ -21,7 +21,6 @@ use lambdaworks_crypto::merkle_tree::merkle::MerkleTree;
 use log::{debug, error, info, warn};
 use tokio::net::{TcpListener, TcpStream};
 use tokio::sync::{Mutex, RwLock};
-use tokio_tungstenite::tungstenite::error::ProtocolError;
 use tokio_tungstenite::tungstenite::{Error, Message};
 use tokio_tungstenite::WebSocketStream;
 use types::batch_queue::BatchQueue;
@@ -176,9 +175,6 @@ impl Batcher {
             .try_for_each(|msg| self.clone().handle_message(msg, outgoing.clone()))
             .await
         {
-            Err(Error::Protocol(ProtocolError::ResetWithoutClosingHandshake)) => {
-                info!("Client {} reset connection", &addr)
-            }
             Err(e) => error!("Unexpected error: {}", e),
             Ok(_) => info!("{} disconnected", &addr),
         }
@@ -189,7 +185,7 @@ impl Batcher {
         self: Arc<Self>,
         message: Message,
         ws_conn_sink: Arc<RwLock<SplitSink<WebSocketStream<TcpStream>, Message>>>,
-    ) -> Result<(), tokio_tungstenite::tungstenite::Error> {
+    ) -> Result<(), Error> {
         // Deserialize verification data from message
         let client_msg: ClientMessage =
             serde_json::from_str(message.to_text().expect("Message is not text"))
@@ -216,34 +212,35 @@ impl Batcher {
 
             if user_balance == U256::from(0) {
                 error!("Insufficient funds for address {:?}", addr);
-                return Err(tokio_tungstenite::tungstenite::Error::Protocol(
-                    ProtocolError::HandshakeIncomplete,
-                ));
+                send_error_message(ws_conn_sink.clone(), ResponseMessage::InsufficientBalanceError(addr))
+                    .await;
+                return Ok(()); // Send error message to the client and return
             }
 
             addr
         } else {
             error!("Signature verification error");
-            return Err(tokio_tungstenite::tungstenite::Error::Protocol(
-                ProtocolError::HandshakeIncomplete,
-            ));
+            send_error_message(ws_conn_sink.clone(), ResponseMessage::SignatureVerificationError())
+                .await;
+            return Ok(()); // Send error message to the client and return
         };
 
         let verification_data = client_msg.verification_data;
         if verification_data.proof.len() <= self.max_proof_size {
             // When pre-verification is enabled, batcher will verify proofs for faster feedback with clients
             if self.pre_verification_is_enabled && !zk_utils::verify(&verification_data) {
-                return Err(tokio_tungstenite::tungstenite::Error::Protocol(
-                    ProtocolError::HandshakeIncomplete,
-                ));
+                error!("Invalid proof detected. Verification failed.");
+                send_error_message(ws_conn_sink.clone(), ResponseMessage::VerificationError())
+                    .await;
+                return Ok(()); // Send error message to the client and return
             }
             self.add_to_batch(verification_data, ws_conn_sink.clone(), submitter_addr)
                 .await;
         } else {
-            // FIXME(marian): Handle this error correctly
-            return Err(tokio_tungstenite::tungstenite::Error::Protocol(
-                ProtocolError::HandshakeIncomplete,
-            ));
+            error!("Proof is too large");
+            send_error_message(ws_conn_sink.clone(), ResponseMessage::ProofTooLargeError())
+                .await;
+            return Ok(()); // Send error message to the client and return
         };
 
         info!("Verification data message handled");
@@ -478,3 +475,19 @@ async fn send_batch_inclusion_data_responses(
         })
         .await;
 }
+
+async fn send_error_message(
+    ws_conn_sink: Arc<RwLock<SplitSink<WebSocketStream<TcpStream>, Message>>>,
+    error_message: ResponseMessage,
+) {
+    let serialized_response =
+        serde_json::to_vec(&error_message).expect("Could not serialize response");
+
+    // Send error message
+    ws_conn_sink
+        .write()
+        .await
+        .send(Message::binary(serialized_response))
+        .await
+        .expect("Failed to send error message");
+}

batcher/aligned-sdk/src/communication/messaging.rs

@@ -1,5 +1,5 @@
 use futures_util::{future, stream::SplitStream, SinkExt, StreamExt, TryStreamExt};
-use log::debug;
+use log::{debug, error};
 use std::sync::Arc;
 use tokio::{net::TcpStream, sync::Mutex};
 
@@ -108,6 +108,21 @@ async fn process_batch_inclusion_data(
                     .to_string(),
             ));
         }
+        Ok(ResponseMessage::Error(e)) => {
+            error!("Batcher responded with error: {}", e);
+        },
+        Ok(ResponseMessage::VerificationError()) => {
+            error!("Invalid proof");
+        },
+        Ok(ResponseMessage::ProofTooLargeError()) => {
+            error!("Proof is too large");
+        },
+        Ok(ResponseMessage::InsufficientBalanceError(address)) => {
+            error!("Insufficient balance for address: {}", address);
+        },
+        Ok(ResponseMessage::SignatureVerificationError()) => {
+            error!("Failed to verify the signature");
+        },
         Err(e) => {
             return Err(SubmitError::SerializationError(e));
         }

batcher/aligned-sdk/src/core/types.rs

@@ -198,6 +198,11 @@ impl AlignedVerificationData {
 pub enum ResponseMessage {
     BatchInclusionData(BatchInclusionData),
     ProtocolVersion(u16),
+    VerificationError(),
+    ProofTooLargeError(),
+    InsufficientBalanceError(Address),
+    SignatureVerificationError(),
+    Error(String),
 }
 
 #[derive(Debug, Clone)]

batcher/aligned/src/main.rs

@@ -291,7 +291,7 @@ async fn main() -> Result<(), AlignedError> {
 
                 if unique_batch_merkle_roots.len() > 1 {
                     info!("Proofs submitted to aligned. See the batches in the explorer:");
-                } else {
+                } else if unique_batch_merkle_roots.len() == 1 {
                     info!("Proofs submitted to aligned. See the batch in the explorer:");
                 }