Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sdk)!: return consensus errors from broadcast methods #2274

Merged
merged 10 commits into from
Oct 29, 2024

Conversation

shumkov
Copy link
Member

@shumkov shumkov commented Oct 24, 2024

Issue being fixed or feature implemented

#2273

What was done?

  • Refactored DapiClientError to remove generic and all associative types from related traits to simplify error handling
  • Removed CBOR encoding form the drive-error-data-bin gRPC metadata header
  • Recovered and returned consensus error in the DAPI Client Error to SDK error conversion trait implementation

How Has This Been Tested?

Updated SDK tests

Breaking Changes

  • SDK methods which broadcasting transactions will return SdkError::Protocol(ProtocolError::Consensus(..)) instead of DapiClientError(String) in case of validation errors.
  • Serialized consensus error moved to a separate gRPC header dash-serialized-consensus-error-bin

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Summary by CodeRabbit

  • New Features
    • Introduced a new method get_identity_contract_nonce in the SDK for retrieving or updating identity contract nonces.
  • Improvements
    • Simplified error handling across various components by consolidating error types to TransportError and DapiClientError.
    • Enhanced error handling in gRPC transport methods for better clarity and consistency.
  • Tests
    • Added new test cases and improved existing ones for better validation of error handling and query parameters in contested resource vote states.

Copy link
Contributor

coderabbitai bot commented Oct 24, 2024

Walkthrough

The pull request modifies the error handling mechanisms across several components of the DAPI client and SDK. Key changes include the simplification of the DapiClientError enum by removing generic type parameters, the introduction of a new TransportError type for centralized error handling, and updates to method signatures across various traits to reflect these changes. The modifications enhance consistency and clarity in error reporting throughout the DAPI client and related SDK components.

Changes

File Path Change Summary
packages/rs-dapi-client/src/dapi_client.rs Updated DapiClientError enum to use TransportError instead of a generic type. Simplified can_retry and execute method signatures. Removed TransportErrorData struct.
packages/rs-dapi-client/src/lib.rs Modified DapiRequest trait by removing associated TransportError type and simplifying execute method return type.
packages/rs-dapi-client/src/mock.rs Updated MockResult<R> type alias to use DapiClientError. Modified load method signature to remove TransportRequest constraint.
packages/rs-dapi-client/src/transport.rs Introduced TransportError enum for centralized error handling. Updated execute_transport and TransportClient methods to return TransportError.
packages/rs-dapi-client/src/transport/grpc.rs Changed error type in PlatformGrpcClient and CoreGrpcClient from dapi_grpc::tonic::Status to TransportError. Updated error handling in execute_transport method.
packages/rs-sdk/src/error.rs Added a new variant for DapiClientError in Error enum. Updated From trait implementation for deserializing consensus errors from gRPC transport errors.
packages/rs-sdk/src/platform/delegate.rs Modified execute_transport return type to Result<Self::Response, TransportError>.
packages/rs-sdk/src/platform/document_query.rs Updated execute_transport method to return TransportError.
packages/rs-sdk/src/platform/identities_contract_keys_query.rs Changed execute_transport method to return TransportError.
packages/rs-sdk/src/platform/transition/put_identity.rs Updated error handling for put_to_platform_and_wait_for_response method to handle TransportError::Grpc.
packages/rs-sdk/src/platform/types/evonode.rs Changed execute_transport error type to TransportError.
packages/rs-sdk/src/platform/types/identity.rs Added import for TransportError.
packages/rs-sdk/src/sdk.rs Introduced get_identity_contract_nonce method. Simplified execute method return type in DapiRequestExecutor implementation.
packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js Modified error handling logic to utilize a new consensusMetadata object.
packages/js-dapi-client/lib/transport/GrpcTransport/createGrpcTransportError.js Enhanced error handling for consensus errors, checking for necessary metadata before deserialization.
packages/dapi/test/unit/grpcServer/handlers/createGrpcErrorFromDriveResponse.spec.js Updated test cases to reflect changes in error handling and metadata.
packages/js-dapi-client/test/unit/transport/GrpcTransport/createGrpcTransportError.spec.js Modified tests to handle new error data serialization for InvalidRequestDPPError.
packages/rs-dapi-client/src/executor.rs Simplified execute method return type in DapiRequestExecutor trait to DapiClientError.
packages/rs-sdk/tests/fetch/contested_resource_vote_state.rs Enhanced test cases for querying contested resource vote states with new and updated tests for error handling.

Possibly related PRs

Suggested labels

enhancement, js-sdk, rs-sdk

Suggested reviewers

  • QuantumExplorer

🐇 In the meadow, changes bloom,
Error handling sheds its gloom.
Transport errors now align,
Clarity in code, so fine!
With each hop, we simplify,
Making errors easier to identify! 🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@shumkov shumkov mentioned this pull request Oct 25, 2024
6 tasks
@shumkov shumkov linked an issue Oct 25, 2024 that may be closed by this pull request
@shumkov shumkov marked this pull request as ready for review October 25, 2024 07:53
@shumkov shumkov added this to the 1.4.2 milestone Oct 25, 2024
@shumkov shumkov changed the title feat(sdk): return consensus errors from broadcast methods feat(sdk)!: return consensus errors from broadcast methods Oct 25, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (9)
packages/rs-sdk/src/platform/identities_contract_keys_query.rs (1)

84-84: Consider improving error handling in the implementation.

While the change to use TransportError is good and aligns with the SDK's error handling standardization, the implementation uses expect() which could panic. Consider propagating the error instead.

Here's a suggested improvement:

    fn execute_transport<'c>(
        self,
        client: &'c mut Self::Client,
        settings: &AppliedRequestSettings,
    ) -> BoxFuture<'c, Result<Self::Response, TransportError>> {
-        let request: GetIdentitiesContractKeysRequest = self
-            .try_into()
-            .expect("IdentitiesContractKeysQuery should always be valid");
+        let request: GetIdentitiesContractKeysRequest = self.try_into().map_err(|e| {
+            TransportError::Internal(format!("Failed to convert IdentitiesContractKeysQuery: {}", e))
+        })?;
        request.execute_transport(client, settings)
    }
packages/rs-sdk/src/platform/transition/put_identity.rs (1)

Line range hint 90-99: Consider enhancing error handling reliability

The TODO comment suggests that the error handling could be more reliable. Consider the following improvements:

  1. Add structured logging for all error cases, not just the AlreadyExists case
  2. Make the error message more specific about why the identity state is inconsistent
  3. Consider adding retry logic for transient errors

Example improvement:

 Err(DapiClientError::Transport(TransportError::Grpc(te), _))
     if te.code() == Code::AlreadyExists =>
 {
+    tracing::info!(
+        ?identity_id,
+        error_code = ?te.code(),
+        "handling identity creation conflict"
+    );
     tracing::debug!(
         ?identity_id,
         "attempt to create identity that already exists"
     );
     let identity = Identity::fetch(sdk, identity_id).await?;
     return identity.ok_or(Error::DapiClientError(
-        "identity was proved to not exist but was said to exist".to_string(),
+        format!("Identity {} state is inconsistent: proved non-existent but reported as existing", identity_id),
     ));
 }
-Err(e) => return Err(e.into()),
+Err(e) => {
+    tracing::error!(
+        ?identity_id,
+        error = ?e,
+        "failed to create identity"
+    );
+    return Err(e.into());
+}
packages/rs-sdk/src/platform/delegate.rs (1)

Line range hint 11-52: Consider updating documentation for error handling changes.

The standardization of error handling using TransportError improves the codebase consistency. Consider:

  1. Updating API documentation to reflect the new error handling approach
  2. Adding examples of error handling patterns with TransportError
  3. Documenting any migration steps for consumers of the SDK
packages/rs-sdk/src/platform/fetch.rs (2)

Line range hint 156-167: Consider adding error context for better debugging.

In the error handling for the fetch_with_metadata method, we could enhance the error context by including the object type and query details when logging errors. This would help with debugging consensus-related issues.

     let response = request
         .clone()
         .execute(sdk, settings.unwrap_or_default())
-        .await?;
+        .await
+        .map_err(|e| {
+            tracing::error!(
+                error = ?e,
+                object_type = object_type,
+                request = ?request,
+                "failed to fetch object from platform"
+            );
+            e
+        })?;

Line range hint 1-285: Consider adding consensus error documentation.

Since this PR focuses on returning consensus errors from broadcast methods, we should enhance the documentation to explain how consensus errors are handled and propagated through the Fetch trait implementations.

Add documentation about consensus error handling to the trait documentation:

 /// Trait implemented by objects that can be fetched from Platform.
 ///
+/// # Error Handling
+/// 
+/// This trait handles various types of errors that may occur during fetching:
+/// - Transport errors from network communication
+/// - Consensus errors from the platform
+/// - Proof verification errors
+/// 
+/// Consensus errors are propagated from the underlying DAPI client and indicate 
+/// issues with the platform's consensus mechanism, such as:
+/// - Invalid state transitions
+/// - Insufficient confirmations
+/// - Chain reorganizations
packages/rs-dapi-client/src/transport/grpc.rs (1)

Line range hint 42-75: Consider updating error handling in channel creation

While the return type has been updated to use TransportError, the channel creation error is still being converted to tonic::Status. Consider mapping the channel creation error directly to TransportError for consistency.

 fn with_uri(uri: Uri, pool: &ConnectionPool) -> Result<Self, TransportError> {
     Ok(pool
         .get_or_create(PoolPrefix::Platform, &uri, None, || {
             match create_channel(uri.clone(), None) {
                 Ok(channel) => Ok(Self::new(channel).into()),
-                Err(e) => Err(dapi_grpc::tonic::Status::failed_precondition(format!(
-                    "Channel creation failed: {}",
-                    e
-                ))),
+                Err(e) => Err(TransportError::Connection(format!(
+                    "Channel creation failed: {}",
+                    e
+                ))),
             }
         })?
         .into())
 }

Similar change should be applied to with_uri_and_settings method.

packages/rs-sdk/src/sdk.rs (1)

Line range hint 421-486: Consider extracting nonce management into a dedicated struct.

The implementation looks correct and follows the same pattern as get_identity_nonce. However, as noted in the TODO comment, this functionality should be moved to a separate struct to improve modularity and maintainability.

Consider creating a dedicated NonceManager struct:

pub struct NonceManager {
    identity_nonce_counter: Arc<Mutex<BTreeMap<Identifier, (IdentityNonce, LastQueryTimestamp)>>>,
    identity_contract_nonce_counter: Arc<Mutex<BTreeMap<(Identifier, Identifier), (IdentityNonce, LastQueryTimestamp)>>>,
}

impl NonceManager {
    pub async fn get_identity_nonce(&self, ...) -> Result<IdentityNonce, Error> {
        // existing implementation
    }
    
    pub async fn get_identity_contract_nonce(&self, ...) -> Result<IdentityNonce, Error> {
        // existing implementation
    }
}
packages/rs-dapi-client/src/transport.rs (1)

Line range hint 41-49: Consider adding more variants to TransportError for comprehensive error handling.

Currently, TransportError has only one variant Grpc. If other types of transport errors can occur (e.g., timeouts, connection errors), consider adding additional variants to handle them explicitly.

packages/rs-sdk/src/error.rs (1)

77-77: Address the TODO: Implement more specific errors

The TODO comment on line 77 suggests returning more specific errors like connection or node errors instead of the generic DapiClientError. Implementing detailed error types will improve error handling and make debugging more efficient.

Would you like assistance in defining and implementing these specific error types?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between e0e152f and dd785cc.

📒 Files selected for processing (14)
  • packages/rs-dapi-client/src/dapi_client.rs (10 hunks)
  • packages/rs-dapi-client/src/lib.rs (2 hunks)
  • packages/rs-dapi-client/src/mock.rs (1 hunks)
  • packages/rs-dapi-client/src/transport.rs (1 hunks)
  • packages/rs-dapi-client/src/transport/grpc.rs (7 hunks)
  • packages/rs-sdk/src/error.rs (2 hunks)
  • packages/rs-sdk/src/platform/delegate.rs (2 hunks)
  • packages/rs-sdk/src/platform/document_query.rs (2 hunks)
  • packages/rs-sdk/src/platform/fetch.rs (1 hunks)
  • packages/rs-sdk/src/platform/identities_contract_keys_query.rs (2 hunks)
  • packages/rs-sdk/src/platform/transition/put_identity.rs (2 hunks)
  • packages/rs-sdk/src/platform/types/evonode.rs (4 hunks)
  • packages/rs-sdk/src/platform/types/identity.rs (1 hunks)
  • packages/rs-sdk/src/sdk.rs (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/rs-sdk/src/platform/types/identity.rs
🧰 Additional context used
📓 Learnings (1)
packages/rs-dapi-client/src/transport.rs (1)
Learnt from: shumkov
PR: dashpay/platform#2259
File: packages/rs-dapi-client/src/transport.rs:51-51
Timestamp: 2024-10-22T10:51:16.910Z
Learning: In implementations of `TransportClient`, only `DapiClientError` is used as the `Error` type, and it implements `std::error::Error`.
🔇 Additional comments (34)
packages/rs-dapi-client/src/lib.rs (2)

36-36: LGTM: Documentation example updated correctly.

The example has been properly updated to reflect the simplified error type, maintaining consistency with the new error handling approach.


61-61: LGTM: Implementation correctly updated.

The implementation has been properly updated to match the trait's new signature while maintaining clean separation of concerns and async compatibility.

packages/rs-sdk/src/platform/identities_contract_keys_query.rs (1)

6-6: LGTM: Transport error type import added.

The addition of TransportError to the imports aligns with the standardization of error handling across the SDK.

packages/rs-sdk/src/platform/types/evonode.rs (4)

11-11: LGTM: Import changes align with error handling improvements

The addition of TransportError to the imports is consistent with the PR's objective of standardizing error handling across the SDK.


101-101: LGTM: Clear and explicit error mapping

The explicit mapping of gRPC errors to TransportError using map_err follows Rust's error handling best practices and improves error handling clarity.


Line range hint 69-107: LGTM: Well-structured implementation with proper resource management

The implementation demonstrates good practices:

  • Comprehensive error handling
  • Proper resource cleanup with explicit drops
  • Clear and predictable error types
  • Appropriate timeout handling

69-69: Verify impact of error type change

The change from a generic error type to TransportError improves type safety and error handling clarity. However, this could be a breaking change for consumers relying on the generic error type.

Let's verify the impact:

✅ Verification successful

Change to TransportError is safe and consistent with the codebase

The change from a generic error type to TransportError is verified to be safe because:

  • All implementations of execute_transport in the codebase already use TransportError in their return type:

    • identities_contract_keys_query.rs
    • delegate.rs
    • document_query.rs
    • evonode.rs
  • TransportError is a well-established type in the codebase, used consistently across the transport layer implementation and error handling paths.

  • The error type is properly integrated with the error handling infrastructure, including retry mechanisms (CanRetry trait) and error conversion utilities.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any custom implementations of TransportRequest that might be affected
ast-grep --pattern 'impl TransportRequest for $type {
  $$$
}'

# Search for direct usage of the execute_transport method with error handling
rg -A 5 'execute_transport.*Result.*Error'

Length of output: 109


Script:

#!/bin/bash
# Let's try a broader search for TransportRequest implementations
ast-grep --pattern 'impl $_ for $_ {
    fn execute_transport'

# Search for any usage of execute_transport
rg "execute_transport"

# Search for TransportError usage
rg "TransportError"

# Look for any struct implementing Transport trait
ast-grep --pattern 'impl Transport for $_ {'

Length of output: 17199

packages/rs-sdk/src/platform/transition/put_identity.rs (2)

17-17: LGTM: TransportError import aligns with error handling improvements

The addition of this import supports the standardization of transport-related error handling across the SDK.


90-92: LGTM: Improved error handling specificity

The error pattern matching has been refined to specifically handle gRPC transport errors, which aligns with the PR's objective of improving consensus error handling.

packages/rs-dapi-client/src/mock.rs (1)

38-38: LGTM! Simplified error handling.

The simplification of MockResult<T> by removing the nested error type parameter makes the code more maintainable while aligning with the broader error handling improvements across the codebase.

Let's verify that all usages of MockResult have been updated to handle the simplified error type:

✅ Verification successful

Based on the search results, I can now generate the final response:

Verification successful: Error type simplification is complete

The codebase scan confirms that:

  • No instances of the old complex error type pattern DapiClientError<.*Error> remain
  • All usages of MockResult have been properly updated to work with the simplified error type
  • The changes are consistently applied across both the mock implementation and dump functionality
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining usages of the old error type pattern
# and verify that all mock implementations properly handle the new error type.

# Search for any remaining complex error type patterns
rg -l "DapiClientError<.*Error>" .

# Find all uses of MockResult to verify proper error handling
rg "MockResult" -A 3 -B 3

Length of output: 4345

packages/rs-sdk/src/platform/delegate.rs (3)

11-11: LGTM: Import aligns with error handling standardization.

The addition of TransportError import supports the standardization of error handling across the transport layer.


52-52: Skip review of formatting change.


48-48: Verify impact of error type change on existing error handlers.

The change from client-specific error type to TransportError standardizes error handling but may affect code that handles specific client errors.

✅ Verification successful

Based on the search results, I can see that the codebase primarily uses generic error handling patterns through map_err transformations, and there are no direct matches or error handling patterns specifically tied to client-specific error types. The error handling is already abstracted through various error enums and traits.

The change to TransportError aligns with existing error handling patterns

The standardization to TransportError appears safe as:

  • Most error handling uses generic error transformations via map_err
  • Errors are typically converted to higher-level error types (e.g., ContextProviderError, ProtocolError)
  • No evidence of direct matching or handling of client-specific transport errors
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for existing error handling code that might be affected
# Look for match expressions or error handling involving the old client error type

# Search for potential error handling patterns
rg -A 5 "match.*Error" packages/rs-sdk/
rg -A 5 "\.map_err" packages/rs-sdk/

Length of output: 14307

packages/rs-sdk/src/platform/fetch.rs (1)

20-20: Verify error propagation from DapiClientError to Error type.

The addition of DapiClientError to imports suggests changes in error handling. However, I notice that the Fetch trait methods still return the generic Error type. We should ensure proper error conversion exists between DapiClientError and the returned Error type.

Let's verify the error handling implementation:

✅ Verification successful

Let me gather more information about the error handling implementation to ensure completeness of the verification.


Let me check one more aspect to see how the execute method handles errors from the client.


Error handling implementation is properly set up

The verification shows that proper error handling is in place:

  • A From<DapiClientError> implementation exists in error.rs that handles the conversion of DapiClientError to the SDK's Error type
  • The implementation includes special handling for consensus errors through the drive-error-data-bin metadata
  • The ? operator in the codebase will automatically use this conversion when propagating errors
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check error handling implementation
# Test 1: Look for From/Into implementations between DapiClientError and Error
rg -A 5 "impl.*(?:From|Into)<DapiClientError>.*for.*Error"

# Test 2: Look for error conversion in execute calls
ast-grep --pattern 'execute($_, $_).await?.into()'

Length of output: 684


Script:

#!/bin/bash
# Check for error handling in the fetch.rs file
rg -A 10 "DapiClientError" "packages/rs-sdk/src/platform/fetch.rs"

# Look for execute method implementations
ast-grep --pattern 'async fn execute($$$) -> Result<$_, $_> {
  $$$
}'

# Check for any direct error conversions in fetch.rs
rg -A 5 "\.into\(\)" "packages/rs-sdk/src/platform/fetch.rs"

Length of output: 1029


Script:

#!/bin/bash
# Check the implementation of execute method and error handling
rg -B 5 -A 15 "async fn execute" "packages/rs-sdk/src/platform/fetch.rs"

# Look for any error mapping or conversion in the file
rg "map_err|\.into\(\)|\.map\(" "packages/rs-sdk/src/platform/fetch.rs"

Length of output: 279

packages/rs-sdk/src/platform/document_query.rs (2)

27-27: LGTM: Import change aligns with error handling standardization.

The addition of TransportError to the imports is consistent with the PR's goal of standardizing error handling across the SDK.


151-151: LGTM: Improved error handling standardization.

The change from a client-specific error type to TransportError in the execute_transport method signature is a good improvement that:

  • Standardizes error handling across transport operations
  • Maintains backward compatibility while providing a more unified error interface
packages/rs-dapi-client/src/transport/grpc.rs (4)

5-5: LGTM: Import changes align with error handling standardization

The addition of TransportError import supports the PR's objective to consolidate error handling across the transport layer.


Line range hint 79-112: Apply consistent error handling updates

The same error handling improvements suggested for PlatformGrpcClient should be applied here for consistency.


Line range hint 154-164: LGTM: Proper error mapping in execute_transport

The changes correctly map gRPC errors to TransportError::Grpc, maintaining the original error information while standardizing the error type across the transport layer.


Line range hint 170-400: LGTM: Consistent error handling across all requests

The macro changes automatically propagate the improved error handling to all gRPC request implementations, ensuring consistent error handling across the entire transport layer.

packages/rs-sdk/src/sdk.rs (1)

676-676: Breaking change: Simplified error handling in execute method.

The return type change from DapiClientError<<R::Client as TransportClient>::Error> to DapiClientError aligns with the PR objective to enhance error handling by consolidating error types.

Let's verify the impact of this change:

✅ Verification successful

The error type change is safe and aligns with the codebase

Based on the search results, the change from DapiClientError<<R::Client as TransportClient>::Error> to DapiClientError is correct and consistent with the codebase for these reasons:

  1. The DapiClientError type is already used without generic parameters throughout the codebase, as seen in function signatures and error handling.
  2. All error handling implementations, including the From implementations and error propagation, use the non-generic DapiClientError type.
  3. The transport error is already handled within DapiClientError's Transport variant, which encapsulates the transport-specific errors internally.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any custom error handling of the generic type parameter
rg -A 5 "DapiClientError<" 

Length of output: 28


Script:

#!/bin/bash
# Let's search for the error type definition and its usages
rg "DapiClientError" -A 5

# Also search for any implementations or traits related to this error type
ast-grep --pattern 'impl $_ for DapiClientError { $$ }'

# And check for any type aliases or uses in function signatures
ast-grep --pattern 'type $_ = $$$DapiClientError<$_>'
ast-grep --pattern 'fn $_($_) -> Result<$_, DapiClientError'

Length of output: 20452

packages/rs-dapi-client/src/transport.rs (3)

60-69: Ensure Mockable implementation covers all variants of TransportError.

The manual serialization and deserialization in the Mockable implementation should correctly handle all current and future variants of TransportError. Please verify that this implementation remains accurate if more variants are added.


Line range hint 36-36: Ensure all TransportRequest implementations use the new TransportError.

The execute_transport method now returns Result<Self::Response, TransportError>. Please verify that all implementations of TransportRequest have updated their execute_transport methods to return TransportError instead of the previous error type.

Run the following script to check the implementations:

#!/bin/bash
# Description: Verify that all `execute_transport` methods return `TransportError`.

ast-grep --pattern $'impl TransportRequest for $_ {
  $$$
  fn execute_transport<$$_>(
    $$_,
    &mut $_,
    &AppliedRequestSettings,
  ) -> BoxFuture<$$_, Result<$$_, TransportError>> {
    $$$
  }
  $$$
}'

73-79: Update all TransportClient implementations to return TransportError.

The methods with_uri and with_uri_and_settings now return Result<Self, TransportError>. Please ensure that all implementations of TransportClient have updated their return types accordingly.

Run the following script to check the implementations:

✅ Verification successful

Let me search for any other implementations of TransportClient to ensure we have a complete view.


All TransportClient implementations correctly return TransportError

Both implementations of TransportClient (PlatformGrpcClient and CoreGrpcClient) already have their with_uri and with_uri_and_settings methods returning Result<Self, TransportError> as required.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all `TransportClient` implementations return `TransportError`.

ast-grep --pattern $'impl TransportClient for $_ {
  $$$
  fn with_uri($_, $_) -> Result<Self, TransportError> {
    $$$
  }
  $$$
  fn with_uri_and_settings($_, $_, $_) -> Result<Self, TransportError> {
    $$$
  }
  $$$
}'

Length of output: 6325


Script:

#!/bin/bash
# Search for all TransportClient implementations
ast-grep --pattern 'impl TransportClient for $_'

Length of output: 6150

packages/rs-sdk/src/error.rs (2)

80-80: Verify pattern matching in error conversion

Please verify that the pattern matching in if let DapiClientError::Transport(TransportError::Grpc(status), _) correctly reflects the structure of DapiClientError::Transport. Ensure that the extra argument (_) is appropriate and that no critical error information is being discarded.


78-91: ⚠️ Potential issue

Ensure comprehensive handling of all DapiClientError variants

In the From<DapiClientError> for Error implementation, only the TransportError::Grpc(status) variant is specifically handled. Consider whether other variants of DapiClientError should also be addressed to prevent loss of important error information and ensure all error cases are properly converted.

packages/rs-dapi-client/src/dapi_client.rs (8)

22-27: Simplify DapiClientError by removing the generic type parameter

The removal of the generic type parameter <TE: Mockable> from the DapiClientError enum simplifies error handling and reduces complexity. This change improves maintainability by consolidating error types.


Line range hint 43-50: Update CanRetry implementation to match the simplified DapiClientError

The impl CanRetry for DapiClientError has been updated to reflect the removal of the generic parameter, ensuring consistency in error handling.


Line range hint 59-68: Adjust Mockable implementation for DapiClientError

Updating impl Mockable for DapiClientError to remove the generic constraint aligns with the simplified error enum. This change streamlines serialization and deserialization processes during mocking.


79-83: Ensure TransportError implements the Mockable trait

At line 83, the constraint TransportError: Mockable is added. Verify that TransportError correctly implements the Mockable trait to maintain compatibility with the mocking framework.


124-128: Update execute method signature and trait bounds

The execute method now returns Result<R::Response, DapiClientError> without the generic transport error type. The addition of TransportError: Mockable ensures that transport errors can be serialized during testing. Confirm that all implementations remain compatible with these changes.


161-164: Handle the absence of live addresses appropriately

The use of ok_or(DapiClientError::NoAvailableAddresses) when retrieving a live address ensures that the client properly reports when no addresses are available, preventing potential panics or undefined behavior.


199-204: Verify comprehensive error information in TransportError

When mapping errors using DapiClientError::Transport(e, address.clone()), ensure that TransportError provides sufficient details for debugging and that no critical error information is lost during the mapping.


215-217: Confirm correct error mapping in ban/unban operations

Errors from ban_address and unban_address are mapped to DapiClientError::AddressList. Verify that any issues in these operations are accurately captured and that error handling remains consistent.

Also applies to: 230-232

packages/rs-dapi-client/src/lib.rs Outdated Show resolved Hide resolved
packages/rs-sdk/src/error.rs Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (4)
packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js (2)

120-121: Add missing trailing comma for consistency.

   const consensusMetadata = {
     code,
     ...createRawMetadata(data),
-    'dash-serialized-consensus-error-bin': serializedConsensusError
+    'dash-serialized-consensus-error-bin': serializedConsensusError,
   };
🧰 Tools
🪛 GitHub Check: JS packages (@dashevo/dapi) / Linting

[failure] 120-120:
Missing trailing comma


Line range hint 1-171: Document breaking changes in error handling.

This implementation introduces significant changes to error handling, particularly in how consensus errors are propagated. To ensure a smooth transition:

  1. Update SDK documentation to reflect that broadcast methods now return SdkError::Protocol(ProtocolError::Consensus(..)).
  2. Consider adding migration guides for users upgrading from previous versions.
  3. Update any relevant error handling examples in the documentation.
packages/rs-sdk/src/error.rs (2)

77-78: Consider tracking the TODO as a separate issue.

The TODO comment indicates plans to decompose DapiClientError into more specific errors. This would improve error handling granularity.

Would you like me to create a GitHub issue to track this future improvement?


78-94: Document the breaking change in error conversion.

The implementation correctly handles consensus errors from gRPC metadata, which is a breaking change as mentioned in the PR objectives. Consider adding documentation to highlight this behavior change.

Add documentation above the impl block:

/// Converts DapiClientError into SDK Error.
/// 
/// # Breaking Changes
/// Now returns `Error::Protocol(ProtocolError::ConsensusError(..))` when consensus
/// errors are present in the gRPC metadata, instead of wrapping them in a generic
/// `DapiClientError` string.
impl From<DapiClientError> for Error {
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between dd785cc and 7bdda0f.

📒 Files selected for processing (4)
  • packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js (4 hunks)
  • packages/js-dapi-client/lib/transport/GrpcTransport/createGrpcTransportError.js (1 hunks)
  • packages/rs-dapi-client/src/mock.rs (3 hunks)
  • packages/rs-sdk/src/error.rs (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/rs-dapi-client/src/mock.rs
🧰 Additional context used
🪛 Biome
packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js

[error] 68-68: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

🪛 GitHub Check: JS packages (@dashevo/dapi) / Linting
packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js

[failure] 120-120:
Missing trailing comma

🔇 Additional comments (5)
packages/js-dapi-client/lib/transport/GrpcTransport/createGrpcTransportError.js (2)

Line range hint 130-132: Verify cleanup of serializedError.

The code removes data.serializedError after deserialization. Let's ensure this cleanup is complete and doesn't affect other code.

Run this script to check for any remaining references:

#!/bin/bash
# Description: Check for any remaining references to serializedError
# Test 1: Search for direct property access
rg "data\.serializedError"
# Test 2: Search for string references
rg '"serializedError"'
# Test 3: Search for object destructuring
rg "{\s*serializedError\s*}"
🧰 Tools
🪛 Biome

[error] 130-130: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


122-128: LGTM! Robust error handling for consensus errors.

The implementation correctly handles the deserialization of consensus errors with proper validation.

Let's verify the consistency of the consensus error metadata field name across the codebase:

✅ Verification successful

Let's search for more specific patterns to understand how consensus errors are handled across the transport layer.


Metadata field name is consistently used across the codebase

The verification confirms that the metadata field name 'dash-serialized-consensus-error-bin' is consistently used across different parts of the system:

  • JavaScript client (js-dapi-client) for deserializing errors
  • Rust SDK for error handling
  • DAPI server for error response creation

The implementation aligns with the cross-language serialization requirements and maintains consistency in the transport layer.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the consensus error metadata field name is used consistently
# Test: Search for any references to the metadata field name
rg "dash-serialized-consensus-error-bin"

Length of output: 1037


Script:

#!/bin/bash
# Search for the metadata field name in the codebase
rg -A 2 "dash-serialized-consensus-error-bin"

# Search for consensus error serialization/deserialization related code
ast-grep --pattern 'deserializeConsensusError'

# Search for metadata field setting in gRPC handlers
rg -A 3 "metadata\[.*\].*=.*" 

Length of output: 1699

packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js (2)

Line range hint 140-165: LGTM! Well-structured error handling implementation.

The error handling implementation is comprehensive with:

  • Clear error code ranges for different types of errors
  • Proper inclusion of consensus metadata
  • Appropriate error logging for deserialization failures

117-122: Verify the new metadata structure compatibility.

The new metadata structure with dash-serialized-consensus-error-bin aligns with the PR objective to return consensus errors. However, we should verify that all consumers can handle this new format.

✅ Verification successful

Let me check the consumers to verify compatibility with the new metadata structure.


The new metadata structure is compatible with existing consumers

The verification shows that the js-dapi-client already handles both metadata fields correctly:

  • drive-error-data-bin is properly decoded from base64 using CBOR
  • dash-serialized-consensus-error-bin is properly deserialized into consensus errors
  • The code field is properly extracted from metadata
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for consumers of the error metadata to ensure compatibility
rg -l 'drive-error-data-bin|dash-serialized-consensus-error-bin' --type js

Length of output: 406


Script:

#!/bin/bash
# Check how the metadata is handled in js-dapi-client
rg -A 10 'drive-error-data-bin|dash-serialized-consensus-error-bin' packages/js-dapi-client/lib/transport/GrpcTransport/createGrpcTransportError.js

Length of output: 938

🧰 Tools
🪛 GitHub Check: JS packages (@dashevo/dapi) / Linting

[failure] 120-120:
Missing trailing comma

packages/rs-sdk/src/error.rs (1)

2-11: LGTM: Import changes align with the new error handling approach.

The new imports support the consensus error handling capabilities while removing unnecessary generic types.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (4)
packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js (1)

Line range hint 124-134: Consider wrapping deserialization errors.

While the error logging is good, throwing the raw deserialization error might lead to unexpected error types in the gRPC response. Consider wrapping it in an InternalGrpcError.

     } catch (e) {
       logger.error({
         err: e,
         data: data.serializedError,
         code,
       }, `Failed to deserialize consensus error with code ${code}: ${e.message}`);
 
-      throw e;
+      throw new InternalGrpcError(e);
     }
packages/dapi/test/unit/grpcServer/handlers/createGrpcErrorFromDriveResponse.spec.js (1)

130-131: Consider enhancing test coverage for state consensus errors.

While the implementation of the new serialization approach is correct, consider adding assertions to verify:

  1. The error message matches the consensus error message
  2. The error code is correct

Add these assertions to improve test coverage:

     expect(error).to.be.an.instanceOf(InvalidArgumentGrpcError);
+    expect(error.message).to.be.equals(consensusError.message);
+    expect(error.getCode()).to.equal(GrpcErrorCodes.INVALID_ARGUMENT);
     expect(error.getRawMetadata()).to.deep.equal({
       code: 40000,
       'dash-serialized-consensus-error-bin': serializedError,
     });

Also applies to: 142-142

packages/js-dapi-client/test/unit/transport/GrpcTransport/createGrpcTransportError.spec.js (2)

158-165: LGTM! Changes align well with PR objectives.

The implementation correctly:

  • Removes CBOR encoding in favor of direct error serialization
  • Uses the new 'dash-serialized-consensus-error-bin' metadata key
  • Maintains browser compatibility with base64 encoding

Consider adding test cases for edge scenarios:

it('should handle empty/corrupted serialized consensus error', async () => {
  metadata.set('dash-serialized-consensus-error-bin', '');
  // Test implementation
});

it('should handle invalid base64 in web environment', async () => {
  metadata.set('dash-serialized-consensus-error-bin', 'invalid-base64');
  // Test implementation
});

161-164: Consider tracking the TODO for future cleanup.

The TODO comment indicates technical debt regarding the dual gRPC implementation support.

Would you like me to create a GitHub issue to track the cleanup of dual gRPC implementation support?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 7bdda0f and 3ca8abe.

📒 Files selected for processing (3)
  • packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js (4 hunks)
  • packages/dapi/test/unit/grpcServer/handlers/createGrpcErrorFromDriveResponse.spec.js (6 hunks)
  • packages/js-dapi-client/test/unit/transport/GrpcTransport/createGrpcTransportError.spec.js (1 hunks)
🧰 Additional context used
🪛 Biome
packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js

[error] 68-68: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

🔇 Additional comments (7)
packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js (4)

67-69: Previous review comment about delete operator is still applicable.

The suggestion to use object destructuring instead of the delete operator remains valid for better performance.

🧰 Tools
🪛 Biome

[error] 68-68: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)


117-122: Well-structured metadata handling!

The new consensusMetadata structure effectively combines error code, raw metadata, and serialized consensus error, aligning with the PR's objective to return consensus errors.


Line range hint 139-166: Clean and consistent error handling implementation!

The error responses are well-structured with:

  • Consistent use of consensusMetadata across different error types
  • Clear error code range segregation
  • Proper mapping to appropriate gRPC error types

Line range hint 1-171: Verify broadcast methods integration.

The changes align well with the PR objectives. Let's verify that all broadcast methods are properly integrated with this new error handling approach.

✅ Verification successful

Let me search for the actual broadcast transaction handlers to verify their integration with the new error handling.


Let me check the broadcast transaction handler implementation to verify its error handling integration.


Based on the review of the code and test files, I can now provide the final response:

The broadcast transaction handler is properly integrated with error handling

The implementation in broadcastTransactionHandlerFactory.js correctly handles various error scenarios:

  • Input validation errors using InvalidArgumentGrpcError
  • Transaction verification errors using InvalidArgumentGrpcError
  • Chain-related errors using AlreadyExistsGrpcError and FailedPreconditionGrpcError

The error handling is well-tested as shown in the test file, and the implementation aligns with the new error handling approach introduced in the PR.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for broadcast method implementations to ensure they handle the new consensus errors
rg -l "broadcast.*Transaction" | xargs rg "DapiClientError|ProtocolError|Consensus"

# Search for any remaining uses of the old drive-error-data-bin metadata key
rg "drive-error-data-bin"

Length of output: 10222


Script:

#!/bin/bash
# Search for broadcast transaction handler implementations
ast-grep --pattern 'async function broadcastTransaction($$$) {
  $$$
}'

# Also search for any files containing broadcastTransaction in their name
fd broadcastTransaction

Length of output: 10250


Script:

#!/bin/bash
# Check the implementation of broadcast transaction handler
cat packages/dapi/lib/grpcServer/handlers/core/broadcastTransactionHandlerFactory.js

# Also check if there are any tests verifying error handling
cat packages/dapi/test/unit/grpcServer/handlers/core/broadcastTransactionHandlerFactory.spec.js

Length of output: 7134

packages/dapi/test/unit/grpcServer/handlers/createGrpcErrorFromDriveResponse.spec.js (3)

91-92: LGTM! Error handling for signature consensus errors is properly tested.

The changes maintain proper error type checking while implementing the new serialization approach.

Also applies to: 105-105


112-113: LGTM! Fee consensus error handling is well-tested.

The test properly handles BigInt values and maintains correct error type checking while implementing the new serialization approach.

Also applies to: 121-121


72-73: LGTM! Verify consistency across other implementations.

The changes correctly implement the new error serialization approach and update the metadata key. The test structure remains robust.

Let's verify the consistency of this new metadata key usage across the codebase:

Also applies to: 82-82

✅ Verification successful

Let me check the actual implementation and usage of this metadata key to ensure consistency.


The metadata key usage is consistent and properly implemented across the stack

The verification shows that the dash-serialized-consensus-error-bin metadata key is consistently used across:

  • DAPI server (error creation and serialization)
  • JS DAPI client (deserialization and error handling)
  • Rust SDK (deserialization and error handling)

All implementations follow the same pattern of using this key for consensus error serialization/deserialization, with proper error handling and type safety.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for all occurrences of the new metadata key to ensure consistent usage
rg -l 'dash-serialized-consensus-error-bin'

Length of output: 402


Script:

#!/bin/bash
# Check the implementation in createGrpcErrorFromDriveResponse.js
rg -A 5 'dash-serialized-consensus-error-bin' packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js

# Check the client-side handling in createGrpcTransportError.js
rg -A 5 'dash-serialized-consensus-error-bin' packages/js-dapi-client/lib/transport/GrpcTransport/createGrpcTransportError.js

# Check Rust SDK implementation
rg -A 5 'dash-serialized-consensus-error-bin' packages/rs-sdk/src/error.rs

Length of output: 1066

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (3)
packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js (2)

Line range hint 125-133: Consider wrapping deserialization error for better error context.

Instead of logging and re-throwing the original error, consider wrapping it in an InternalGrpcError to maintain consistent error handling throughout the API.

     try {
       consensusError = deserializeConsensusError(serializedConsensusError);
     } catch (e) {
       logger.error({
         err: e,
         data: data.serializedError,
         code,
       }, `Failed to deserialize consensus error with code ${code}: ${e.message}`);
-      throw e;
+      throw new InternalGrpcError(e, {
+        code,
+        originalError: e.message,
+      });
     }

Line range hint 134-166: Document error code ranges and their corresponding error types.

The code maps different ranges of error codes to specific gRPC error types, but the reasoning behind these mappings isn't documented. Consider adding JSDoc comments explaining:

  • Why certain ranges map to specific error types
  • The difference between Basic (10000-20000) vs State (40000-50000) errors, which both use InvalidArgumentGrpcError
+    /**
+     * Error codes are mapped as follows:
+     * - 10000-20000: Basic validation errors (InvalidArgumentGrpcError)
+     * - 20000-30000: Signature verification errors (UNAUTHENTICATED)
+     * - 30000-40000: Fee validation errors (FailedPreconditionGrpcError)
+     * - 40000-50000: State validation errors (InvalidArgumentGrpcError)
+     */
     // Basic
     if (code >= 10000 && code < 20000) {
packages/rs-sdk/src/error.rs (1)

76-76: Track technical debt: DapiClientError decomposition.

The TODO comment indicates future work to decompose DapiClientError into more specific error types. This aligns with best practices for error handling granularity.

Would you like me to create a GitHub issue to track this technical debt?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 3ca8abe and e19902c.

📒 Files selected for processing (2)
  • packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js (4 hunks)
  • packages/rs-sdk/src/error.rs (2 hunks)
🧰 Additional context used
🪛 Biome
packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js

[error] 68-68: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

🔇 Additional comments (3)
packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js (1)

67-69: LGTM: Error serialization extraction is clear and maintainable.

🧰 Tools
🪛 Biome

[error] 68-68: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

packages/rs-sdk/src/error.rs (2)

2-10: LGTM! Import changes align with the PR objectives.

The new imports support the enhanced error handling capabilities, particularly for consensus errors, while removing the generic type parameter from DapiClientError.


77-93: LGTM! Verify error handling coverage.

The implementation successfully extracts and deserializes consensus errors from gRPC metadata, with appropriate fallback handling. This aligns with the PR objective to return consensus errors from broadcast methods.

Let's verify the error handling coverage:

✅ Verification successful

Let me generate additional shell scripts to search for metadata key usage to complete the verification:

The first search will help verify the specific consensus error metadata key usage, while the second search will help identify any other metadata key patterns we should consider. This will allow me to make a complete assessment of the error handling coverage.


Based on the search results, I can now provide a final assessment of the error handling coverage:

Error handling implementation is complete and well-structured

The implementation correctly handles consensus errors by:

  1. Extracting serialized consensus errors from gRPC metadata using the standardized key "dash-serialized-consensus-error-bin"
  2. Deserializing the error bytes into proper ConsensusError type
  3. Wrapping in appropriate Protocol error variant with fallback handling

The code follows best practices by:

  • Using a consistent metadata key for error propagation
  • Providing proper error type conversion
  • Including fallback error handling when deserialization fails
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for potential error cases we need to handle
# Test 1: Look for other potential consensus error sources
rg -g '*.rs' -A 3 'ConsensusError'

# Test 2: Check for other gRPC metadata keys we might need to handle
rg -g '*.rs' 'metadata\(\).get_bin\(".*"\)'

Length of output: 516434


Script:

#!/bin/bash
# Search for metadata key usage in gRPC/transport code
rg -g '*.rs' -B 3 -A 3 'dash-serialized-consensus-error-bin'

# Search for other metadata key usage patterns
rg -g '*.rs' -B 3 -A 3 'metadata.*get_bin'

Length of output: 681

shumkov and others added 3 commits October 28, 2024 21:45
# Conflicts:
#	packages/rs-dapi-client/src/dapi_client.rs
#	packages/rs-dapi-client/src/lib.rs
#	packages/rs-dapi-client/src/mock.rs
#	packages/rs-dapi-client/src/transport.rs
#	packages/rs-sdk/src/error.rs
#	packages/rs-sdk/src/platform/fetch.rs
#	packages/rs-sdk/src/platform/transition/put_identity.rs
#	packages/rs-sdk/src/sdk.rs
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (6)
packages/rs-sdk/src/error.rs (2)

77-77: Track TODO comment in issue tracker.

This TODO suggests important future work to improve error handling granularity.

Would you like me to create a GitHub issue to track this TODO for decomposing DapiClientError into more specific error types?


94-94: Consider improving the fallback error message.

Using Debug formatting (:?) in error messages might expose internal details that shouldn't be visible to end users.

Consider using a more user-friendly error format:

-        Self::DapiClientError(format!("{:?}", value))
+        Self::DapiClientError(value.to_string())
packages/rs-sdk/tests/fetch/contested_resource_vote_state.rs (2)

Line range hint 276-350: LGTM: Comprehensive test coverage for error cases

The test cases thoroughly cover various edge cases and error conditions, aligning well with the PR's focus on improved error handling. They validate:

  • Input bounds and constraints
  • Non-existent resources
  • Invalid configurations
  • Different result types

Consider adding a brief comment block above the test cases to document the overall testing strategy and the categories of cases being tested.


Line range hint 351-411: Consider making error validation more robust

The current implementation uses string matching for validating error messages, which could be fragile if error messages change. Consider these improvements:

  1. Define error constants or enums instead of hardcoded strings:
const ERROR_MESSAGES = {
    INVALID_LIMIT: "limit {} out of bounds of [1, 100]",
    CONTRACT_NOT_FOUND: "contract not found error",
    // ... other error messages
};
  1. Use stronger typing for expected results:
enum ExpectedResult {
    Success(ExpectedData),
    Error(ErrorType),
}

enum ErrorType {
    InvalidArgument(String),
    ContractNotFound,
    // ... other error types
}

// Update the test function signature
async fn contested_rss_vote_state_fields(
    query_mut_fn: MutFn,
    expect: ExpectedResult,
) -> Result<(), String>

This would make the tests more maintainable and less prone to breaking when error messages are updated.

packages/rs-sdk/src/sdk.rs (2)

Line range hint 424-499: Consider extracting nonce management to reduce code duplication.

While the implementation is correct and thread-safe, there's significant code duplication with get_identity_nonce. Consider extracting the common nonce management logic into a shared implementation.

Here's a suggested approach:

// Create a generic nonce manager
struct NonceManager<K> {
    counter: Mutex<BTreeMap<K, (IdentityNonce, LastQueryTimestamp)>>,
}

impl<K: Eq + Hash> NonceManager<K> {
    async fn get_nonce(
        &self,
        key: K,
        bump_first: bool,
        settings: Option<PutSettings>,
        fetch_nonce: impl Future<Output = Result<IdentityNonce, Error>>,
    ) -> Result<IdentityNonce, Error> {
        // Common nonce management logic here
    }
}

This would allow both methods to share the same core logic while handling different key types.


424-424: TODO: Consider creating a tracking issue for the struct extraction.

The TODO comment suggests moving this to a separate struct. This would improve code organization and maintainability.

Would you like me to create a GitHub issue to track this refactoring task?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between e19902c and 75eb334.

📒 Files selected for processing (11)
  • packages/rs-dapi-client/src/dapi_client.rs (5 hunks)
  • packages/rs-dapi-client/src/executor.rs (2 hunks)
  • packages/rs-dapi-client/src/lib.rs (2 hunks)
  • packages/rs-dapi-client/src/mock.rs (2 hunks)
  • packages/rs-sdk/src/error.rs (2 hunks)
  • packages/rs-sdk/src/platform/delegate.rs (1 hunks)
  • packages/rs-sdk/src/platform/document_query.rs (2 hunks)
  • packages/rs-sdk/src/platform/identities_contract_keys_query.rs (2 hunks)
  • packages/rs-sdk/src/platform/transition/put_identity.rs (2 hunks)
  • packages/rs-sdk/src/sdk.rs (4 hunks)
  • packages/rs-sdk/tests/fetch/contested_resource_vote_state.rs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • packages/rs-dapi-client/src/lib.rs
  • packages/rs-dapi-client/src/mock.rs
  • packages/rs-sdk/src/platform/delegate.rs
  • packages/rs-sdk/src/platform/document_query.rs
  • packages/rs-sdk/src/platform/identities_contract_keys_query.rs
  • packages/rs-sdk/src/platform/transition/put_identity.rs
🔇 Additional comments (12)
packages/rs-dapi-client/src/executor.rs (1)

14-18: LGTM: Simplified error handling in execute method

The changes align well with the PR objectives by:

  1. Removing the generic transport error type parameter from DapiClientError
  2. Simplifying the error handling interface
  3. Making the code more maintainable by reducing type complexity

The modification maintains type safety while making the API more ergonomic.

Let's verify that this change is consistently applied across the codebase:

✅ Verification successful

Based on the search results, I can now generate the final response as we have sufficient evidence to verify the changes:

Verified: DapiClientError simplification is consistently applied

The codebase search confirms that:

  1. All usages of DapiClientError have been updated to use the non-generic version
  2. The error type is consistently used without type parameters across the codebase
  3. The execute method signatures in both SDK and DAPI client use the simplified error type

The changes maintain consistency throughout the codebase and successfully achieve the PR's objective of simplifying error handling.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining usages of DapiClientError with generic type parameters
# Expected: No matches should be found as all occurrences should be updated

# Search for potential inconsistencies in DapiClientError usage
rg "DapiClientError\s*<" 

# Search for execute method implementations to verify signature consistency
ast-grep --pattern 'async fn execute<$_>($_) -> ExecutionResult<$_, $_>'

Length of output: 101


Script:

#!/bin/bash
# Let's try a broader search to verify the changes

# 1. Search for all occurrences of DapiClientError to verify its usage
rg "DapiClientError" -A 2

# 2. Search for all execute method signatures in the codebase
rg "fn execute.*ExecutionResult" -A 2

# 3. Look for any remaining generic type parameters in error types
rg "Error\s*<.*>" -A 2

# 4. Find all files that might contain related implementations
fd "executor|error|client" --type f

Length of output: 56505

packages/rs-sdk/src/error.rs (2)

2-10: LGTM: Import changes align with new error handling requirements.

The new imports for ConsensusError and TransportError support the enhanced error handling capabilities described in the PR objectives.


80-93: LGTM: Consensus error handling implementation.

The implementation correctly extracts and deserializes consensus errors from gRPC metadata, matching the PR objectives. The error path handling aligns with the discussed approach in previous reviews.

packages/rs-sdk/tests/fetch/contested_resource_vote_state.rs (1)

270-273: LGTM: Improved assertion message clarity

The assertion message has been corrected to be more grammatically accurate.

packages/rs-sdk/src/sdk.rs (2)

30-31: LGTM: Import changes align with error handling simplification.

The removal of generic type parameters from DapiClientError aligns with the PR objective to simplify error handling.


679-679: LGTM: Method signature change aligns with error handling simplification.

The simplified return type removes the generic error parameter, making the error handling more straightforward. This breaking change is properly documented in the PR objectives.

packages/rs-dapi-client/src/dapi_client.rs (6)

24-30: Simplify DapiClientError by removing the generic type parameter

The removal of the generic type parameter <TE: Mockable> from DapiClientError simplifies the error handling and reduces complexity associated with generics. This change enhances clarity and maintainability.


27-30: Ensure proper serialization for TransportError

The serde(with = "dapi_grpc::mock::serde_mockable") attribute applied to TransportError ensures that serialization is handled correctly when the mocks feature is enabled. Confirm that TransportError implements the Mockable trait correctly and that serialization/deserialization functions as expected.

You can verify the serialization implementation by reviewing the Mockable trait implementation for TransportError and adding relevant tests.


Line range hint 44-53: Confirm correctness of CanRetry implementation

The implementation of CanRetry for DapiClientError has been updated. Ensure that the can_retry method correctly reflects the retry logic for each variant of DapiClientError, especially with the changes to the Transport variant.


Line range hint 60-68: Update Mockable implementation for DapiClientError

With the removal of the generic type parameter, the Mockable implementation for DapiClientError should be reviewed to ensure it serializes and deserializes correctly, especially when the mocks feature is enabled.

Consider adding tests to confirm that serialization and deserialization work as intended for all error variants.


110-114: Update method signature to reflect new error type

The return type of the execute method has been updated to ExecutionResult<R::Response, DapiClientError>, simplifying error handling. Ensure that all call sites of execute handle DapiClientError appropriately, and that the removal of the generic error type does not introduce any issues.

Run existing tests to ensure that the method's behavior remains correct with the new error type.


114-114: Ensure TransportError implements the Mockable trait

Line 114 adds a where clause TransportError: Mockable. Confirm that TransportError implements the Mockable trait for proper serialization in mock environments.

If TransportError does not implement Mockable, implement it or adjust the constraints accordingly.

Comment on lines +148 to +151
let address_result = address_list
.get_live_address()
.cloned()
.ok_or(DapiClientError::NoAvailableAddresses);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Handle NoAvailableAddresses error correctly

The change at lines 148-151 introduces DapiClientError::NoAvailableAddresses when there are no live addresses. Verify that this error is handled appropriately in calling code, and that user-facing error messages are clear when this condition occurs.

Consider adding user-friendly messages or logging to help diagnose issues when no addresses are available, enhancing the developer experience during debugging.

@lklimek lklimek self-assigned this Oct 29, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)
packages/rs-sdk/tests/fetch/contested_resource_identity_votes.rs (1)

70-70: Consider improving test automation.

While the detailed manual setup instructions are helpful, consider:

  1. Creating a helper script to automate the setup process
  2. Using fixtures or mock data for CI/CD environments
  3. Adding environment variables to configure test behavior

Would you like me to help create:

  1. A setup script that automates these manual steps?
  2. Mock data fixtures for CI/CD environments?
packages/rs-sdk/tests/fetch/data_contract.rs (1)

Line range hint 133-157: Consider improving test documentation.

The test's documentation thoroughly explains the manual setup process, which is excellent. However, consider:

  1. Adding a link to where these test vectors are documented
  2. Including information about how to verify the test vector's validity

Would you like me to help create a documentation template for test vectors?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 75eb334 and f806087.

📒 Files selected for processing (82)
  • packages/rs-sdk/tests/fetch/config.rs (2 hunks)
  • packages/rs-sdk/tests/fetch/contested_resource_identity_votes.rs (1 hunks)
  • packages/rs-sdk/tests/fetch/contested_resource_vote_state.rs (2 hunks)
  • packages/rs-sdk/tests/fetch/data_contract.rs (1 hunks)
  • packages/rs-sdk/tests/vectors/check_mn_voting_prerequisities/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/check_mn_voting_prerequisities/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_resource_identity_votes_not_found/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_resource_identity_votes_not_found/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_resource_identity_votes_ok/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_resource_identity_votes_ok/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_resource_vote_states_not_found/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_resource_vote_states_not_found/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_resource_vote_states_ok/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_resource_vote_states_ok/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_resource_vote_states_with_limit/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_resource_vote_states_with_limit/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_resource_voters_for_existing_contestant/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_resource_voters_for_existing_contestant/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_fields_0760ac4854fc8db803bbcbab8709f390bd31511a05e29cd3f170b48ca6b87584/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_fields_0760ac4854fc8db803bbcbab8709f390bd31511a05e29cd3f170b48ca6b87584/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_fields_2dd27fc692c7bf9389f3b48c73a4d2538a4eb72d54c01cc4620b16b06eb31f4c/data_contract-e668c659af66aee1e72c186dde7b5b7e0a1d712a09c40d5721f622bf53c53155.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_fields_2dd27fc692c7bf9389f3b48c73a4d2538a4eb72d54c01cc4620b16b06eb31f4c/quorum_pubkey-106-24de77bd1f67e0c3e20a399b6a3e6bfae60312e2f833b302a62be93e181e7a0e.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_fields_473cf8e4a270ced75e199e5a3e907b4df4cd66b64365d1ac77c45bcaed443a03/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_fields_473cf8e4a270ced75e199e5a3e907b4df4cd66b64365d1ac77c45bcaed443a03/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_fields_948b5a301af5fc73db7fed418a4fe90f64952b4ddd6b03a7f21d029dc110af50/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_fields_948b5a301af5fc73db7fed418a4fe90f64952b4ddd6b03a7f21d029dc110af50/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_fields_9f57b982b3e3b0286093d8b48ab27b87b22f67a172579913f2fec7a6b5ea31b7/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_fields_9f57b982b3e3b0286093d8b48ab27b87b22f67a172579913f2fec7a6b5ea31b7/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_fields_b7df460e812b958de6e703d8ea325df9aab3448d0409ece3f0baf1d26629e44f/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_fields_b7df460e812b958de6e703d8ea325df9aab3448d0409ece3f0baf1d26629e44f/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_fields_ccb199c48ee58a8bb98742b964cba7bda3b4469b740201d2628f15f926f39347/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_fields_ccb199c48ee58a8bb98742b964cba7bda3b4469b740201d2628f15f926f39347/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_limit/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_limit/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_start_at_value/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_resources_start_at_value/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__2c32c08bee77a2d062db8d1cdb999a841b21043ccc5e7fc5ff1507f9703754a9/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__2c32c08bee77a2d062db8d1cdb999a841b21043ccc5e7fc5ff1507f9703754a9/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__67dc63814e035b20ff705840e19ebb329026b51129b8dd9cd029d26a0a77408b/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__67dc63814e035b20ff705840e19ebb329026b51129b8dd9cd029d26a0a77408b/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__a39a5f7014a616de3d7002bb4f18772c5b5d3526cb2b3c33566bd64512882669/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__a39a5f7014a616de3d7002bb4f18772c5b5d3526cb2b3c33566bd64512882669/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__c71b4f8d63f8a1357936129a226f99e349e7b21b166609cf59062e27ed68a1ee/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__c71b4f8d63f8a1357936129a226f99e349e7b21b166609cf59062e27ed68a1ee/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__cfe3ebd0f17745be80e4348304fe468f3eb8c2d17e23d310582b0b34f2d5bba7/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__cfe3ebd0f17745be80e4348304fe468f3eb8c2d17e23d310582b0b34f2d5bba7/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__fbed6654145b9fba061d182cac5a3879bc9bacb92dc9ae64ed2a07ca2ec0392f/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__fbed6654145b9fba061d182cac5a3879bc9bacb92dc9ae64ed2a07ca2ec0392f/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/document_list_bug_value_text_decode_base58/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/document_list_bug_value_text_decode_base58/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/document_list_document_query/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/document_list_document_query/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/document_list_drive_query/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/document_list_drive_query/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/document_read/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/document_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/document_read_no_contract/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/document_read_no_contract/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/document_read_no_document/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/document_read_no_document/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/test_contested_resource_voters_for_identity_not_found/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/test_contested_resource_voters_for_identity_not_found/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/test_contested_resources_ok/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/test_contested_resources_ok/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/test_data_contract_history_read/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/test_data_contract_history_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/test_data_contract_read/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/test_data_contract_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/test_data_contract_read_not_found/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/test_data_contract_read_not_found/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/test_data_contracts_1_ok_1_nx/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/test_data_contracts_1_ok_1_nx/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/test_data_contracts_2_nx/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/test_data_contracts_2_nx/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/test_epoch_fetch/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/test_epoch_fetch/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/test_epoch_fetch_current/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/test_epoch_fetch_current/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/test_epoch_fetch_future/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/test_epoch_fetch_future/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
  • packages/rs-sdk/tests/vectors/test_epoch_list/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json (1 hunks)
  • packages/rs-sdk/tests/vectors/test_epoch_list/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (0 hunks)
⛔ Files not processed due to max files limit (18)
  • packages/rs-sdk/tests/vectors/test_epoch_list_limit/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json
  • packages/rs-sdk/tests/vectors/test_epoch_list_limit/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_identity_balance_read/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json
  • packages/rs-sdk/tests/vectors/test_identity_balance_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_identity_balance_revision_read/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json
  • packages/rs-sdk/tests/vectors/test_identity_balance_revision_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_identity_contract_no_nonce_read/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json
  • packages/rs-sdk/tests/vectors/test_identity_contract_no_nonce_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_identity_public_keys_all_read/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json
  • packages/rs-sdk/tests/vectors/test_identity_public_keys_all_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_identity_read/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json
  • packages/rs-sdk/tests/vectors/test_identity_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_identity_read_by_key/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json
  • packages/rs-sdk/tests/vectors/test_identity_read_by_key/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_prefunded_specialized_balance_not_found/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json
  • packages/rs-sdk/tests/vectors/test_prefunded_specialized_balance_not_found/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_prefunded_specialized_balance_ok/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json
  • packages/rs-sdk/tests/vectors/test_prefunded_specialized_balance_ok/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
💤 Files with no reviewable changes (40)
  • packages/rs-sdk/tests/vectors/check_mn_voting_prerequisities/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_resource_identity_votes_not_found/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_resource_identity_votes_ok/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_resource_vote_states_not_found/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_resource_vote_states_ok/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_resource_vote_states_with_limit/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_resource_voters_for_existing_contestant/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_resources_fields_0760ac4854fc8db803bbcbab8709f390bd31511a05e29cd3f170b48ca6b87584/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_resources_fields_2dd27fc692c7bf9389f3b48c73a4d2538a4eb72d54c01cc4620b16b06eb31f4c/data_contract-e668c659af66aee1e72c186dde7b5b7e0a1d712a09c40d5721f622bf53c53155.json
  • packages/rs-sdk/tests/vectors/contested_resources_fields_2dd27fc692c7bf9389f3b48c73a4d2538a4eb72d54c01cc4620b16b06eb31f4c/quorum_pubkey-106-24de77bd1f67e0c3e20a399b6a3e6bfae60312e2f833b302a62be93e181e7a0e.json
  • packages/rs-sdk/tests/vectors/contested_resources_fields_473cf8e4a270ced75e199e5a3e907b4df4cd66b64365d1ac77c45bcaed443a03/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_resources_fields_948b5a301af5fc73db7fed418a4fe90f64952b4ddd6b03a7f21d029dc110af50/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_resources_fields_9f57b982b3e3b0286093d8b48ab27b87b22f67a172579913f2fec7a6b5ea31b7/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_resources_fields_b7df460e812b958de6e703d8ea325df9aab3448d0409ece3f0baf1d26629e44f/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_resources_fields_ccb199c48ee58a8bb98742b964cba7bda3b4469b740201d2628f15f926f39347/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_resources_limit/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_resources_start_at_value/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__2c32c08bee77a2d062db8d1cdb999a841b21043ccc5e7fc5ff1507f9703754a9/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__67dc63814e035b20ff705840e19ebb329026b51129b8dd9cd029d26a0a77408b/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__a39a5f7014a616de3d7002bb4f18772c5b5d3526cb2b3c33566bd64512882669/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__c71b4f8d63f8a1357936129a226f99e349e7b21b166609cf59062e27ed68a1ee/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__cfe3ebd0f17745be80e4348304fe468f3eb8c2d17e23d310582b0b34f2d5bba7/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__fbed6654145b9fba061d182cac5a3879bc9bacb92dc9ae64ed2a07ca2ec0392f/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/document_list_bug_value_text_decode_base58/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/document_list_document_query/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/document_list_drive_query/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/document_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/document_read_no_contract/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/document_read_no_document/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_contested_resource_voters_for_identity_not_found/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_contested_resources_ok/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_data_contract_history_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_data_contract_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_data_contract_read_not_found/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_data_contracts_1_ok_1_nx/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_data_contracts_2_nx/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_epoch_fetch/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_epoch_fetch_current/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_epoch_fetch_future/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
  • packages/rs-sdk/tests/vectors/test_epoch_list/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/rs-sdk/tests/fetch/contested_resource_vote_state.rs
🧰 Additional context used
🪛 Biome
packages/rs-sdk/tests/vectors/check_mn_voting_prerequisities/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_resource_identity_votes_not_found/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_resource_identity_votes_ok/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_resource_vote_states_not_found/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_resource_vote_states_ok/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_resource_vote_states_with_limit/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_resource_voters_for_existing_contestant/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_resources_fields_0760ac4854fc8db803bbcbab8709f390bd31511a05e29cd3f170b48ca6b87584/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_resources_fields_473cf8e4a270ced75e199e5a3e907b4df4cd66b64365d1ac77c45bcaed443a03/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_resources_fields_948b5a301af5fc73db7fed418a4fe90f64952b4ddd6b03a7f21d029dc110af50/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_resources_fields_9f57b982b3e3b0286093d8b48ab27b87b22f67a172579913f2fec7a6b5ea31b7/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_resources_fields_b7df460e812b958de6e703d8ea325df9aab3448d0409ece3f0baf1d26629e44f/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_resources_fields_ccb199c48ee58a8bb98742b964cba7bda3b4469b740201d2628f15f926f39347/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_resources_limit/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_resources_start_at_value/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__2c32c08bee77a2d062db8d1cdb999a841b21043ccc5e7fc5ff1507f9703754a9/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__67dc63814e035b20ff705840e19ebb329026b51129b8dd9cd029d26a0a77408b/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__a39a5f7014a616de3d7002bb4f18772c5b5d3526cb2b3c33566bd64512882669/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__c71b4f8d63f8a1357936129a226f99e349e7b21b166609cf59062e27ed68a1ee/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__cfe3ebd0f17745be80e4348304fe468f3eb8c2d17e23d310582b0b34f2d5bba7/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__fbed6654145b9fba061d182cac5a3879bc9bacb92dc9ae64ed2a07ca2ec0392f/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/document_list_bug_value_text_decode_base58/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/document_list_document_query/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/document_list_drive_query/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/document_read/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/document_read_no_contract/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/document_read_no_document/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/test_contested_resource_voters_for_identity_not_found/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/test_contested_resources_ok/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/test_data_contract_history_read/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/test_data_contract_read/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/test_data_contract_read_not_found/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/test_data_contracts_1_ok_1_nx/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/test_data_contracts_2_nx/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/test_epoch_fetch/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/test_epoch_fetch_current/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/test_epoch_fetch_future/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

packages/rs-sdk/tests/vectors/test_epoch_list/quorum_pubkey-106-029045172fbebcf97290db8879490a48ac0d1d60e249d031b5ad79e3e46e3fca.json

[error] 1-1: String values must be double quoted.

(parse)

🔇 Additional comments (5)
packages/rs-sdk/tests/fetch/contested_resource_identity_votes.rs (2)

70-70: Verify command outputs in documentation.

The documentation references specific command outputs and file locations. Let's ensure these are accurate and up-to-date.

✅ Verification successful

Let me check the actual documentation content first to verify what command outputs are being referenced.


Now I can generate the final response since I've seen the actual documentation and command outputs being referenced.

Documentation accurately reflects the command outputs and file locations

The documentation in the test file contested_resource_identity_votes_ok() correctly specifies:

  • The exact command to run: egrep '(ProRegTx transaction ID|Owner Private Key)' logs/setup.log|head -n2
  • The expected location of the log file: logs/setup.log
  • The purpose and context of these commands in the setup process
  • Clear step-by-step instructions on how to use these outputs

The fact that setup.log wasn't found in our verification is expected as this is a test environment. The documentation explicitly states these steps are for manual setup and the test is appropriately marked with #[cfg_attr(not(feature = "offline-testing"), ignore = "...")].

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the grep command output format mentioned in docs

# Check if the mentioned log patterns exist
echo "Checking log patterns..."
if [ -f logs/setup.log ]; then
    echo "Verifying log patterns..."
    rg "ProRegTx transaction ID|Owner Private Key" logs/setup.log
else
    echo "setup.log not found, cannot verify patterns"
fi

Length of output: 183


Script:

#!/bin/bash
# Check the content of the test file
cat packages/rs-sdk/tests/fetch/contested_resource_identity_votes.rs

Length of output: 4431


70-70: Documentation improvements enhance test setup clarity.

The detailed setup instructions are well-structured and provide clear steps for manual test configuration. This is particularly valuable for developers who need to run these tests locally.

Let's verify the referenced files and tools exist:

packages/rs-sdk/tests/fetch/data_contract.rs (1)

138-138: Verify the new data contract ID test vector.

The hardcoded data contract ID has been updated. Since this test requires special setup and is ignored without the offline-testing feature, we should verify:

  1. That this ID corresponds to a data contract with keep_history set to true
  2. That the test vectors are properly documented

Let's verify the data contract ID's usage:

✅ Verification successful

The first search showed the ID is only used in one file, but we need to understand its context and documentation better. Let's examine the test file content and look for any test vector documentation in the rs-sdk package.


The test file shows extensive documentation about the test vector's setup requirements, mentioning it needs a data contract with keep_history set to true. Let's verify if the test vector generation script contains related information and check the referenced test file for the contract setup.


Based on the gathered information, I can now provide the final response:

The data contract ID test vector is well documented and properly configured

The test file contains comprehensive documentation explaining:

  • The test requires a data contract with keep_history=true
  • Detailed setup instructions are provided in comments
  • The test is correctly marked with #[cfg_attr(not(feature = "offline-testing"), ignore = "...")]
  • The ID is used only in this specific test case
  • Test vector generation is supported through the provided generate_test_vectors.sh script
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other occurrences of this ID to ensure consistency
rg -l "eacc9ceb6c11ee1ae82afb5590d78d686f43bc0f0e0cd65de1e23c150e41f97f"

# Look for test vector documentation
fd "README|TEST_VECTORS" --type f

Length of output: 1240


Script:

#!/bin/bash
# Let's examine the test file to understand the context and documentation of this test vector
cat packages/rs-sdk/tests/fetch/data_contract.rs

# Also check if there's any test vector documentation in the rs-sdk
fd "test.*vector|fixture" --type f packages/rs-sdk/

Length of output: 6172


Script:

#!/bin/bash
# Let's check the test vector generation script to see if it's related to this test case
cat packages/rs-sdk/scripts/generate_test_vectors.sh

# Also check if there are any related test fixtures
rg -A 5 "keep_history" packages/platform-test-suite/test/functional/platform/DataContract.spec.js

Length of output: 853

packages/rs-sdk/tests/fetch/config.rs (2)

228-228: Consider automating test identity creation.

The TODO comment highlights a maintenance burden where test identities need manual updates. This process is error-prone and could be automated to improve test reliability.

Would you like me to help create an automated solution for test identity generation? This could involve:

  1. Creating a helper function to generate deterministic test identities
  2. Adding a setup script to generate and persist test identities
  3. Documenting the process for future test vector updates

255-255: Document ProTxHash update requirements.

The ProTxHash is critical for test vectors, but there's limited documentation on when and how it should be updated. Consider adding more context about:

  1. The conditions that require a ProTxHash update
  2. The process to obtain a valid ProTxHash
  3. The impact on test vectors when this value changes

Let's verify if this ProTxHash is referenced in other test files:

@shumkov shumkov merged commit 90980d9 into v1.4-dev Oct 29, 2024
48 checks passed
@shumkov shumkov deleted the feat/sdk/return-consensus-errors branch October 29, 2024 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Return consensus errors instead of transport errors in SDK
2 participants