hotfix: overestimate batch size to avoid a DoS (#1775)

Co-authored-by: Urix <[email protected]>
Co-authored-by: Mauro Toscano <[email protected]>
Co-authored-by: MauroFab <[email protected]>

Author: 3 people

batcher/aligned-batcher/src/lib.rs

@@ -25,9 +25,9 @@ use std::time::Duration;
 use aligned_sdk::core::constants::{
     ADDITIONAL_SUBMISSION_GAS_COST_PER_PROOF, BATCHER_SUBMISSION_BASE_GAS_COST,
     BUMP_BACKOFF_FACTOR, BUMP_MAX_RETRIES, BUMP_MAX_RETRY_DELAY, BUMP_MIN_RETRY_DELAY,
-    CONNECTION_TIMEOUT, DEFAULT_MAX_FEE_PER_PROOF, ETHEREUM_CALL_BACKOFF_FACTOR,
-    ETHEREUM_CALL_MAX_RETRIES, ETHEREUM_CALL_MAX_RETRY_DELAY, ETHEREUM_CALL_MIN_RETRY_DELAY,
-    GAS_PRICE_PERCENTAGE_MULTIPLIER, PERCENTAGE_DIVIDER,
+    CBOR_ARRAY_MAX_OVERHEAD, CONNECTION_TIMEOUT, DEFAULT_MAX_FEE_PER_PROOF,
+    ETHEREUM_CALL_BACKOFF_FACTOR, ETHEREUM_CALL_MAX_RETRIES, ETHEREUM_CALL_MAX_RETRY_DELAY,
+    ETHEREUM_CALL_MIN_RETRY_DELAY, GAS_PRICE_PERCENTAGE_MULTIPLIER, PERCENTAGE_DIVIDER,
     RESPOND_TO_TASK_FEE_LIMIT_PERCENTAGE_MULTIPLIER,
 };
 use aligned_sdk::core::types::{
@@ -114,6 +114,16 @@ impl Batcher {
         let s3_client = s3::create_client(upload_endpoint).await;
 
         let config = ConfigFromYaml::new(config_file);
+        // Ensure max_batch_bytes_size can at least hold one proof of max_proof_size,
+        // including the overhead introduced by serialization
+        assert!(
+            config.batcher.max_proof_size + CBOR_ARRAY_MAX_OVERHEAD
+                <= config.batcher.max_batch_byte_size,
+            "max_batch_bytes_size ({}) not big enough for one max_proof_size ({}) proof",
+            config.batcher.max_batch_byte_size,
+            config.batcher.max_proof_size
+        );
+
         let deployment_output =
             ContractDeploymentOutput::new(config.aligned_layer_deployment_config_file_path);
 

batcher/aligned-batcher/src/types/batch_queue.rs

@@ -1,6 +1,9 @@
 use aligned_sdk::{
     communication::serialization::cbor_serialize,
-    core::types::{NoncedVerificationData, VerificationDataCommitment},
+    core::{
+        constants::CBOR_ARRAY_MAX_OVERHEAD,
+        types::{NoncedVerificationData, VerificationDataCommitment},
+    },
 };
 use ethers::types::{Address, Signature, U256};
 use priority_queue::PriorityQueue;
@@ -132,7 +135,10 @@ pub(crate) fn calculate_batch_size(batch_queue: &BatchQueue) -> Result<usize, Ba
     });
 
     if let ControlFlow::Continue(batch_size) = folded_result {
-        Ok(batch_size)
+        // We over-estimate the size of the batch by at most 8 bytes.
+        // This saves us from a scenario where we actually try to send more
+        // than the maximum allowed bytes and get rejected by operators.
+        Ok(CBOR_ARRAY_MAX_OVERHEAD + batch_size)
     } else {
         Err(BatcherError::SerializationError(String::from(
             "Could not calculate size of batch",

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

@@ -11,6 +11,15 @@ pub const DEFAULT_MAX_FEE_PER_PROOF: u128 =
     ADDITIONAL_SUBMISSION_GAS_COST_PER_PROOF * 100_000_000_000; // gas_price = 100 Gwei = 0.0000001 ether (high gas price)
 pub const CONNECTION_TIMEOUT: u64 = 30; // 30 secs
 
+// According to:
+// - https://www.rfc-editor.org/rfc/rfc8949.html#section-3.1-2.10
+// - https://www.rfc-editor.org/rfc/rfc8949.html#section-3-3.2
+// - https://www.rfc-editor.org/rfc/rfc8949.html#section-3-3.4
+// 9 bytes are the maximum overhead from aggregating data into an array in CBOR
+// (it may be as little as just 1 byte, but it depends on the number of elements
+// and serialization parameters).
+pub const CBOR_ARRAY_MAX_OVERHEAD: usize = 9;
+
 // % modifiers: (100% is x1, 10% is x0.1, 1000% is x10)
 pub const RESPOND_TO_TASK_FEE_LIMIT_PERCENTAGE_MULTIPLIER: u128 = 250; // fee_for_aggregator -> respondToTaskFeeLimit modifier
 pub const DEFAULT_AGGREGATOR_FEE_PERCENTAGE_MULTIPLIER: u128 = 125; // feeForAggregator modifier