Address code review feedback: enhance workflows, add RPC logging macro, fix error handling

Co-authored-by: larp0 <[email protected]>

Author: Copilot

.github/workflows/audit.yml

@@ -18,8 +18,17 @@ jobs:
       - name: Install Rust
         uses: dtolnay/rust-toolchain@stable
 
+      - name: Cache cargo-audit
+        uses: actions/cache@v3
+        with:
+          path: ~/.cargo/bin/cargo-audit
+          key: ${{ runner.os }}-cargo-audit-${{ hashFiles('~/.cargo/bin/cargo-audit') }}
+        
       - name: Install cargo-audit
-        run: cargo install cargo-audit
+        run: |
+          if [ ! -f ~/.cargo/bin/cargo-audit ]; then
+            cargo install cargo-audit
+          fi
         
       - name: Run cargo-audit
-        run: cargo audit
+        run: cargo audit

.github/workflows/build.yml

@@ -44,8 +44,12 @@ jobs:
         run: |
           cargo build --release --target ${{ matrix.target }}
 
+      - name: Check for dependency drift
+        run: |
+          cargo update --dry-run
+
       - name: Run tests
         run: |
           cargo test --target ${{ matrix.target }}
         # Skip tests for cross-compilation targets that can't run natively
-        if: ${{ !(matrix.os == 'macos-latest' && matrix.target == 'aarch64-apple-darwin' && runner.arch == 'X64') }}
+        if: ${{ !(matrix.os == 'macos-latest' && matrix.target == 'aarch64-apple-darwin' && runner.arch == 'X64') }}

src/lib.rs

@@ -1,5 +1,6 @@
 pub mod config;
 pub mod error;
+#[macro_use]
 pub mod logging;
 pub mod protocol;
 pub mod rpc;

src/logging.rs

@@ -412,6 +412,97 @@ pub fn create_result_summary(result: &Value) -> String {
     }
 }
 
+/// Macro to reduce repetitive boilerplate around timing and logs for RPC calls
+#[macro_export]
+macro_rules! log_rpc_call {
+    ($method:expr, $client:expr, $async_block:expr) => {{
+        let request_id = $crate::logging::new_request_id();
+        let start_time = std::time::Instant::now();
+        
+        $crate::logging::log_rpc_request_start(
+            request_id,
+            $method,
+            Some(&$client.url()),
+            None,
+        );
+
+        match $async_block.await {
+            Ok(result) => {
+                let duration = start_time.elapsed().as_millis() as u64;
+                
+                $crate::logging::log_rpc_request_success(
+                    request_id,
+                    $method,
+                    duration,
+                    Some("request completed"),
+                );
+                
+                Ok(result)
+            }
+            Err(e) => {
+                let duration = start_time.elapsed().as_millis() as u64;
+                let error = $crate::error::McpError::from(e)
+                    .with_request_id(request_id)
+                    .with_method($method)
+                    .with_rpc_url(&$client.url());
+                
+                $crate::logging::log_rpc_request_failure(
+                    request_id,
+                    $method,
+                    duration,
+                    &error.to_string(),
+                    Some("request failed"),
+                );
+                
+                Err(error)
+            }
+        }
+    }};
+    ($method:expr, $client:expr, $async_block:expr, $params:expr) => {{
+        let request_id = $crate::logging::new_request_id();
+        let start_time = std::time::Instant::now();
+        
+        $crate::logging::log_rpc_request_start(
+            request_id,
+            $method,
+            Some(&$client.url()),
+            Some($params),
+        );
+
+        match $async_block.await {
+            Ok(result) => {
+                let duration = start_time.elapsed().as_millis() as u64;
+                
+                $crate::logging::log_rpc_request_success(
+                    request_id,
+                    $method,
+                    duration,
+                    Some("request completed"),
+                );
+                
+                Ok(result)
+            }
+            Err(e) => {
+                let duration = start_time.elapsed().as_millis() as u64;
+                let error = $crate::error::McpError::from(e)
+                    .with_request_id(request_id)
+                    .with_method($method)
+                    .with_rpc_url(&$client.url());
+                
+                $crate::logging::log_rpc_request_failure(
+                    request_id,
+                    $method,
+                    duration,
+                    &error.to_string(),
+                    Some("request failed"),
+                );
+                
+                Err(error)
+            }
+        }
+    }};
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/rpc/accounts.rs

@@ -366,7 +366,7 @@ pub async fn get_program_accounts_with_config(
             min_context_slot: None,
         },
         with_context: None,
-        sort_results: None,
+        sort_results: None, // Use default sorting behavior
     };
     match client.get_program_accounts_with_config(program_id, config).await {
         Ok(accounts) => {
@@ -421,7 +421,7 @@ pub async fn get_largest_accounts(
     let config = solana_client::rpc_config::RpcLargestAccountsConfig {
         commitment: None,
         filter,
-        sort_results: None,
+        sort_results: None, // Use default sorting behavior
     };
 
     match client.get_largest_accounts_with_config(config).await {

src/rpc/system.rs

@@ -464,9 +464,50 @@ pub async fn request_airdrop(
     client: &RpcClient,
     pubkey: &Pubkey,
     lamports: u64,
-) -> anyhow::Result<Value> {
-    let signature = client.request_airdrop(pubkey, lamports).await?;
-    Ok(serde_json::json!({ "signature": signature }))
+) -> McpResult<Value> {
+    let request_id = new_request_id();
+    let start_time = Instant::now();
+    let method = "requestAirdrop";
+    
+    log_rpc_request_start(
+        request_id,
+        method,
+        Some(&client.url()),
+        Some(&format!("pubkey: {}, lamports: {}", pubkey, lamports)),
+    );
+
+    match client.request_airdrop(pubkey, lamports).await {
+        Ok(signature) => {
+            let duration = start_time.elapsed().as_millis() as u64;
+            let result = serde_json::json!({ "signature": signature });
+            
+            log_rpc_request_success(
+                request_id,
+                method,
+                duration,
+                Some("airdrop requested"),
+            );
+            
+            Ok(result)
+        }
+        Err(e) => {
+            let duration = start_time.elapsed().as_millis() as u64;
+            let error = McpError::from(e)
+                .with_request_id(request_id)
+                .with_method(method)
+                .with_rpc_url(&client.url());
+            
+            log_rpc_request_failure(
+                request_id,
+                method,
+                &error.to_string(),
+                duration,
+                None,
+            );
+            
+            Err(error)
+        }
+    }
 }
 
 pub async fn request_airdrop_with_config(