refactor: improved code quality

Author: frolvanya

crates/foreign-chain-config-tester/src/checks.rs

@@ -22,7 +22,7 @@ use foreign_chain_inspector::{
     },
     sui::{
         SuiTransactionDigest,
-        inspector::{SuiFinality, SuiInspector},
+        inspector::{SuiExtractor, SuiFinality, SuiInspector},
     },
 };
 use foreign_chain_rpc_interfaces::aptos::ReqwestAptosClient;
@@ -151,16 +151,21 @@ pub async fn check_sui(client: impl SuiRpcClient, expected_chain_id: &str) -> an
     let tx = golden::base58_32(digest)?;
 
     let inspector = SuiInspector::new(client);
+    // Probe the first event so the extraction pipeline (BCS pass-through, address parsing,
+    // type-tag normalization, contents-name cross-check) is exercised whenever the probe
+    // transaction emits events. A transaction with no events (`LogIndexOutOfBounds`) or a
+    // failed one still proves the provider serves canonical checkpointed data.
     match inspector
         .extract(
             SuiTransactionDigest::from(tx),
             SuiFinality::Checkpointed,
-            vec![],
+            vec![SuiExtractor::Event { event_index: 0 }],
         )
         .await
     {
-        // A failed transaction still proves the provider serves canonical checkpointed data.
-        Ok(_) | Err(ForeignChainInspectionError::TransactionFailed) => Ok(()),
+        Ok(_)
+        | Err(ForeignChainInspectionError::TransactionFailed)
+        | Err(ForeignChainInspectionError::LogIndexOutOfBounds) => Ok(()),
         Err(e) => Err(e).context("failed to inspect a transaction from the latest checkpoint"),
     }
 }

crates/foreign-chain-inspector/src/sui/inspector.rs

@@ -222,7 +222,7 @@ impl SuiExtractor {
                     .value
                     .as_ref()
                     .map(|value| value.to_vec())
-                    .ok_or_else(|| malformed_event_field("bcs contents"))?;
+                    .ok_or_else(|| malformed_event_field("bcs contents value"))?;
 
                 Ok(SuiExtractedValue::Event(SuiEvent {
                     package_id,

crates/foreign-chain-rpc-interfaces/src/sui.rs

@@ -4,27 +4,52 @@ use std::time::Duration;
 use http::{HeaderName, HeaderValue};
 use sui_rpc::field::{FieldMask, FieldMaskUtil as _};
 use sui_rpc::proto::sui::rpc::v2::{
-    GetCheckpointRequest, GetCheckpointResponse, GetServiceInfoRequest, GetServiceInfoResponse,
-    GetTransactionRequest, GetTransactionResponse, get_checkpoint_request::CheckpointId,
+    Checkpoint, ExecutedTransaction, GetCheckpointRequest, GetCheckpointResponse,
+    GetServiceInfoRequest, GetServiceInfoResponse, GetTransactionRequest, GetTransactionResponse,
+    get_checkpoint_request::CheckpointId,
 };
 
 pub use sui_rpc::proto::sui::rpc::v2 as proto;
 pub use tonic::{Code, Status};
 
-/// The exact fields the inspector verifies.
-const TRANSACTION_READ_MASK: &[&str] = &[
-    "digest",
-    "checkpoint",
-    "effects.status",
-    "events.events.package_id",
-    "events.events.module",
-    "events.events.sender",
-    "events.events.event_type",
-    "events.events.contents",
-];
-
-/// Sections needed to pick a probe transaction from a checkpoint.
-const CHECKPOINT_READ_MASK: &[&str] = &["sequence_number", "transactions.digest"];
+fn transaction_read_mask() -> FieldMask {
+    FieldMask::from_paths([
+        ExecutedTransaction::path_builder().digest(),
+        ExecutedTransaction::path_builder().checkpoint(),
+        ExecutedTransaction::path_builder()
+            .effects()
+            .status()
+            .finish(),
+        ExecutedTransaction::path_builder()
+            .events()
+            .events()
+            .package_id(),
+        ExecutedTransaction::path_builder()
+            .events()
+            .events()
+            .module(),
+        ExecutedTransaction::path_builder()
+            .events()
+            .events()
+            .sender(),
+        ExecutedTransaction::path_builder()
+            .events()
+            .events()
+            .event_type(),
+        ExecutedTransaction::path_builder()
+            .events()
+            .events()
+            .contents()
+            .finish(),
+    ])
+}
+
+fn checkpoint_read_mask() -> FieldMask {
+    FieldMask::from_paths([
+        Checkpoint::path_builder().sequence_number(),
+        Checkpoint::path_builder().transactions().digest(),
+    ])
+}
 
 /// Client interface for the Sui gRPC API (`sui.rpc.v2.LedgerService`).
 pub trait SuiRpcClient: Send + Sync {
@@ -87,7 +112,7 @@ impl SuiRpcClient for GrpcSuiClient {
         let request = self.request_with_timeout(
             GetTransactionRequest::default()
                 .with_digest(digest)
-                .with_read_mask(FieldMask::from_paths(TRANSACTION_READ_MASK.iter().copied())),
+                .with_read_mask(transaction_read_mask()),
         );
         async move {
             let response = client.ledger_client().get_transaction(request).await?;
@@ -111,8 +136,7 @@ impl SuiRpcClient for GrpcSuiClient {
         sequence_number: u64,
     ) -> impl Future<Output = Result<GetCheckpointResponse, Status>> + Send {
         let mut client = self.client.clone();
-        let mut message = GetCheckpointRequest::default()
-            .with_read_mask(FieldMask::from_paths(CHECKPOINT_READ_MASK.iter().copied()));
+        let mut message = GetCheckpointRequest::default().with_read_mask(checkpoint_read_mask());
         message.checkpoint_id = Some(CheckpointId::SequenceNumber(sequence_number));
         let request = self.request_with_timeout(message);
         async move {
@@ -166,13 +190,30 @@ mod tests {
 
     #[test]
     fn transaction_read_mask__should_request_only_verified_sections() {
-        // Given / When
-        let request = GetTransactionRequest::default()
-            .with_digest("digest")
-            .with_read_mask(FieldMask::from_paths(TRANSACTION_READ_MASK.iter().copied()));
+        // Then — pins that the typed path builders produce exactly the wire paths the server
+        // honors (the live manual test proves those paths return the expected fields), and that
+        // the events' `json` rendering is not among them.
+        assert_eq!(
+            transaction_read_mask().paths,
+            vec![
+                "digest",
+                "checkpoint",
+                "effects.status",
+                "events.events.package_id",
+                "events.events.module",
+                "events.events.sender",
+                "events.events.event_type",
+                "events.events.contents",
+            ]
+        );
+    }
 
+    #[test]
+    fn checkpoint_read_mask__should_request_only_probe_sections() {
         // Then
-        assert_eq!(request.digest.as_deref(), Some("digest"));
-        assert_eq!(request.read_mask.unwrap().paths, TRANSACTION_READ_MASK);
+        assert_eq!(
+            checkpoint_read_mask().paths,
+            vec!["sequence_number", "transactions.digest"]
+        );
     }
 }

crates/node-config/src/foreign_chains.rs

@@ -77,8 +77,18 @@ impl ForeignChainProviderConfig {
         self.auth.strip_placeholder(&self.rpc_url)
     }
 
-    fn validate_auth_config(&self) -> anyhow::Result<()> {
-        auth::validate_auth_config(&self.auth, &self.rpc_url)
+    fn validate_auth_config(&self, chain: dtos::ForeignChain) -> anyhow::Result<()> {
+        auth::validate_auth_config(&self.auth, &self.rpc_url)?;
+
+        // Sui is reached over gRPC, which carries credentials in request metadata; a token
+        // substituted into the URL path or query would never be sent.
+        if chain == dtos::ForeignChain::Sui {
+            anyhow::ensure!(
+                matches!(self.auth, AuthConfig::None | AuthConfig::Header { .. }),
+                "path or query auth is not supported: gRPC providers support only header auth",
+            );
+        }
+        Ok(())
     }
 }
 
@@ -134,18 +144,10 @@ impl ForeignChainsConfig {
                     rpc_url
                 );
 
-                // valid auth configuration
-                provider.validate_auth_config()?;
-
-                // Sui providers are reached over gRPC, which carries credentials in request
-                // metadata; a token substituted into the URL path or query would never be sent.
-                if identifier == dtos::ForeignChain::Sui {
-                    anyhow::ensure!(
-                        matches!(provider.auth, AuthConfig::None | AuthConfig::Header { .. }),
-                        "sui provider `{}` uses path or query auth, but gRPC providers support only header auth",
-                        provider_name.as_str(),
-                    );
-                }
+                // valid auth configuration for the chain's transport
+                provider.validate_auth_config(identifier).with_context(|| {
+                    format!("provider `{}` has invalid auth", provider_name.as_str())
+                })?;
             }
         }
 
@@ -938,8 +940,9 @@ foreign_chains:
             serde_yaml::from_str(yaml).expect("yaml fixture should be correct");
         let result = config.validate();
 
-        // Then
-        let error = result.unwrap_err().to_string();
+        // Then — the full error chain names the offending provider and the transport rule.
+        let error = format!("{:#}", result.unwrap_err());
+        assert!(error.contains("provider `keyinpath`"), "{error}");
         assert!(error.contains("support only header auth"), "{error}");
     }
 }