diff --git a/batcher/aligned-batcher/src/lib.rs b/batcher/aligned-batcher/src/lib.rs index 7fa09d539..fbfc112cc 100644 --- a/batcher/aligned-batcher/src/lib.rs +++ b/batcher/aligned-batcher/src/lib.rs @@ -664,7 +664,10 @@ impl Batcher { if self.user_balance_is_unlocked(&addr).await { send_message( ws_conn_sink.clone(), - SubmitProofResponseMessage::InsufficientBalance(addr), + SubmitProofResponseMessage::InsufficientBalance( + addr, + nonced_verification_data.nonce, + ), ) .await; self.metrics.user_error(&["insufficient_balance", ""]); @@ -756,7 +759,10 @@ impl Batcher { std::mem::drop(batch_state_lock); send_message( ws_conn_sink.clone(), - SubmitProofResponseMessage::InsufficientBalance(addr), + SubmitProofResponseMessage::InsufficientBalance( + addr, + nonced_verification_data.nonce, + ), ) .await; self.metrics.user_error(&["insufficient_balance", ""]); @@ -1731,7 +1737,10 @@ impl Batcher { error!("Could not get balance for non-paying address {replacement_addr:?}"); send_message( ws_sink.clone(), - SubmitProofResponseMessage::InsufficientBalance(replacement_addr), + SubmitProofResponseMessage::InsufficientBalance( + replacement_addr, + client_msg.verification_data.nonce, + ), ) .await; return Ok(()); @@ -1741,7 +1750,10 @@ impl Batcher { error!("Insufficient funds for non-paying address {replacement_addr:?}"); send_message( ws_sink.clone(), - SubmitProofResponseMessage::InsufficientBalance(replacement_addr), + SubmitProofResponseMessage::InsufficientBalance( + replacement_addr, + client_msg.verification_data.nonce, + ), ) .await; return Ok(()); diff --git a/batcher/aligned-sdk/src/communication/messaging.rs b/batcher/aligned-sdk/src/communication/messaging.rs index 8677ff1b4..8fa08631e 100644 --- a/batcher/aligned-sdk/src/communication/messaging.rs +++ b/batcher/aligned-sdk/src/communication/messaging.rs @@ -80,7 +80,7 @@ pub async fn send_messages( sent_verification_data.push(Ok(verification_data)); } - info!("All proofs sent"); + info!("All proofs sent, wait until they are included in a batch"); // This vector is reversed so that while responses are received, removing from the end is cheaper. let sent_verification_data_rev: Vec> = sent_verification_data.into_iter().rev().collect(); @@ -95,11 +95,12 @@ pub async fn send_messages( pub async fn receive( response_stream: Arc>, mut sent_verification_data_rev: Vec>, + first_nonce: U256, ) -> Vec> { // Responses are filtered to only admit binary or close messages. let mut response_stream = response_stream.lock().await; let mut aligned_submitted_data: Vec> = Vec::new(); - let last_proof_nonce = get_biggest_nonce(&sent_verification_data_rev); + let mut last_valid_proof_nonce = get_biggest_nonce(&sent_verification_data_rev); // read from WS while let Some(Ok(msg)) = response_stream.next().await { @@ -123,7 +124,28 @@ pub async fn receive( let batch_inclusion_data_message = match handle_batcher_response(msg).await { Ok(data) => data, Err(e) => { - warn!("Error while handling batcher response: {:?}", e); + warn!("Error detected while handling batcher response: {:?}", e); + // When submitting multiple proofs, an InsufficientBalance error may occur, when the required balance of a user would exceed his balance in the BatcherPaymentService.sol + // This leads to a scenario where some of the submitted proofs are accepted and others rejected, with + // the SubmitError::InsufficientBalance(error_nonce) thrown. To ensure the user is notified that some of their proofs were rejected, + // we return upon erroring the nonce of the proof that has errored and set the new `last_valid_proof_nonce` value accordingly. + // This ensures the client messaging protocol continues receiving responses until all messages are received. + if let SubmitError::InsufficientBalance(_, error_nonce) = e { + aligned_submitted_data.push(Err(e)); + + if error_nonce == first_nonce { + // no proofs where accepted by the batcher + // this also covers error_nonce==0, which as uint risks an underflow when substracting 1 + break; // stop listening responses + } + + if last_valid_proof_nonce > (error_nonce - 1) { + // last_valid_proof_nonce needs to be updated, since there is a new error_nonce + last_valid_proof_nonce = error_nonce - 1; + } + + continue; // continue listening responses + } aligned_submitted_data.push(Err(e)); break; } @@ -159,7 +181,7 @@ pub async fn receive( aligned_submitted_data.push(Ok(aligned_verification_data)); debug!("Message response handled successfully"); - if batch_inclusion_data_message.user_nonce == last_proof_nonce { + if batch_inclusion_data_message.user_nonce == last_valid_proof_nonce { break; } } @@ -190,9 +212,13 @@ async fn handle_batcher_response(msg: Message) -> Result { - error!("Batcher responded with insufficient balance. Funds have not been spent for submittions which had insufficient balance."); - Err(SubmitError::InsufficientBalance(addr)) + Ok(SubmitProofResponseMessage::InsufficientBalance(addr, error_nonce)) => { + // If we receive an invalid balance we should grab the error_nonce. + warn!( + "Batcher responded with insufficient balance to pay for proof of nonce: {}. If you sent more proofs, your other proofs may have been accepted. Funds have not been spent for submittions which had insufficient balance.", + error_nonce + ); + Err(SubmitError::InsufficientBalance(addr, error_nonce)) } Ok(SubmitProofResponseMessage::InvalidChainId) => { error!("Batcher responded with invalid chain id. Funds have not been spent."); diff --git a/batcher/aligned-sdk/src/core/errors.rs b/batcher/aligned-sdk/src/core/errors.rs index 31fc3ee54..b4ba221da 100644 --- a/batcher/aligned-sdk/src/core/errors.rs +++ b/batcher/aligned-sdk/src/core/errors.rs @@ -2,7 +2,7 @@ use core::fmt; use ethers::providers::ProviderError; use ethers::signers::WalletError; use ethers::types::transaction::eip712::Eip712Error; -use ethers::types::{SignatureError, H160}; +use ethers::types::{SignatureError, H160, U256}; use serde::{Deserialize, Serialize}; use std::io; use std::path::PathBuf; @@ -90,7 +90,7 @@ pub enum SubmitError { InvalidProof(ProofInvalidReason), ProofTooLarge, InvalidReplacementMessage, - InsufficientBalance(H160), + InsufficientBalance(H160, U256), InvalidPaymentServiceAddress(H160, H160), BatchSubmissionFailed(String), AddToBatchError, @@ -195,8 +195,12 @@ impl fmt::Display for SubmitError { SubmitError::InvalidProof(reason) => write!(f, "Invalid proof {}", reason), SubmitError::ProofTooLarge => write!(f, "Proof too Large"), SubmitError::InvalidReplacementMessage => write!(f, "Invalid replacement message"), - SubmitError::InsufficientBalance(addr) => { - write!(f, "Insufficient balance, address: {}", addr) + SubmitError::InsufficientBalance(addr, error_nonce) => { + write!( + f, + "Insufficient balance, address: {} error_nonce: {}", + addr, error_nonce + ) } SubmitError::InvalidPaymentServiceAddress(received_addr, expected_addr) => { write!( diff --git a/batcher/aligned-sdk/src/core/types.rs b/batcher/aligned-sdk/src/core/types.rs index 320cef6c1..3ef6aeb8e 100644 --- a/batcher/aligned-sdk/src/core/types.rs +++ b/batcher/aligned-sdk/src/core/types.rs @@ -389,7 +389,7 @@ pub enum SubmitProofResponseMessage { InvalidSignature, ProofTooLarge, InvalidMaxFee, - InsufficientBalance(Address), + InsufficientBalance(Address, U256), InvalidChainId, InvalidReplacementMessage, AddToBatchError, diff --git a/batcher/aligned-sdk/src/sdk.rs b/batcher/aligned-sdk/src/sdk.rs index b2623d03d..9c20714ab 100644 --- a/batcher/aligned-sdk/src/sdk.rs +++ b/batcher/aligned-sdk/src/sdk.rs @@ -294,12 +294,12 @@ async fn _submit_multiple( nonce, ) .await; - receive(response_stream, sent_verification_data_rev).await + receive(response_stream, sent_verification_data_rev, nonce).await } .await; // Close connection - info!("Closing WS connection"); + info!("All responses received, closing WS connection"); if let Err(e) = ws_write_clone.lock().await.close().await { return vec![Err(errors::SubmitError::GenericError(e.to_string()))]; } diff --git a/batcher/aligned/src/main.rs b/batcher/aligned/src/main.rs index 62e6e0490..26aec0914 100644 --- a/batcher/aligned/src/main.rs +++ b/batcher/aligned/src/main.rs @@ -456,8 +456,6 @@ async fn main() -> Result<(), AlignedError> { } }; - warn!("Nonce: {nonce}"); - let verification_data = verification_data_from_args(&submit_args)?; let verification_data_arr = vec![verification_data; repetitions]; @@ -490,15 +488,22 @@ async fn main() -> Result<(), AlignedError> { .insert(aligned_verification_data.batch_merkle_root); } Err(e) => { - warn!("Error while submitting proof: {:?}", e); - handle_submit_err(e).await; - return Ok(()); + warn!("Error detected while submitting proof: {:?}", e); + handle_submit_err(&e).await; + // In the case of an InsufficientBalance error we record and continue processing the entire msg queue. + // This covers the case of a `submit_multiple` in which some submissions succeed but others fail because of a cumulative `insufficient balance`. + if let SubmitError::InsufficientBalance(_, _) = e { + continue; + } else { + return Ok(()); + } } }; } match unique_batch_merkle_roots.len() { 1 => info!("Proofs submitted to aligned. See the batch in the explorer:"), + 0 => (), // No verification data, we do not log the msg. This happens when insufficient balance for first nonce _ => info!("Proofs submitted to aligned. See the batches in the explorer:"), } @@ -705,7 +710,7 @@ fn verification_data_from_args(args: &SubmitArgs) -> Result { error!("Invalid nonce. try again"); @@ -714,10 +719,10 @@ async fn handle_submit_err(err: SubmitError) { error!("Batch was reset. try resubmitting the proof"); } SubmitError::InvalidProof(reason) => error!("Submitted proof is invalid: {}", reason), - SubmitError::InsufficientBalance(sender_address) => { - error!( - "Insufficient balance to pay for the transaction, address: {}", - sender_address + SubmitError::InsufficientBalance(sender_address, error_nonce) => { + warn!( + "Insufficient balance to pay for the proof verification, address: {}, for proof_nonce: {}.", + sender_address, error_nonce ) } _ => {}