chore(sequencer)!: change Deposit byte length calculation

crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap

@@ -3,36 +3,36 @@ source: crates/astria-sequencer/src/app/tests_breaking_changes.rs
 expression: app.app_hash.as_bytes()
 ---
 [
-  18,
-  206,
-  200,
+  223,
+  34,
+  80,
   101,
-  160,
-  129,
-  124,
-  188,
-  249,
-  181,
-  44,
-  218,
   209,
-  251,
-  208,
-  119,
-  122,
-  211,
-  105,
-  201,
-  75,
   214,
-  56,
-  145,
-  239,
+  82,
+  86,
+  3,
+  65,
+  117,
+  84,
+  67,
+  250,
+  90,
+  90,
+  232,
+  199,
+  39,
+  173,
+  9,
+  58,
+  116,
+  197,
+  172,
   132,
-  48,
-  0,
-  79,
-  13,
-  53,
-  156
+  63,
+  121,
+  66,
+  17,
+  108,
+  196
 ]

crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_finalize_block_snapshot.snap

@@ -3,36 +3,36 @@ source: crates/astria-sequencer/src/app/tests_breaking_changes.rs
 expression: app.app_hash.as_bytes()
 ---
 [
-  113,
-  62,
-  40,
-  174,
-  69,
-  178,
-  27,
-  49,
-  166,
-  136,
-  209,
-  163,
-  103,
-  135,
-  254,
-  99,
-  63,
-  197,
-  35,
-  165,
-  202,
-  177,
-  78,
+  111,
+  25,
+  76,
   238,
+  112,
+  77,
+  102,
+  234,
+  8,
+  97,
+  24,
+  100,
+  73,
+  128,
+  228,
   106,
-  144,
-  250,
-  69,
-  90,
-  192,
-  129,
-  187
+  82,
+  255,
+  119,
+  93,
+  248,
+  224,
+  51,
+  239,
+  115,
+  58,
+  9,
+  149,
+  86,
+  23,
+  248,
+  114
 ]

crates/astria-sequencer/src/app/tests_block_fees.rs

@@ -31,7 +31,7 @@ use crate::{
     },
     assets::StateReadExt as _,
     bridge::{
-        get_deposit_byte_len,
+        calculate_base_deposit_fee,
         StateWriteExt as _,
     },
     sequence::{
@@ -281,7 +281,7 @@ async fn ensure_correct_block_fees_bridge_lock() {
         .map(|(_, fee)| fee)
         .sum();
     let expected_fees = transfer_base_fee
-        + (get_deposit_byte_len(&test_deposit) * bridge_lock_byte_cost_multiplier);
+        + (calculate_base_deposit_fee(&test_deposit).unwrap() * bridge_lock_byte_cost_multiplier);
     assert_eq!(total_block_fees, expected_fees);
 }
 

crates/astria-sequencer/src/app/tests_execute_transaction.rs

@@ -756,7 +756,7 @@ async fn app_execute_transaction_bridge_lock_action_ok() {
             .get_bridge_lock_byte_cost_multiplier()
             .await
             .unwrap()
-            * crate::bridge::get_deposit_byte_len(&expected_deposit);
+            * crate::bridge::calculate_base_deposit_fee(&expected_deposit).unwrap();
     assert_eq!(
         app.state
             .get_account_balance(alice_address, nria())

crates/astria-sequencer/src/bridge/bridge_lock_action.rs

@@ -34,6 +34,10 @@ use crate::{
     utils::create_deposit_event,
 };
 
+// The base byte length of a deposit, as determined by the unit test
+// `get_base_deposit_byte_length()` below.
+const DEPOSIT_BASE_FEE: u128 = 16;
+
 #[async_trait::async_trait]
 impl ActionHandler for BridgeLockAction {
     async fn check_stateless(&self) -> Result<()> {
@@ -99,7 +103,10 @@ impl ActionHandler for BridgeLockAction {
             .await
             .wrap_err("failed to get byte cost multiplier")?;
         let fee = byte_cost_multiplier
-            .saturating_mul(get_deposit_byte_len(&deposit))
+            .saturating_mul(
+                calculate_base_deposit_fee(&deposit)
+                    .expect("deposit fee calculation should not fail"),
+            )
             .saturating_add(transfer_fee);
         ensure!(from_balance >= fee, "insufficient funds for fee payment");
 
@@ -126,7 +133,9 @@ impl ActionHandler for BridgeLockAction {
             .get_bridge_lock_byte_cost_multiplier()
             .await
             .wrap_err("failed to get byte cost multiplier")?;
-        let fee = byte_cost_multiplier.saturating_mul(get_deposit_byte_len(&deposit));
+        let fee = byte_cost_multiplier.saturating_mul(
+            calculate_base_deposit_fee(&deposit).expect("deposit fee calculation should not fail"),
+        );
         state
             .get_and_increase_block_fees(&self.fee_asset, fee, Self::full_name())
             .await
@@ -145,19 +154,31 @@ impl ActionHandler for BridgeLockAction {
     }
 }
 
-/// returns the length of a serialized `Deposit` message.
-pub(crate) fn get_deposit_byte_len(deposit: &Deposit) -> u128 {
-    use prost::Message as _;
-    let raw = deposit.clone().into_raw();
-    raw.encoded_len() as u128
+/// Returns a modified byte length of the deposit event. Length is calculated with reasonable values
+/// for all fields except `asset` and `destination_chain_address`, ergo it may not be representative
+/// of on-wire length.
+pub(crate) fn calculate_base_deposit_fee(deposit: &Deposit) -> Option<u128> {
+    let variable_length = deposit
+        .asset
+        .to_string()
+        .len()
+        .checked_add(deposit.destination_chain_address.len())
+        .expect("deposit byte length overflowed usize") as u128;
+    DEPOSIT_BASE_FEE.checked_add(variable_length)
 }
 
 #[cfg(test)]
 mod tests {
     use astria_core::primitive::v1::{
-        asset,
+        asset::{
+            self,
+        },
+        Address,
         RollupId,
         TransactionId,
+        ADDRESS_LEN,
+        ROLLUP_ID_LEN,
+        TRANSACTION_ID_LEN,
     };
     use cnidarium::StateDelta;
 
@@ -217,7 +238,7 @@ mod tests {
 
         // not enough balance; should fail
         state
-            .put_account_balance(from_address, &asset, 100 + transfer_fee)
+            .put_account_balance(from_address, &asset, transfer_fee)
             .unwrap();
         assert_eyre_error(
             &bridge_lock.check_and_execute(&mut state).await.unwrap_err(),
@@ -226,18 +247,118 @@ mod tests {
 
         // enough balance; should pass
         let expected_deposit_fee = transfer_fee
-            + get_deposit_byte_len(&Deposit {
+            + calculate_base_deposit_fee(&Deposit {
                 bridge_address,
                 rollup_id,
                 amount: 100,
                 asset: asset.clone(),
                 destination_chain_address: "someaddress".to_string(),
                 source_transaction_id: transaction_id,
                 source_action_index: 0,
-            }) * 2;
+            })
+            .unwrap()
+                * 2;
         state
             .put_account_balance(from_address, &asset, 100 + expected_deposit_fee)
             .unwrap();
         bridge_lock.check_and_execute(&mut state).await.unwrap();
     }
+
+    #[test]
+    fn test_calculate_base_deposit_fee() {
+        // Test for length equality with drastically different int values
+        let deposit = Deposit {
+            bridge_address: astria_address(&[1; 20]),
+            rollup_id: RollupId::from_unhashed_bytes(b"test_rollup_id"),
+            amount: u128::MAX,
+            asset: test_asset(),
+            destination_chain_address: "someaddress".to_string(),
+            source_transaction_id: TransactionId::new([0; 32]),
+            source_action_index: u64::MAX,
+        };
+        let calculated_len_0 = calculate_base_deposit_fee(&deposit).unwrap();
+
+        let deposit = Deposit {
+            bridge_address: astria_address(&[1; 20]),
+            rollup_id: RollupId::from_unhashed_bytes(b"test_rollup_id"),
+            amount: 0,
+            asset: test_asset(),
+            destination_chain_address: "someaddress".to_string(),
+            source_transaction_id: TransactionId::new([0; 32]),
+            source_action_index: 0,
+        };
+        let calculated_len_1 = calculate_base_deposit_fee(&deposit).unwrap();
+        assert_eq!(calculated_len_0, calculated_len_1);
+
+        // Ensure longer asset name results in longer byte length.
+        let deposit = Deposit {
+            bridge_address: astria_address(&[1; 20]),
+            rollup_id: RollupId::from_unhashed_bytes(b"test_rollup_id"),
+            amount: 0,
+            asset: "test_asset".parse().unwrap(),
+            destination_chain_address: "someaddress".to_string(),
+            source_transaction_id: TransactionId::new([0; 32]),
+            source_action_index: 0,
+        };
+        let calculated_len_2 = calculate_base_deposit_fee(&deposit).unwrap();
+        assert!(calculated_len_2 >= calculated_len_1);
+
+        // Ensure longer destination chain address results in longer byte length.
+        let deposit = Deposit {
+            bridge_address: astria_address(&[1; 20]),
+            rollup_id: RollupId::from_unhashed_bytes(b"test_rollup_id"),
+            amount: 0,
+            asset: "test_asset".parse().unwrap(),
+            destination_chain_address: "someaddresslonger".to_string(),
+            source_transaction_id: TransactionId::new([0; 32]),
+            source_action_index: 0,
+        };
+        let calculated_len_3 = calculate_base_deposit_fee(&deposit).unwrap();
+        assert!(calculated_len_3 >= calculated_len_2);
+
+        // Ensure calculated length is as expected with absurd string
+        // lengths (have tested up to 99999999, but this makes testing very slow)
+        let absurd_string: String = ['a'; u16::MAX as usize].iter().collect();
+        let deposit = Deposit {
+            bridge_address: astria_address(&[1; 20]),
+            rollup_id: RollupId::from_unhashed_bytes(b"test_rollup_id"),
+            amount: 0,
+            asset: absurd_string.parse().unwrap(),
+            destination_chain_address: absurd_string.to_string(),
+            source_transaction_id: TransactionId::new([0; 32]),
+            source_action_index: 0,
+        };
+        let calculated_len = calculate_base_deposit_fee(&deposit).unwrap();
+        let expected_len = 16 + u128::from(u16::MAX) * 2;
+        assert_eq!(calculated_len, expected_len);
+    }
+
+    /// Used to determine the base deposit byte length for `get_deposit_byte_len()`. This is based
+    /// on "reasonable" values for all fields except `asset` and `destination_chain_address`. These
+    /// are empty strings, whose length will be added to the base cost at the time of
+    /// calculation.
+    ///
+    /// This test determines 165 bytes for an average deposit with empty `asset` and
+    /// `destination_chain_address`, which is divided by 10 to get our base byte length of 16. This
+    /// is to allow for more flexibility in overall fees (we have more flexibility multiplying by a
+    /// lower number, and if we want fees to be higher we can just raise the multiplier).
+    #[test]
+    fn get_base_deposit_fee() {
+        use prost::Message as _;
+        let bridge_address = Address::builder()
+            .prefix("astria-bridge")
+            .slice(&[0u8; ADDRESS_LEN][..])
+            .try_build()
+            .unwrap();
+        let raw_deposit = astria_core::generated::sequencerblock::v1alpha1::Deposit {
+            bridge_address: Some(bridge_address.to_raw()),
+            rollup_id: Some(RollupId::from_unhashed_bytes([0; ROLLUP_ID_LEN]).to_raw()),
+            amount: Some(1000.into()),
+            asset: String::new(),
+            destination_chain_address: String::new(),
+            source_transaction_id: Some(TransactionId::new([0; TRANSACTION_ID_LEN]).to_raw()),
+            source_action_index: 0,
+        };
+        assert_eq!(DEPOSIT_BASE_FEE, raw_deposit.encoded_len() as u128 / 10);
+    }
 }

crates/astria-sequencer/src/bridge/mod.rs

@@ -6,7 +6,7 @@ pub(crate) mod init_bridge_account_action;
 pub(crate) mod query;
 mod state_ext;
 
-pub(crate) use bridge_lock_action::get_deposit_byte_len;
+pub(crate) use bridge_lock_action::calculate_base_deposit_fee;
 pub(crate) use state_ext::{
     StateReadExt,
     StateWriteExt,

crates/astria-sequencer/src/bridge/snapshots/astria_sequencer__bridge__bridge_lock_action__tests__get_deposit_byte_length_snapshot.snap

@@ -0,0 +1,5 @@
+---
+source: crates/astria-sequencer/src/bridge/bridge_lock_action.rs
+expression: get_deposit_byte_len(&deposit)
+---
+31

crates/astria-sequencer/src/transaction/checks.rs

@@ -275,7 +275,7 @@ fn bridge_lock_update_fees(
     use astria_core::sequencerblock::v1alpha1::block::Deposit;
 
     let expected_deposit_fee = transfer_fee.saturating_add(
-        crate::bridge::get_deposit_byte_len(&Deposit {
+        crate::bridge::calculate_base_deposit_fee(&Deposit {
             bridge_address: act.to,
             // rollup ID doesn't matter here, as this is only used as a size-check
             rollup_id: RollupId::from_unhashed_bytes([0; 32]),
@@ -285,6 +285,7 @@ fn bridge_lock_update_fees(
             source_transaction_id: TransactionId::new([0; 32]),
             source_action_index: tx_index_of_action,
         })
+        .unwrap()
         .saturating_mul(bridge_lock_byte_cost_multiplier),
     );