Skip to content
Merged
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 30 additions & 17 deletions batcher/aligned-batcher/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use aligned_sdk::core::constants::{
ADDITIONAL_SUBMISSION_GAS_COST_PER_PROOF, AGGREGATOR_GAS_COST, CANCEL_TRANSACTION_MAX_RETRIES,
CONSTANT_GAS_COST, DEFAULT_AGGREGATOR_FEE_PERCENTAGE_MULTIPLIER, DEFAULT_BACKOFF_FACTOR,
DEFAULT_MAX_FEE_PER_PROOF, DEFAULT_MAX_RETRIES, DEFAULT_MIN_RETRY_DELAY,
GAS_PRICE_PERCENTAGE_MULTIPLIER, MIN_FEE_PER_PROOF, PERCENTAGE_DIVIDER,
GAS_PRICE_PERCENTAGE_MULTIPLIER, PERCENTAGE_DIVIDER,
RESPOND_TO_TASK_FEE_LIMIT_PERCENTAGE_MULTIPLIER,
};
use aligned_sdk::core::types::{
Expand Down Expand Up @@ -672,7 +672,21 @@ impl Batcher {
return Ok(());
};

if !self.check_min_balance(proofs_in_batch + 1, user_balance) {
let msg_max_fee = nonced_verification_data.max_fee;
let Some(user_min_fee) = batch_state_lock.get_user_min_fee(&addr).await else {
std::mem::drop(batch_state_lock);
send_message(
ws_conn_sink.clone(),
SubmitProofResponseMessage::InvalidNonce,
)
.await;
self.metrics.user_error(&["invalid_nonce", ""]);
return Ok(());
};

// We estimate the minimum balance for submission to be the product of the user's `user_min_fee`,
// and the number of user's proof in a batch including the currently submitted proof (`proofs_in_the_batch + 1`).
if !self.check_min_balance(user_min_fee, proofs_in_batch + 1, user_balance, msg_max_fee) {
std::mem::drop(batch_state_lock);
send_message(
ws_conn_sink.clone(),
Expand Down Expand Up @@ -724,18 +738,6 @@ impl Batcher {
return Ok(());
}

let msg_max_fee = nonced_verification_data.max_fee;
let Some(user_min_fee) = batch_state_lock.get_user_min_fee(&addr).await else {
std::mem::drop(batch_state_lock);
send_message(
ws_conn_sink.clone(),
SubmitProofResponseMessage::InvalidNonce,
)
.await;
self.metrics.user_error(&["invalid_nonce", ""]);
return Ok(());
};

if msg_max_fee > user_min_fee {
std::mem::drop(batch_state_lock);
warn!("Invalid max fee for address {addr}, had fee {user_min_fee:?} < {msg_max_fee:?}");
Expand Down Expand Up @@ -777,9 +779,20 @@ impl Batcher {
zk_utils::is_verifier_disabled(*disabled_verifiers, verifier)
}

// Checks user has sufficient balance for paying all its the proofs in the current batch.
fn check_min_balance(&self, user_proofs_in_batch: usize, user_balance: U256) -> bool {
let min_balance = U256::from(user_proofs_in_batch) * U256::from(MIN_FEE_PER_PROOF);
// Checks user has sufficient balance for paying all the users proofs in the current batch.
fn check_min_balance(
&self,
user_min_fee: U256,
user_proofs_in_batch: usize,
user_balance: U256,
user_max_fee: U256,
) -> bool {
// `user_min_fee` is the minimum `max_fee` the user submitted to the batcher and represents the maximium price that the user will pay for each submitted proof. We define 'user_min_fee' as an upper bound for the proof submission cost of the user, and use it to validate the user's balance has enough fund available to pay for all submitted proofs.
let mut min_fee = user_min_fee;
if user_min_fee == U256::max_value() {
min_fee = user_max_fee
}
let min_balance: U256 = U256::from(user_proofs_in_batch) * min_fee;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I am wrong here but why don't we move the min_balance check after the msg_max_fee check? This way, in the min_balance check we'll verify against the msg_max_fee directly (as it will become the new min_fee if everything goes ok), we'll save ourselves this if:

        if user_min_fee == U256::max_value() {
            min_fee = user_max_fee
        }

It is hard to understand that U256::max_value() is the default value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thoughts are to leave this check in as it handles an explicit case that a user's proof has been submitted for the first time not the replacement max_fee invariant. I we move this check behind the msg_max_fee check we still need to have logic to check and specify that if a proof is submitted for the first time we use it's submitted max_fee instead of the user_max_fee. I think it's worth considering verifying that these checks are not redundant, but lets address that in another issue.

user_balance >= min_balance
}

Expand Down