add retry in calldata (#1151)

* add retry in signature

* add retries to the entrypoint value

* remove extra signature length

* answer comments

---------

Co-authored-by: Gregory Edison <[email protected]>

.trunk/trunk.yaml

@@ -22,16 +22,16 @@ lint:
     - git-diff-check
     - [email protected]
     - [email protected]
-    - [email protected].3
+    - [email protected].4
     - [email protected]
-    - prettier@3.2.5
+    - prettier@3.3.0
     - [email protected]
     - [email protected]
     - [email protected]
     - [email protected]
     - [email protected]
-    - trivy@0.51.4
-    - trufflehog@3.77.0
+    - trivy@0.52.0
+    - trufflehog@3.78.0
     - [email protected]
   ignore:
     - linters: [ALL]

src/bin/hive_chain.rs

@@ -62,7 +62,7 @@ async fn main() -> eyre::Result<()> {
         for transaction in body.transactions {
             let signer = transaction.recover_signer().ok_or_eyre("failed to recover signer")?;
             let chain_id = transaction.chain_id().ok_or_eyre("failed to recover chain id")?;
-            let starknet_tx = to_starknet_transaction(&transaction, Some(chain_id), signer, u64::MAX)?;
+            let starknet_tx = to_starknet_transaction(&transaction, Some(chain_id), signer, u64::MAX, 0)?;
 
             let nonce = match &starknet_tx {
                 BroadcastedInvokeTransaction::V1(starknet_tx) => starknet_tx.nonce,

src/eth_provider/constant.rs

@@ -20,7 +20,7 @@ pub const ADDRESS_HEX_STRING_LEN: usize = 40;
 /// Starknet Modulus: 0x800000000000011000000000000000000000000000000000000000000000001
 pub const STARKNET_MODULUS: U256 = U256::from_limbs([0x1, 0, 0, 0x0800_0000_0000_0011]);
 /// Maximum number of times a transaction can be retried
-pub const TRANSACTION_MAX_RETRIES: u64 = 10;
+pub const TRANSACTION_MAX_RETRIES: u8 = 10;
 
 #[cfg(feature = "hive")]
 use {

src/eth_provider/database/types/transaction.rs

@@ -68,19 +68,19 @@ pub struct StoredPendingTransaction {
     #[serde(deserialize_with = "crate::eth_provider::database::types::serde::deserialize_intermediate")]
     pub tx: Transaction,
     /// Number of retries
-    pub retries: u64,
+    pub retries: u8,
 }
 
 impl StoredPendingTransaction {
-    pub const fn new(tx: Transaction, retries: u64) -> Self {
+    pub const fn new(tx: Transaction, retries: u8) -> Self {
         Self { tx, retries }
     }
 }
 
 #[cfg(any(test, feature = "arbitrary", feature = "testing"))]
 impl<'a> StoredPendingTransaction {
     pub fn arbitrary_with_optional_fields(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result<Self> {
-        Ok(Self { tx: StoredTransaction::arbitrary_with_optional_fields(u)?.into(), retries: u64::arbitrary(u)? })
+        Ok(Self { tx: StoredTransaction::arbitrary_with_optional_fields(u)?.into(), retries: u8::arbitrary(u)? })
     }
 }
 

src/eth_provider/provider.rs

@@ -552,31 +552,8 @@ where
         let filter = into_filter("tx.hash", &transaction_signed.hash, HASH_HEX_STRING_LEN);
         let pending_transaction = self.database.get_one::<StoredPendingTransaction>(filter.clone(), None).await?;
 
-        // Determine the maximum fee
-        let max_fee = if cfg!(feature = "hive") {
-            u64::MAX
-        } else {
-            // TODO(Kakarot Fee Mechanism): When we no longer need to use the Starknet fees, remove this line.
-            // We need to get the balance (in Kakarot/Starknet native Token) of the signer to compute the Starknet maximum `max_fee`.
-            // We used to set max_fee = u64::MAX, but it'll fail if the signer doesn't have enough balance to pay the fees.
-            let eth_fees_per_gas =
-                transaction_signed.effective_gas_price(Some(transaction_signed.max_fee_per_gas() as u64)) as u64;
-            let eth_fees = eth_fees_per_gas.saturating_mul(transaction_signed.gas_limit());
-            let balance = self.balance(signer, None).await?;
-            let max_fee: u64 = balance.try_into().unwrap_or(u64::MAX);
-            let max_fee = (u128::from(max_fee) * 80 / 100) as u64;
-
-            // We add the retry count to the max fee in order to bypass the
-            // DuplicateTx error in Starknet, which rejects incoming transactions
-            // with the same hash. Incrementing the max fee causes the Starknet
-            // hash to change, allowing the transaction to pass.
-            let retries = pending_transaction.as_ref().map(|tx| tx.retries + 1).unwrap_or_default();
-            max_fee.saturating_sub(eth_fees).saturating_add(retries)
-        };
-
-        // Deploy EVM transaction signer if Hive feature is enabled
-        #[cfg(feature = "hive")]
-        self.deploy_evm_transaction_signer(signer).await?;
+        // Number of retries for the transaction (0 if it's a new transaction)
+        let retries = pending_transaction.as_ref().map(|tx| tx.retries).unwrap_or_default();
 
         // Serialize transaction document
         let transaction =
@@ -601,12 +578,22 @@ where
             self.database.update_one::<StoredPendingTransaction>(transaction.into(), filter, true).await?;
         }
 
+        // The max fee is always set to 0. This means that no fee is perceived by the
+        // Starknet sequencer, which is the intended behavior has fee perception is
+        // handled by the Kakarot execution layer through EVM gas accounting.
+        let max_fee = 0;
+
         // Convert the transaction to a Starknet transaction
-        let starnet_transaction = to_starknet_transaction(&transaction_signed, maybe_chain_id, signer, max_fee)?;
+        let starknet_transaction =
+            to_starknet_transaction(&transaction_signed, maybe_chain_id, signer, max_fee, retries + 1)?;
+
+        // Deploy EVM transaction signer if Hive feature is enabled
+        #[cfg(feature = "hive")]
+        self.deploy_evm_transaction_signer(signer).await?;
 
         // Add the transaction to the Starknet provider
         let res =
-            self.starknet_provider.add_invoke_transaction(starnet_transaction).await.map_err(KakarotError::from)?;
+            self.starknet_provider.add_invoke_transaction(starknet_transaction).await.map_err(KakarotError::from)?;
 
         // Return transaction hash if testing feature is enabled, otherwise log and return Ethereum hash
         if cfg!(feature = "testing") {

src/eth_provider/starknet/kakarot_core.rs

@@ -99,6 +99,7 @@ pub fn to_starknet_transaction(
     chain_id: Option<u64>,
     signer: Address,
     max_fee: u64,
+    retries: u8,
 ) -> EthProviderResult<BroadcastedInvokeTransaction> {
     let sender_address = starknet_address(signer);
 
@@ -141,10 +142,20 @@ pub fn to_starknet_transaction(
     }
 
     let mut calldata = Vec::with_capacity(capacity);
+
+    // assert that the selector < FieldElement::MAX - retries
+    assert!(*ETH_SEND_TRANSACTION < FieldElement::MAX - retries.into());
+    let selector = *ETH_SEND_TRANSACTION + retries.into();
+
+    // Retries are used to alter the transaction hash in order to avoid the
+    // DuplicateTx error from the Starknet gateway, encountered whenever
+    // a transaction with the same hash is sent multiple times.
+    // We add the retries to the selector in the calldata, since the selector
+    // is not used in by the EOA contract during the transaction execution.
     calldata.append(&mut vec![
         FieldElement::ONE,        // call array length
         *KAKAROT_ADDRESS,         // contract address
-        *ETH_SEND_TRANSACTION,    // selector
+        selector,                 // selector + retries
         FieldElement::ZERO,       // data offset
         signed_data.len().into(), // data length
         signed_data.len().into(), // calldata length
@@ -179,6 +190,7 @@ mod tests {
             Some(1_802_203_764),
             Address::from_str("0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266").unwrap(),
             0,
+            0,
         )
         .unwrap()
         {
@@ -190,7 +202,7 @@ mod tests {
                     FieldElement::from(131_015_958_324_370_192_097_986_834_655_742_602_650_u128),
                     FieldElement::from(263_740_705_169_547_910_390_939_684_488_449_712_973_u128),
                     FieldElement::from(164_374_192_806_466_935_713_473_791_294_001_132_486_u128),
-                    FieldElement::ONE
+                    FieldElement::ONE,
                 ]
             );
 
@@ -252,6 +264,6 @@ mod tests {
         transaction.transaction.set_input(vec![0; 30000].into());
 
         // Attempt to convert the transaction into a Starknet transaction
-        to_starknet_transaction(&transaction, Some(1), transaction.recover_signer().unwrap(), 100_000_000).unwrap();
+        to_starknet_transaction(&transaction, Some(1), transaction.recover_signer().unwrap(), 100_000_000, 0).unwrap();
     }
 }

src/tracing/mod.rs

@@ -288,7 +288,7 @@ mod tests {
     use url::Url;
 
     #[tokio::test(flavor = "multi_thread")]
-    #[ignore = "This test is used for debugging purposes only"]
+    #[ignore = "this test is used for debugging purposes only"]
     async fn test_debug_tracing() {
         // Set the env vars
         std::env::set_var("KAKAROT_ADDRESS", "CHECK THE KAKAROT ADDRESS FOR THE BLOCK YOU ARE DEBUGGING");

tests/tests/eth_provider.rs

@@ -837,25 +837,43 @@ async fn test_retry_transactions(#[future] katana: Katana, _setup: ()) {
         .await
         .expect("Failed to insert transaction in mined collection");
 
-    // Retrieve the retried transactions.
-    let retried_transactions = eth_provider.retry_transactions().await.expect("Failed to retry transactions");
+    let mut pending_tx_hashes: Vec<B256> = Vec::new();
 
-    // Assert that there is only one retried transaction.
-    assert_eq!(retried_transactions.len(), 1);
+    for i in 0..TRANSACTION_MAX_RETRIES + 2 {
+        // Retrieve the retried transactions.
+        let retried_transactions = eth_provider.retry_transactions().await.expect("Failed to retry transactions");
 
-    // Retrieve the pending transactions.
-    let pending_transactions = eth_provider
-        .database()
-        .get::<StoredPendingTransaction>(None, None)
-        .await
-        .expect("Failed get pending transactions");
+        // Assert that there is only one retried transaction before reaching retry limit.
+        assert_eq!(retried_transactions.len(), usize::from(i < TRANSACTION_MAX_RETRIES));
+
+        // Retrieve the pending transactions.
+        let pending_transactions = eth_provider
+            .database()
+            .get::<StoredPendingTransaction>(None, None)
+            .await
+            .expect("Failed get pending transactions");
+
+        if i < TRANSACTION_MAX_RETRIES {
+            // Ensure that the spurious transactions are dropped from the pending transactions collection
+            assert_eq!(pending_transactions.len(), 1);
 
-    // Ensure that the spurious transactions are dropped from the pending transactions collection
-    assert_eq!(pending_transactions.len(), 1);
+            // Ensure that the retry is incremented for the first transaction
+            assert_eq!(pending_transactions.first().unwrap().retries, i + 1);
 
-    // Ensure that the retry is incremented for the first transaction
-    assert_eq!(pending_transactions.first().unwrap().retries, 1);
+            // Ensure that the transaction1 is still in the pending transactions collection
+            assert_eq!(pending_transactions.first().unwrap().tx, transaction1);
 
-    // Ensure that the transaction1 is still in the pending transactions collection
-    assert_eq!(pending_transactions.first().unwrap().tx, transaction1);
+            // Get the pending transaction hash
+            let pending_tx_hash = retried_transactions.first().unwrap();
+
+            // Ensure that the pending transaction hash is not already in the list
+            // Transaction hashes should be unique
+            assert!(!pending_tx_hashes.contains(pending_tx_hash));
+
+            // Add the pending transaction hash to the list
+            pending_tx_hashes.push(*pending_tx_hash);
+        } else {
+            assert_eq!(pending_transactions.len(), 0);
+        }
+    }
 }