fix: handlel1 transaction validation (keep-starknet-strange#1410)

fishseabowl · Jan 30, 2024 · f9b8b70 · f9b8b70
1 parent 3c11964
commit f9b8b70
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 41 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -269,3 +269,4 @@
 - feat: add `madara_tsukuyomi` as a submodule
 - branding: use new logo in the README
 - dev: Get the block status from the actual block in get_block_with_tx_hashes
+- fix: l1-l2 messaging
diff --git a/crates/client/l1-messages/src/worker.rs b/crates/client/l1-messages/src/worker.rs
@@ -54,37 +54,46 @@ pub async fn run_worker<C, P, B>(
         }
     };
 
-    while let Some(Ok((event, meta))) = event_stream.next().await {
-        log::info!(
-            "⟠ Processing L1 Message from block: {:?}, transaction_hash: {:?}, log_index: {:?}",
-            meta.block_number,
-            meta.transaction_hash,
-            meta.log_index
-        );
-
-        match process_l1_message(event, &client, &pool, &backend, &meta.block_number.as_u64(), &meta.log_index.as_u64())
+    while let Some(event_res) = event_stream.next().await {
+        if let Ok((event, meta)) = event_res {
+            log::info!(
+                "⟠ Processing L1 Message from block: {:?}, transaction_hash: {:?}, log_index: {:?}",
+                meta.block_number,
+                meta.transaction_hash,
+                meta.log_index
+            );
+
+            match process_l1_message(
+                event,
+                &client,
+                &pool,
+                &backend,
+                &meta.block_number.as_u64(),
+                &meta.log_index.as_u64(),
+            )
             .await
-        {
-            Ok(Some(tx_hash)) => {
-                log::info!(
-                    "⟠ L1 Message from block: {:?}, transaction_hash: {:?}, log_index: {:?} submitted, transaction \
-                     hash on L2: {:?}",
-                    meta.block_number,
-                    meta.transaction_hash,
-                    meta.log_index,
-                    tx_hash
-                );
-            }
-            Ok(None) => {}
-            Err(e) => {
-                log::error!(
-                    "⟠ Unexpected error while processing L1 Message from block: {:?}, transaction_hash: {:?}, \
-                     log_index: {:?}, error: {:?}",
-                    meta.block_number,
-                    meta.transaction_hash,
-                    meta.log_index,
-                    e
-                )
+            {
+                Ok(Some(tx_hash)) => {
+                    log::info!(
+                        "⟠ L1 Message from block: {:?}, transaction_hash: {:?}, log_index: {:?} submitted, \
+                         transaction hash on L2: {:?}",
+                        meta.block_number,
+                        meta.transaction_hash,
+                        meta.log_index,
+                        tx_hash
+                    );
+                }
+                Ok(None) => {}
+                Err(e) => {
+                    log::error!(
+                        "⟠ Unexpected error while processing L1 Message from block: {:?}, transaction_hash: {:?}, \
+                         log_index: {:?}, error: {:?}",
+                        meta.block_number,
+                        meta.transaction_hash,
+                        meta.log_index,
+                        e
+                    )
+                }
             }
         }
     }

diff --git a/crates/pallets/starknet/src/lib.rs b/crates/pallets/starknet/src/lib.rs
@@ -747,14 +747,21 @@ pub mod pallet {
                 .longevity(T::TransactionLongevity::get())
                 .propagate(true);
 
-            // Make sure txs from same account are executed in correct order (nonce based ordering)
-            if let TxPriorityInfo::RegularTxs { sender_address, transaction_nonce, sender_nonce } = tx_priority_info {
-                valid_transaction_builder =
-                    valid_transaction_builder.and_provides((sender_address, Felt252Wrapper(transaction_nonce.0)));
-                if transaction_nonce > sender_nonce {
-                    valid_transaction_builder = valid_transaction_builder
-                        .and_requires((sender_address, Felt252Wrapper(transaction_nonce.0 - FieldElement::ONE)));
+            match tx_priority_info {
+                // Make sure txs from same account are executed in correct order (nonce based ordering)
+                TxPriorityInfo::RegularTxs { sender_address, transaction_nonce, sender_nonce } => {
+                    valid_transaction_builder =
+                        valid_transaction_builder.and_provides((sender_address, Felt252Wrapper(transaction_nonce.0)));
+                    if transaction_nonce > sender_nonce {
+                        valid_transaction_builder = valid_transaction_builder
+                            .and_requires((sender_address, Felt252Wrapper(transaction_nonce.0 - FieldElement::ONE)));
+                    }
                 }
+                TxPriorityInfo::L1Handler { nonce } => {
+                    valid_transaction_builder =
+                        valid_transaction_builder.and_provides((Felt252Wrapper::ZERO, Felt252Wrapper(nonce.0)));
+                }
+                _ => {}
             }
 
             valid_transaction_builder.build()

diff --git a/crates/pallets/starknet/src/tests/l1_handler_validation.rs b/crates/pallets/starknet/src/tests/l1_handler_validation.rs
@@ -1,4 +1,5 @@
 use assert_matches::assert_matches;
+use mp_felt::Felt252Wrapper;
 use mp_transactions::{HandleL1MessageTransaction, UserAndL1HandlerTransaction};
 use sp_runtime::transaction_validity::InvalidTransaction;
 use starknet_api::api_core::Nonce;
@@ -40,7 +41,10 @@ fn should_accept_unused_nonce() {
 
         let tx = UserAndL1HandlerTransaction::L1Handler(transaction, Fee(100));
 
-        assert_matches!(Starknet::validate_unsigned_tx_nonce(&tx), Ok(TxPriorityInfo::L1Handler));
+        assert_eq!(
+            Starknet::validate_unsigned_tx_nonce(&tx),
+            Ok(TxPriorityInfo::L1Handler { nonce: Felt252Wrapper::ONE })
+        );
     });
 }
 

diff --git a/crates/pallets/starknet/src/transaction_validation.rs b/crates/pallets/starknet/src/transaction_validation.rs
@@ -55,10 +55,10 @@ impl<OuterOrigin: Into<Result<RawOrigin, OuterOrigin>> + From<RawOrigin>> Ensure
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq)]
 pub enum TxPriorityInfo {
     InvokeV0,
-    L1Handler,
+    L1Handler { nonce: Felt252Wrapper },
     RegularTxs { sender_address: Felt252Wrapper, transaction_nonce: Felt252Wrapper, sender_nonce: Felt252Wrapper },
 }
 
@@ -95,7 +95,8 @@ impl<T: Config> Pallet<T> {
             }
             UserAndL1HandlerTransaction::L1Handler(tx, _fee) => {
                 Self::ensure_l1_message_not_executed(&Nonce(StarkFelt::from(tx.nonce)))?;
-                Ok(TxPriorityInfo::L1Handler)
+
+                Ok(TxPriorityInfo::L1Handler { nonce: tx.nonce.into() })
             }
         }
     }