Cleanup

Author: snawaz

magicblock-committor-service/src/intent_executor/error.rs

@@ -177,6 +177,8 @@ impl TransactionStrategyExecutionError {
         tasks: &[Box<dyn BaseTask>],
     ) -> Result<Self, TransactionError> {
         // There's always 3 budget instructions in front
+        // TODO (snawaz): this is offset-sensitive, if we add or remove any instruction from the
+        // front, that leads to incorrect error reporting. so if possible, make it offset-insensitive.
         const OFFSET: u8 = 3;
         const NONCE_OUT_OF_ORDER: u32 =
             dlp::error::DlpError::NonceOutOfOrder as u32;

magicblock-committor-service/src/intent_executor/mod.rs

@@ -619,8 +619,6 @@ where
             )
             .await?;
 
-        //      println!("prepared_message: {:#?}", prepared_message);
-
         // Execute strategy
         let execution_result = self
             .execute_message_with_retries(

magicblock-committor-service/src/intent_executor/single_stage_executor.rs

@@ -58,8 +58,6 @@ where
                 .await
                 .map_err(IntentExecutorError::FailedFinalizePreparationError)?;
 
-            println!("execution_result: {:#?}", execution_result);
-
             // Process error: Ok - return, Err - handle further
             let execution_err = match execution_result {
                 // break with result, strategy that was executed at this point has to be returned for cleanup
@@ -133,7 +131,6 @@ where
             TransactionStrategyExecutionError::CommitIDError(_, _) => {
                 // Here we patch strategy for it to be retried in next iteration
                 // & we also record data that has to be cleaned up after patch
-                println!("patch_strategy");
                 let to_cleanup = inner
                     .handle_commit_id_error(
                         committed_pubkeys,

magicblock-committor-service/src/intent_executor/task_info_fetcher.rs

@@ -197,7 +197,6 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher {
                 cache.push(*pubkey, *id);
             });
 
-            println!("fetcher! commit_ids: {:?}", result);
             return Ok(result);
         }
 
@@ -230,7 +229,6 @@ impl TaskInfoFetcher for CacheTaskInfoFetcher {
                 });
         }
 
-        println!("fetcher commit_ids: {:?}", result);
         Ok(result)
     }
 

magicblock-committor-service/src/tasks/task_builder.rs

@@ -134,8 +134,6 @@ impl TasksBuilder for TaskBuilderImpl {
         let commit_ids =
             commit_ids.map_err(TaskBuilderError::CommitTasksBuildError)?;
 
-        println!("task_builder commit_ids: {:?}", commit_ids);
-
         let base_accounts = match base_accounts {
             Ok(map) => map,
             Err(err) => {

test-integration/test-committor-service/tests/test_intent_executor.rs

@@ -15,7 +15,7 @@ use futures::future::join_all;
 use magicblock_committor_program::pdas;
 use magicblock_committor_service::{
     intent_executor::{
-        error::{IntentExecutorError, TransactionStrategyExecutionError},
+        error::TransactionStrategyExecutionError,
         task_info_fetcher::{CacheTaskInfoFetcher, TaskInfoFetcher},
         ExecutionOutput, IntentExecutionResult, IntentExecutor,
         IntentExecutorImpl,
@@ -52,7 +52,6 @@ use solana_sdk::{
     transaction::Transaction,
 };
 
-use self::utils::transactions::print_log_messages;
 use crate::{
     common::TestFixture,
     utils::{
@@ -364,16 +363,6 @@ async fn test_commit_id_error_recovery() {
         patched_errors,
     } = res;
 
-    if let Err(IntentExecutorError::FailedToFinalizeError {
-        err: _,
-        commit_signature: _,
-        finalize_signature: Some(sig),
-    }) = &res
-    {
-        let rpc_client = RpcClient::new("http://localhost:7799".to_string());
-        print_log_messages(&rpc_client, sig).await;
-    }
-
     assert!(
         res.is_ok(),
         "res: {:?}, patched_errors: {:#?}",

test-integration/test-committor-service/tests/test_ix_commit_local.rs

@@ -109,7 +109,7 @@ async fn test_ix_commit_order_book_change_671_bytes() {
 async fn test_ix_commit_order_book_change_673_bytes() {
     // We cannot use 672 as changed_len because that both 671 and 672 produce encoded tx
     // of size 1644 (which is the max limit), but while the size of raw bytes for 671 is within
-    // 1232 limit, the size for 672 execeds by 1 (1233). That is why we used
+    // 1232 limit, the size for 672 exceeds by 1 (1233). That is why we used
     // 673 as changed_len where CommitStrategy goes from Args to FromBuffer.
     commit_book_order_account(673, CommitStrategy::FromBuffer, false).await;
 }

test-integration/test-committor-service/tests/utils/transactions.rs

@@ -69,54 +69,10 @@ macro_rules! get_account {
 }
 
 #[allow(dead_code)]
-pub async fn print_log_messages(rpc_client: &RpcClient, signature: &Signature) {
-    const MAX_RETRIES: usize = 5;
-    let mut retries = MAX_RETRIES;
-    let tx = loop {
-        match rpc_client
-            .get_transaction_with_config(
-                signature,
-                RpcTransactionConfig {
-                    commitment: Some(CommitmentConfig::confirmed()),
-                    max_supported_transaction_version: Some(0),
-                    ..Default::default()
-                },
-            )
-            .await
-        {
-            Ok(tx) => break tx,
-            Err(err) => {
-                log::error!("Failed to get transaction: {}", err);
-                retries -= 1;
-                if retries == 0 {
-                    panic!(
-                        "Failed to get transaction after {} retries",
-                        MAX_RETRIES
-                    );
-                }
-                tokio::time::sleep(tokio::time::Duration::from_millis(100))
-                    .await;
-            }
-        };
-    };
-    let logs = tx
-        .transaction
-        .meta
-        .as_ref()
-        .unwrap()
-        .log_messages
-        .clone()
-        .unwrap_or_else(Vec::new);
-
-    println!("logs: {:#?}", logs);
-}
-
-#[allow(dead_code)]
-pub async fn tx_logs_contain(
+pub async fn fetch_tx_logs(
     rpc_client: &RpcClient,
     signature: &Signature,
-    needle: &str,
-) -> bool {
+) -> Vec<String> {
     // NOTE: we encountered the following error a few times which makes tests fail for the
     //       wrong reason:
     //       Error {
@@ -156,25 +112,40 @@ pub async fn tx_logs_contain(
             }
         };
     };
-    let logs = tx
-        .transaction
+    tx.transaction
         .meta
         .as_ref()
         .unwrap()
         .log_messages
         .clone()
-        .unwrap_or_else(Vec::new);
-    logs.iter().any(|log| {
-        // Lots of existing tests pass "CommitState" as needle argument to this function, but since now CommitTask
-        // could invoke CommitState or CommitDiff depending on the size of the account, we also look for "CommitDiff"
-        // in the logs when needle == CommitState. It's easier to make this little adjustment here than computing
-        // the decision and passing either CommitState or CommitDiff from the tests themselves.
-        if needle == "CommitState" {
-            log.contains(needle) || log.contains("CommitDiff")
-        } else {
-            log.contains(needle)
-        }
-    })
+        .unwrap_or_else(Vec::new)
+}
+
+#[allow(dead_code)]
+pub async fn print_tx_logs(rpc_client: &RpcClient, signature: &Signature) {
+    println!("logs: {:#?}", fetch_tx_logs(rpc_client, signature).await);
+}
+
+#[allow(dead_code)]
+pub async fn tx_logs_contain(
+    rpc_client: &RpcClient,
+    signature: &Signature,
+    needle: &str,
+) -> bool {
+    fetch_tx_logs(rpc_client, signature)
+        .await
+        .iter()
+        .any(|log| {
+            // Lots of existing tests pass "CommitState" as needle argument to this function, but since now CommitTask
+            // could invoke CommitState or CommitDiff depending on the size of the account, we also look for "CommitDiff"
+            // in the logs when needle == CommitState. It's easier to make this little adjustment here than computing
+            // the decision and passing either CommitState or CommitDiff from the tests themselves.
+            if needle == "CommitState" {
+                log.contains(needle) || log.contains("CommitDiff")
+            } else {
+                log.contains(needle)
+            }
+        })
 }
 
 /// This needs to be run for each test that required a new counter to be delegated