Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sdk): Partially sent batch should display results of processing #1675

Open
wants to merge 19 commits into
base: staging
Choose a base branch
from
Open
20 changes: 16 additions & 4 deletions batcher/aligned-batcher/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,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,
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
),
)
.await;
self.metrics.user_error(&["insufficient_balance", ""]);
Expand Down Expand Up @@ -749,7 +752,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", ""]);
Expand Down Expand Up @@ -1684,7 +1690,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(());
Expand All @@ -1694,7 +1703,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(());
Expand Down
38 changes: 35 additions & 3 deletions batcher/aligned-sdk/src/communication/messaging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,14 @@ pub async fn send_messages(
pub async fn receive(
response_stream: Arc<Mutex<ResponseStream>>,
mut sent_verification_data_rev: Vec<Result<NoncedVerificationData, SubmitError>>,
first_nonce: U256,
) -> Vec<Result<AlignedVerificationData, SubmitError>> {
// Responses are filtered to only admit binary or close messages.
let mut response_stream = response_stream.lock().await;
let mut aligned_submitted_data: Vec<Result<AlignedVerificationData, SubmitError>> = Vec::new();
let last_proof_nonce = get_biggest_nonce(&sent_verification_data_rev);
let last_sent_proof_nonce = get_biggest_nonce(&sent_verification_data_rev);
let mut last_proof_nonce = last_sent_proof_nonce;
info!("last_sent_proof_nonce: {}", last_sent_proof_nonce);
PatStiles marked this conversation as resolved.
Show resolved Hide resolved

// read from WS
while let Some(Ok(msg)) = response_stream.next().await {
Expand All @@ -124,6 +127,25 @@ pub async fn receive(
Ok(data) => data,
Err(e) => {
warn!("Error while handling batcher response: {:?}", e);
// When submitting multiple batches a InsufficientBalance error may occur when the `max_balance` of a user within the
// BatcherPaymentService.sol is exceeded. This leads to a scenario where some proofs are verified 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 (is returned earlier) and set that as the new `last_proof_nonce`.
// This ensures the client messaging protocol continues receivng verification and error responses until all messages are received.
uri-99 marked this conversation as resolved.
Show resolved Hide resolved
if let SubmitError::InsufficientBalance(_, error_nonce) = e {
aligned_submitted_data.push(Err(e));

let last_valid_nonce = error_nonce - 1;
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
if last_valid_nonce < last_proof_nonce {
last_proof_nonce = last_valid_nonce;
}

if last_proof_nonce < first_nonce {
break;
}
PatStiles marked this conversation as resolved.
Show resolved Hide resolved

continue;
}
aligned_submitted_data.push(Err(e));
break;
}
Expand Down Expand Up @@ -159,6 +181,12 @@ pub async fn receive(
aligned_submitted_data.push(Ok(aligned_verification_data));
debug!("Message response handled successfully");

info!("last_proof_nonce: {}", last_proof_nonce);
info!(
"batch_inclusion_data_message.user_nonce: {}",
batch_inclusion_data_message.user_nonce
);
PatStiles marked this conversation as resolved.
Show resolved Hide resolved

if batch_inclusion_data_message.user_nonce == last_proof_nonce {
break;
}
Expand Down Expand Up @@ -190,9 +218,13 @@ async fn handle_batcher_response(msg: Message) -> Result<BatchInclusionData, Sub
error!("Batcher responded with invalid max fee");
Err(SubmitError::InvalidMaxFee)
}
Ok(SubmitProofResponseMessage::InsufficientBalance(addr)) => {
Ok(SubmitProofResponseMessage::InsufficientBalance(addr, last_sent_valid_nonce)) => {
// If we receive an invalid balance we should grab the last_sent_valid_nonce.
error!("Batcher responded with insufficient balance");
Err(SubmitError::InsufficientBalance(addr))
Err(SubmitError::InsufficientBalance(
addr,
last_sent_valid_nonce,
))
}
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
Ok(SubmitProofResponseMessage::InvalidChainId) => {
error!("Batcher responded with invalid chain id");
Expand Down
12 changes: 8 additions & 4 deletions batcher/aligned-sdk/src/core/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -90,7 +90,7 @@ pub enum SubmitError {
InvalidProof(ProofInvalidReason),
ProofTooLarge,
InvalidReplacementMessage,
InsufficientBalance(H160),
InsufficientBalance(H160, U256),
InvalidPaymentServiceAddress(H160, H160),
BatchSubmissionFailed(String),
AddToBatchError,
Expand Down Expand Up @@ -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, last_sent_valid_nonce) => {
write!(
f,
"Insufficient balance, address: {} last_sent_valid_nonce: {}",
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
addr, last_sent_valid_nonce
)
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
}
SubmitError::InvalidPaymentServiceAddress(received_addr, expected_addr) => {
write!(
Expand Down
2 changes: 1 addition & 1 deletion batcher/aligned-sdk/src/core/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ pub enum SubmitProofResponseMessage {
InvalidSignature,
ProofTooLarge,
InvalidMaxFee,
InsufficientBalance(Address),
InsufficientBalance(Address, U256),
InvalidChainId,
InvalidReplacementMessage,
AddToBatchError,
Expand Down
2 changes: 1 addition & 1 deletion batcher/aligned-sdk/src/sdk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ async fn _submit_multiple(
nonce,
)
.await;
receive(response_stream, sent_verification_data_rev).await
receive(response_stream, sent_verification_data_rev, nonce).await
}
.await;

Expand Down
20 changes: 14 additions & 6 deletions batcher/aligned/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,14 +392,22 @@ async fn main() -> Result<(), AlignedError> {
}
Err(e) => {
warn!("Error while submitting proof: {:?}", e);
handle_submit_err(e).await;
return Ok(());
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 multiple submissions that succeed but fail for a comulative balance of all max_fee's.
uri-99 marked this conversation as resolved.
Show resolved Hide resolved
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:"),
// If no verification data we do not log the msg.
0 => (),
_ => info!("Proofs submitted to aligned. See the batches in the explorer:"),
}

Expand Down Expand Up @@ -601,7 +609,7 @@ fn verification_data_from_args(args: &SubmitArgs) -> Result<VerificationData, Su
})
}

async fn handle_submit_err(err: SubmitError) {
async fn handle_submit_err(err: &SubmitError) {
match err {
SubmitError::InvalidNonce => {
error!("Invalid nonce. try again");
Expand All @@ -610,10 +618,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) => {
SubmitError::InsufficientBalance(sender_address, last_sent_valid_nonce) => {
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
error!(
"Insufficient balance to pay for the transaction, address: {}",
sender_address
"Insufficient balance to pay for the transaction, address: {} last_valid_nonce: {}",
sender_address, last_sent_valid_nonce
)
}
_ => {}
Expand Down
Loading