feat: check evm tx actually succeeded before determining its tx status

rust/main/lander/src/adapter/chains/ethereum/adapter/tx_status_checker.rs

@@ -4,7 +4,7 @@ use ethers::types::U64;
 use hyperlane_ethereum::{EthereumReorgPeriod, EvmProviderForLander};
 use tracing::warn;
 
-use crate::{LanderError, TransactionStatus};
+use crate::{LanderError, TransactionDropReason, TransactionStatus};
 
 async fn block_number_result_to_tx_status(
     provider: &Arc<dyn EvmProviderForLander>,
@@ -38,16 +38,139 @@ pub async fn get_tx_hash_status(
     hash: hyperlane_core::H512,
     reorg_period: &EthereumReorgPeriod,
 ) -> Result<TransactionStatus, LanderError> {
-    match provider.get_transaction_receipt(hash.into()).await {
-        Ok(None) => Err(LanderError::TxHashNotFound(
-            "Transaction not found".to_string(),
+    let receipt = provider
+        .get_transaction_receipt(hash.into())
+        .await
+        .map_err(|err| LanderError::TxHashNotFound(err.to_string()))?
+        .ok_or_else(|| LanderError::TxHashNotFound("Transaction not found".to_string()))?;
+
+    tracing::debug!(?receipt, "tx receipt");
+    match receipt.status.as_ref().map(|s| s.as_u64()) {
+        // https://eips.ethereum.org/EIPS/eip-658
+        Some(0) => Ok(TransactionStatus::Dropped(
+            TransactionDropReason::RevertedByChain,
         )),
-        Ok(Some(receipt)) => {
-            Ok(
+        _ => {
+            let res =
                 block_number_result_to_tx_status(provider, receipt.block_number, reorg_period)
-                    .await,
-            )
+                    .await;
+            Ok(res)
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use std::str::FromStr;
+
+    use ethers::{
+        providers::{Middleware, MockProvider, Provider},
+        types::{Address, Bloom, TransactionReceipt, H256, U256},
+    };
+    use hyperlane_core::{HyperlaneDomain, KnownHyperlaneDomain, H512};
+    use hyperlane_ethereum::EthereumProvider;
+
+    use super::*;
+
+    fn test_tx_receipt(transaction_hash: H256, status: Option<U64>) -> TransactionReceipt {
+        TransactionReceipt {
+            transaction_hash,
+            transaction_index: U64::from(206),
+            block_hash: Some(
+                H256::from_str("bd36ff1aeafac61b89642ac30e682234b4dfa87c9ff6987b66f709c09f60d1d0")
+                    .unwrap(),
+            ),
+            block_number: Some(U64::from(23327789)),
+            from: Address::from_str("74cae0ecc47b02ed9b9d32e000fd70b9417970c5").unwrap(),
+            to: Some(Address::from_str("c005dc82818d67af737725bd4bf75435d065d239").unwrap()),
+            contract_address: None,
+            cumulative_gas_used: U256::from(17343049),
+            effective_gas_price: Some(U256::from(291228702)),
+            gas_used: Some(U256::from(39040)),
+            logs: Vec::new(),
+            status,
+            root: None,
+            logs_bloom: Bloom::default(),
+            transaction_type: Some(U64::from(206)),
         }
-        Err(err) => Err(LanderError::TxHashNotFound(err.to_string())),
+    }
+
+    /// When the transaction was sent to network, but failed
+    /// during execution.
+    #[tokio::test]
+    async fn test_get_tx_hash_status_failed_tx() {
+        let transaction_hash =
+            H256::from_str("575841942e0de82d3129cccf53e4e9c75b6d8a163f8a83d330a2e8d574820a4d")
+                .unwrap();
+
+        let mock_provider = MockProvider::new();
+
+        let tx_receipt = test_tx_receipt(transaction_hash, Some(U64::from(0)));
+        let _ = mock_provider.push(tx_receipt);
+
+        let ethers_provider = Provider::new(mock_provider);
+        let evm_provider: Arc<dyn EvmProviderForLander> = Arc::new(EthereumProvider::new(
+            Arc::new(ethers_provider),
+            HyperlaneDomain::Known(KnownHyperlaneDomain::Ethereum),
+        ));
+        let reorg_period = EthereumReorgPeriod::Blocks(15);
+
+        let tx_status = get_tx_hash_status(&evm_provider, transaction_hash.into(), &reorg_period)
+            .await
+            .unwrap();
+        assert_eq!(
+            tx_status,
+            TransactionStatus::Dropped(TransactionDropReason::RevertedByChain)
+        );
+    }
+
+    #[tokio::test]
+    async fn test_get_tx_hash_status_success() {
+        let transaction_hash =
+            H256::from_str("575841942e0de82d3129cccf53e4e9c75b6d8a163f8a83d330a2e8d574820a4d")
+                .unwrap();
+
+        let mock_provider = MockProvider::new();
+
+        let _ = mock_provider.push(U64::from(23327790u64));
+        let tx_receipt = test_tx_receipt(transaction_hash, Some(U64::from(1)));
+        let _ = mock_provider.push(tx_receipt);
+
+        let ethers_provider = Provider::new(mock_provider);
+        let evm_provider: Arc<dyn EvmProviderForLander> = Arc::new(EthereumProvider::new(
+            Arc::new(ethers_provider),
+            HyperlaneDomain::Known(KnownHyperlaneDomain::Ethereum),
+        ));
+        let reorg_period = EthereumReorgPeriod::Blocks(15);
+
+        let tx_status = get_tx_hash_status(&evm_provider, transaction_hash.into(), &reorg_period)
+            .await
+            .unwrap();
+        assert_eq!(tx_status, TransactionStatus::Included);
+    }
+
+    #[tokio::test]
+    async fn test_get_tx_hash_status_success_finalized() {
+        let transaction_hash =
+            H256::from_str("575841942e0de82d3129cccf53e4e9c75b6d8a163f8a83d330a2e8d574820a4d")
+                .unwrap();
+
+        let mock_provider = MockProvider::new();
+
+        let _ = mock_provider.push(U64::from(23328000u64));
+        let tx_receipt = test_tx_receipt(transaction_hash, Some(U64::from(1)));
+        let _ = mock_provider.push(tx_receipt);
+
+        let ethers_provider = Provider::new(mock_provider);
+        let evm_provider: Arc<dyn EvmProviderForLander> = Arc::new(EthereumProvider::new(
+            Arc::new(ethers_provider),
+            HyperlaneDomain::Known(KnownHyperlaneDomain::Ethereum),
+        ));
+        let reorg_period = EthereumReorgPeriod::Blocks(15);
+
+        let tx_status = get_tx_hash_status(&evm_provider, transaction_hash.into(), &reorg_period)
+            .await
+            .unwrap();
+        assert_eq!(tx_status, TransactionStatus::Finalized);
     }
 }

rust/main/lander/src/dispatcher/tests.rs

@@ -8,8 +8,8 @@ use crate::dispatcher::{BuildingStageQueue, DispatcherState, PayloadDbLoader};
 use crate::tests::test_utils::{dummy_tx, tmp_dbs, MockAdapter};
 use crate::transaction::TransactionUuid;
 use crate::{
-    Dispatcher, DispatcherEntrypoint, Entrypoint, FullPayload, LanderError, PayloadStatus,
-    PayloadUuid, TransactionStatus,
+    Dispatcher, DispatcherEntrypoint, Entrypoint, FullPayload, LanderError, PayloadDropReason,
+    PayloadStatus, PayloadUuid, TransactionDropReason, TransactionStatus,
 };
 
 use super::PayloadDb;
@@ -266,6 +266,61 @@ async fn test_entrypoint_send_fails_estimation_after_first_submission() {
     assert_metrics(metrics, metrics_assertion);
 }
 
+#[tracing_test::traced_test]
+#[tokio::test]
+async fn test_entrypoint_send_reverts_onchain() {
+    let payload = FullPayload::random();
+
+    let mut adapter = MockAdapter::new();
+    // the payload always fails simulation
+    adapter
+        .expect_simulate_tx()
+        .returning(move |_| Ok(Vec::new()));
+    adapter.expect_estimate_tx().returning(move |_| Ok(()));
+    adapter.expect_tx_status().returning(move |_| {
+        Ok(TransactionStatus::Dropped(
+            TransactionDropReason::RevertedByChain,
+        ))
+    });
+    let adapter = mock_adapter_methods(adapter, payload.clone());
+    let adapter = Arc::new(adapter);
+    let (entrypoint, dispatcher) = mock_entrypoint_and_dispatcher(adapter.clone()).await;
+    let metrics = dispatcher.inner.metrics.clone();
+
+    let _payload_dispatcher = tokio::spawn(async move { dispatcher.spawn().await });
+    entrypoint.send_payload(&payload).await.unwrap();
+
+    // wait until the payload status is InTransaction(Dropped(_))
+    wait_until_payload_status(
+        entrypoint.inner.payload_db.clone(),
+        payload.uuid(),
+        |payload_status| {
+            matches!(
+                payload_status,
+                PayloadStatus::InTransaction(TransactionStatus::Dropped(_))
+            )
+        },
+    )
+    .await;
+    sleep(Duration::from_millis(200)).await; // Wait for the metrics to be updated
+
+    // Even though the error is RevertedByChain, in inclusion_stage::process_txs_step()
+    // we hardcode it to TxDropReason::FailedSimulation
+    let metrics_assertion = MetricsAssertion {
+        domain: entrypoint.inner.domain.clone(),
+        finalized_txs: 0,
+        building_stage_queue_length: 0,
+        inclusion_stage_pool_length: 0,
+        finality_stage_pool_length: 0,
+        dropped_payloads: 1,
+        dropped_transactions: 1,
+        dropped_payload_reason: "DroppedInTransaction(FailedSimulation)".to_string(),
+        dropped_transaction_reason: "FailedSimulation".to_string(),
+        transaction_submissions: 0,
+    };
+    assert_metrics(metrics, metrics_assertion);
+}
+
 #[tracing_test::traced_test]
 #[tokio::test]
 async fn test_entrypoint_send_fails_estimation_before_first_submission() {

rust/main/lander/src/transaction/types.rs

@@ -102,6 +102,8 @@ pub enum DropReason {
     DroppedByChain,
     /// dropped by the submitter
     FailedSimulation,
+    /// tx reverted
+    RevertedByChain,
 }
 
 // add nested enum entries as we add VMs