feat(sdk)!: provide request execution information #2259

shumkov · 2024-10-21T12:31:43Z

Issue being fixed or feature implemented

In SDK we would like to have more information on request execution including address and retries count for logging, error handling, and retry implementation on the SDK level

What was done?

Added ExecutionResult type that contains ExecutionResponse and ExecutionError. Both stucts contains retries, address and inner objects.
Updated retry policy to make delays between retries constant. Since we are doing requests to different nodes there is no point in having exponential delays.
Removed Address from the DapiClientError::Transport error
Added temporary handlers for ExecutionResponse and ExecutionError to SDK

How Has This Been Tested?

Running tests

Breaking Changes

DapiRequestExecutor::execute and DapiRequest::execute now returns ExecutionResult

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
- Enhanced error handling and response processing for various methods within the SDK, improving clarity and robustness.
- Introduction of new ExecutionResult type for better handling of execution contexts in requests.
- New JSON files added for testing various functionalities related to quorum public keys and identity management.
Bug Fixes
- Removed deprecated files and improved error handling across multiple modules.
Documentation
- Updated comments and documentation to reflect changes in error handling and new features.

coderabbitai · 2024-10-21T14:08:57Z

Walkthrough

The pull request introduces significant changes to the DAPI client and its associated components. Key modifications include the implementation of a new retry strategy using a fixed delay, enhancements to error handling by simplifying error types, and the introduction of a new ExecutionResult type for managing responses. The changes also involve the addition of a new module for request execution, updating method signatures across various traits, and improving mock implementations for testing. Overall, the updates aim to create a more structured and robust error handling and execution framework within the DAPI client.

Changes

File Path	Change Summary
`packages/rs-dapi-client/src/dapi_client.rs`	Modified `DapiClient` to change retry strategy from `ExponentialBuilder` to `ConstantBuilder`, simplified `DapiClientError` enum by removing `Address`, updated `can_retry` method, and refactored `execute` method return type to `ExecutionResult`.
`packages/rs-dapi-client/src/executor.rs`	Introduced `DapiRequestExecutor` trait with an `execute` method, defined `ExecutionError` and `ExecutionResponse` structs, and added `ExecutionResult` type alias.
`packages/rs-dapi-client/src/lib.rs`	Added `executor` module, modified `execute` method return type in `DapiRequest` trait, and updated documentation examples.
`packages/rs-dapi-client/src/mock.rs`	Enhanced `MockDapiClient` to utilize `ExecutionResult`, refined error handling in `execute` method, and updated `ExpectedResponse` to `ExpectedResult`.
`packages/rs-dapi-client/src/transport.rs`	Updated `TransportClient` trait to require `std::error::Error` for associated `Error` type.
`packages/rs-sdk/src/core/transaction.rs`	Enhanced error handling in `start_instant_send_lock_stream` and `wait_for_asset_lock_proof_for_transaction` methods.
`packages/rs-sdk/src/mock/sdk.rs`	Updated `MockDashPlatformSdk` to use `ExecutionResponse` and refined mock response handling.
`packages/rs-sdk/src/platform/fetch.rs`	Improved error handling in `fetch_with_metadata` and `fetch_with_metadata_and_proof` methods.
`packages/rs-sdk/src/platform/fetch_many.rs`	Enhanced error handling in `fetch_many` methods.
`packages/rs-sdk/src/platform/fetch_unproved.rs`	Updated `fetch_unproved_with_settings` method for better error handling.
`packages/rs-sdk/src/platform/transition/*.rs`	Improved error handling in various transition methods including `broadcast`, `put_to_platform`, and `wait_for_response`.
`packages/rs-sdk/src/sdk.rs`	Updated `execute` method in `DapiRequestExecutor` to return `ExecutionResult`.
`packages/rs-sdk/tests/*`	Various test files updated to reflect changes in data structures and error handling, with new test vectors added.
`packages/rs-dapi-client/src/address_list.rs`	Added `FromStr` implementation for `Address` and updated `AddressListError` enum.
`packages/rs-sdk/tests/fetch/config.rs`	Modified `default_identity_id` and `default_protxhash` methods to change default values.
`packages/rs-sdk/tests/fetch/contested_resource_identity_votes.rs`	Minor comment change in test cases.
`packages/rs-sdk/tests/fetch/data_contract.rs`	Added comprehensive tests for fetching data contracts.
`packages/rs-sdk/tests/vectors/*`	Multiple JSON files added and deleted, reflecting changes in test data and scenarios.

Possibly related PRs

feat(sdk): fix client tls connections #2223: Modifies the return type of the execute method in DapiRequestExecutor, directly related to changes in this PR.
feat(sdk)!: detect stale nodes #2254: Enhances error handling and introduces StaleNodeError, relevant to the error handling improvements in this PR.
fix(sdk): opposite retry trigger #2265: Modifies error handling logic in DapiRequestExecutor, aligning with the refinements made in this PR.

Suggested reviewers

QuantumExplorer
lklimek

Poem

🐇 In the land of code, where rabbits hop,
New changes bloom, we can't let them stop.
With retries now steady and errors refined,
The DAPI client shines, oh so well-defined!
So let’s celebrate, with a joyful cheer,
For the code is alive, and the future is near! 🌟

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.

coderabbitai

Actionable comments posted: 19

🧹 Outside diff range and nitpick comments (26)

packages/rs-dapi-client/tests/mock_dapi_client.rs (2)

26-30: LGTM: ExecutionResponse correctly implemented.

The ExecutionResponse object is properly created with the inner response, retries set to 0, and a default Address. This aligns with the new structure introduced in the PR.

Consider using a more specific Uri for the address field instead of the default. This could provide more realistic test scenarios and potentially catch edge cases related to different URIs.

Line range hint 1-42: Summary: Test updated successfully to use new ExecutionResponse structure.

The changes in this file successfully implement the new ExecutionResponse structure in the test_mock_get_identity_dapi_client test. The modifications include:

Adding necessary imports

Renaming the response to inner

Creating an ExecutionResponse object

Updating mock expectations

Modifying assertions

These changes align well with the PR objectives and maintain the integrity of the test while adapting it to the new structure. The test now correctly validates the behavior of the updated DAPI client implementation.

As the DAPI client evolves, consider adding more specific test cases that cover different scenarios with the new ExecutionResponse structure, such as varying retry counts or different address values.

packages/rs-dapi-client/src/lib.rs (2)

23-23: LGTM: Public use of executor module items.

The pub use executor::*; statement makes all public items from the executor module available at the crate's root level, which aligns with the PR objectives of providing access to new execution information types.

For consistency, consider updating the executor module declaration on line 10 to pub mod executor; to match this usage.

52-52: LGTM: Updated return type for execute method.

The change from Result to ExecutionResult aligns with the PR objectives of providing more detailed information about request execution.

Consider updating the method documentation to reflect the new return type and explain the additional information provided by ExecutionResult. This will help users understand the implications of this breaking change.

packages/rs-sdk/src/platform/transition/broadcast.rs (1)

Line range hint 1-80: Overall improvements in error handling, with opportunities for further refinement.

The changes in this file consistently enhance error handling and response processing, aligning well with the PR objectives. The introduction of map and map_err operations to handle ExecutionResponse and ExecutionError types improves the robustness of the code.

Recommendations:

Address the TODO comments by developing a standardized approach for handling execution responses and errors.

Consider the suggested refactoring to reduce code duplication and centralize the error handling logic.

Ensure that the proposed changes are consistently applied across the codebase, particularly in other files that may have similar patterns.

These improvements will further enhance the maintainability and consistency of the error handling mechanisms throughout the SDK.

packages/rs-sdk/src/platform/transition/top_up_identity.rs (1)

50-59: Overall improvement in error handling, but further enhancements recommended.

The changes made to the top_up_identity function represent a positive step towards better error handling in the SDK. The consistent approach to extracting inner errors and responses aligns well with the PR objectives.

To further improve the error handling and align with the PR objectives:

Implement a structured ExecutionResult type that includes retry count and address information.

Replace the current error mapping with this new type.

Update the TODO comments with specific tasks or create GitHub issues to track the planned improvements.

Consider implementing a custom error type for top_up_identity that can encapsulate both execution errors and business logic errors.

These enhancements will provide more detailed execution information and improve error handling consistency across the SDK.
packages/rs-sdk/src/platform/transition/put_identity.rs (1)
107-111: Consistent improvement in response and error handling.

The changes in this segment are consistent with the previous modifications, enhancing response and error handling. The repeated TODO comment suggests a systematic need for improvement in handling execution responses and errors across the codebase.

Consider creating a unified approach to handle execution responses and errors, which can be applied consistently across all similar instances in the codebase. This could involve creating a helper function or macro to standardize the error handling process.

Example helper function:
fn handle_execution_result<T, E>(result: Result<ExecutionResponse<T>, ExecutionError<E>>) -> Result<T, E> {
    result
        .map(|execution_response| execution_response.into_inner())
        .map_err(|execution_error| execution_error.into_inner())
}
This function could then be used to replace the repeated code blocks:
let response = handle_execution_result(
    request.execute(sdk, RequestSettings::default()).await
)?;
Would you like assistance in implementing this refactoring across the codebase?
packages/rs-sdk/src/platform/transition/put_contract.rs (3)
89-90: Improved error handling, but further refinement needed.

The change enhances error handling by explicitly converting the error to its inner type, which aligns with the PR's objective of improving error management in the SDK. However, the TODO comment indicates that further improvements are necessary.

Would you like assistance in creating a GitHub issue to track the TODO for implementing a better way to handle execution errors?

104-108: Improved response and error handling, but further refinement needed.

The change enhances both response and error handling by explicitly converting them to their inner types, which aligns with the PR's objective of improving error management in the SDK. However, the TODO comment indicates that further improvements are necessary.

Consider refactoring this pattern of mapping responses and errors to their inner types into a separate function, as it appears in both put_to_platform and wait_for_response methods. This would reduce code duplication and improve maintainability.

Consider creating a helper function to handle the mapping of execution responses and errors:
fn map_execution_result<T>(result: Result<ExecutionResponse<T>, ExecutionError>) -> Result<T, Error> {
    result
        .map(|execution_response| execution_response.into_inner())
        .map_err(|execution_error| execution_error.into_inner().into())
}
Then use it in both methods:
let response = map_execution_result(request.execute(sdk, RequestSettings::default()).await)?;
Would you like assistance in creating a GitHub issue to track the TODO for implementing a better way to handle execution responses and errors?

Line range hint 1-153: Summary: Improved error handling aligns with PR objectives

The changes in this file consistently improve error handling for asynchronous operations, which aligns well with the PR's objective of enhancing error management in the SDK. The introduction of explicit mapping for both responses and errors to their inner types provides better clarity and control over error propagation.

However, the TODO comments indicate that this is an intermediate step towards a more comprehensive error handling mechanism. As the SDK evolves, it would be beneficial to revisit these areas and implement a more robust and standardized approach to handling execution errors and responses across all asynchronous operations.

Consider developing a standardized error handling strategy for asynchronous operations across the entire SDK. This could involve creating custom error types that encapsulate both execution and domain-specific errors, providing a consistent interface for error handling throughout the codebase.
packages/rs-sdk/tests/fetch/epoch.rs (2)
28-30: Improved error handling, but TODO needs addressing

The new implementation provides more granular control over response and error handling, aligning with the ExecutionResult changes. However, the TODO comment suggests that further improvements are needed.

Consider creating a GitHub issue to track the TODO for implementing a better way to handle execution response and errors.

Add tests for error handling scenarios in get_current_epoch

The current test cases do not cover error handling scenarios for the get_current_epoch function. Please add tests to handle potential errors and edge cases.

🔗 Analysis chain

Line range hint 1-180: Consider adding tests for new error handling scenarios

While the existing tests remain unchanged and should still be valid, it might be beneficial to add new tests that cover the enhanced error handling scenarios introduced in the get_current_epoch function.

To ensure comprehensive test coverage, please run the following command to check the current test coverage for the get_current_epoch function:

If the results show limited coverage for error scenarios, consider adding new test cases.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Check test coverage for get_current_epoch function

# Test: Search for test cases involving get_current_epoch
rg -n 'get_current_epoch' packages/rs-sdk/tests/
Length of output: 666

Script:
#!/bin/bash
# Description: Check for error handling in tests using get_current_epoch

# Search for common error handling patterns in test cases
rg 'expect_err|assert_err|should_panic|unwrap_err' packages/rs-sdk/tests/fetch/epoch.rs
Length of output: 89
packages/rs-sdk/src/platform/transition/transfer_document.rs (1)

Line range hint 1-180: Overall improvement in error handling, but standardization needed.

The changes in this file represent a step forward in error handling for asynchronous operations. However, the TODO comments and the repetitive nature of these changes across the codebase suggest that a more standardized approach to error handling could be beneficial.

Consider the following suggestions:

Create a custom error type that encapsulates both execution responses and errors.

Implement a helper function or macro that standardizes the error handling pattern across the codebase.

Update the relevant parts of the codebase to use this new standardized approach.

This would improve code consistency, reduce duplication, and make future maintenance easier.

packages/rs-sdk/src/platform/transition/put_document.rs (1)

Line range hint 1-174: Summary of changes in put_document.rs

The modifications in this file focus on improving error handling in the PutDocument trait implementation for Document. These changes align with the PR objectives to enhance the SDK's ability to provide detailed information about request execution.

Key improvements:

Extraction of inner errors and responses from execution results.

Addition of TODO comments for future enhancements in error handling.

These changes contribute to a more robust error handling mechanism across the SDK. However, there's room for further standardization of these patterns across the codebase.

Consider the following suggestions for future improvements:

Implement helper functions or macros to standardize error handling patterns.

Develop a comprehensive error handling strategy that addresses the TODO comments.

Ensure consistent error handling across all SDK components.

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

104-105: Improved error handling, but consider addressing the TODO.

The change enhances error handling by extracting the inner error, which aligns with the PR's objective. However, the TODO comment suggests this might be a temporary solution.

Would you like me to create a GitHub issue to track the TODO for implementing a better way to handle execution errors?

120-124: Improved error and response handling, but consider addressing the TODO and generalizing the approach.

The changes enhance both error and response handling by extracting the inner types, which aligns with the PR's objective. However, the TODO comment suggests this might be a temporary solution.

Consider creating a helper function or method to handle execution results consistently across different methods. This could help reduce code duplication and provide a unified approach to error handling.

Would you like me to create a GitHub issue to track the TODO for implementing a better way to handle execution response and errors?

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

Line range hint 1-324: Summary of changes in fetch.rs

The changes in this file focus on improving error handling in the fetch_with_metadata and fetch_with_metadata_and_proof methods of the Fetch trait. These improvements align well with the PR objectives of enhancing error handling and introducing the new ExecutionResult type.

Key points:

Consistent error handling improvements across both methods.

TODO comments indicate areas for future refinement of the error handling mechanism.

The rest of the file, including trait definitions and implementations, remains unchanged.

Consider the following for future improvements:

Implement a more robust way of handling execution responses and errors, addressing the TODO comments.

Evaluate if similar error handling improvements are needed in other parts of the SDK.

Once the error handling mechanism is finalized, update the documentation to reflect the new behavior and any potential changes in error types that callers might need to handle.

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

Line range hint 1-265: Summary: Consistent error handling improvements with room for further refinement

The changes in this file demonstrate a consistent approach to improving error handling in the Sdk implementation. Both modified methods now handle execution responses and errors more explicitly, which is a positive step. However, the TODO comments indicate that this is an interim solution.

Next steps:

Create a task to design and implement a more robust, standardized error handling mechanism for execution responses and errors throughout the SDK.

Consider extracting the common error handling pattern into a utility function or macro to promote code reuse and consistency.

Once the improved error handling is implemented, update these methods and remove the TODO comments.

Ensure that similar improvements are applied consistently across the entire codebase.

These changes will contribute to a more maintainable and reliable SDK implementation.
packages/rs-sdk/src/platform/fetch_many.rs (3)
149-151: Improved error handling, but TODO needs tracking

The changes enhance error handling by properly transforming the execution response and errors. However, the TODO comment suggests this is a temporary solution.

Consider creating a separate issue to track the TODO for implementing a better way to handle execution response and errors. This will ensure it's not forgotten in future development cycles.

237-241: Consistent error handling, but consider reducing duplication

The changes in the Document implementation are consistent with the modifications in the trait implementation, which is good for maintaining a uniform approach to error handling.

Consider creating a helper function or macro to handle the execution response and error transformation. This would reduce code duplication and make it easier to implement the TODO improvement across all implementations in the future.

Example helper function:
fn handle_execution_result<T>(result: ExecutionResult<T>) -> Result<T, Error> {
    result
        .map(|execution_response| execution_response.into_inner())
        .map_err(|execution_error| execution_error.into_inner().into())
}
This function could then be used in both the trait implementation and the Document implementation, reducing duplication and centralizing the logic for future improvements.

Line range hint 1-437: Overall improvements in error handling with room for further refinement

The changes in this file consistently improve error handling across different implementations of the FetchMany trait. The modifications align well with the PR objectives of enhancing the SDK's request execution information.

For future improvements:

Consider implementing a generic helper function or macro to handle execution results, reducing code duplication.

Track the TODO comments as separate issues to ensure they're addressed in future development cycles.

Once the TODO items are resolved, consider updating the documentation to reflect the new error handling mechanisms.

These steps will further enhance the maintainability and consistency of the codebase.
packages/rs-dapi-client/src/dapi_client.rs (4)

127-129: Re-evaluate the constant retry delay strategy

Changing the retry strategy to a constant delay of 10 milliseconds may lead to aggressive retry attempts, which could overwhelm the nodes or not provide sufficient time for transient issues to resolve. Consider whether a slightly longer delay or a configurable delay setting would be more appropriate to balance retry effectiveness and resource utilization.

193-196: Handle potential errors from TransportClient::with_uri_and_settings more gracefully

When initializing the transport client, errors from with_uri_and_settings are mapped to ExecutionError. Ensure that these errors are appropriately logged and provide sufficient context for troubleshooting, including the address and settings used.

Line range hint 113-117: Update method documentation to reflect the new return type ExecutionResult

The execute method's return type has changed to ExecutionResult. Ensure that the method's documentation and any associated comments are updated to reflect this change for clarity and to aid developers in understanding the new return structure.

276-276: Consider using as_secs_f64 for higher precision in logging durations

In the logging statement, duration.as_secs_f32() is used to display the sleep duration. For higher precision, especially for durations less than one second, consider using as_secs_f64().

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

Line range hint 675-685: Update method documentation to reflect new return type

Consider updating the documentation comments for the execute method to reflect the new return type ExecutionResult<R::Response, DapiClientError<...>>. This will help users understand the additional execution information being returned, such as retries and addresses.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between a2034fd and 5af8822.

📒 Files selected for processing (51)

packages/rs-dapi-client/src/dapi_client.rs (9 hunks)
packages/rs-dapi-client/src/executor.rs (1 hunks)
packages/rs-dapi-client/src/lib.rs (4 hunks)
packages/rs-dapi-client/src/mock.rs (8 hunks)
packages/rs-dapi-client/src/transport.rs (1 hunks)
packages/rs-dapi-client/tests/mock_dapi_client.rs (4 hunks)
packages/rs-sdk/src/core/transaction.rs (2 hunks)
packages/rs-sdk/src/mock/sdk.rs (3 hunks)
packages/rs-sdk/src/platform/fetch.rs (2 hunks)
packages/rs-sdk/src/platform/fetch_many.rs (3 hunks)
packages/rs-sdk/src/platform/fetch_unproved.rs (1 hunks)
packages/rs-sdk/src/platform/transition/broadcast.rs (2 hunks)
packages/rs-sdk/src/platform/transition/purchase_document.rs (2 hunks)
packages/rs-sdk/src/platform/transition/put_contract.rs (2 hunks)
packages/rs-sdk/src/platform/transition/put_document.rs (2 hunks)
packages/rs-sdk/src/platform/transition/put_identity.rs (3 hunks)
packages/rs-sdk/src/platform/transition/top_up_identity.rs (1 hunks)
packages/rs-sdk/src/platform/transition/transfer_document.rs (2 hunks)
packages/rs-sdk/src/platform/transition/update_price_of_document.rs (2 hunks)
packages/rs-sdk/src/platform/transition/vote.rs (3 hunks)
packages/rs-sdk/src/sdk.rs (2 hunks)
packages/rs-sdk/tests/fetch/broadcast.rs (1 hunks)
packages/rs-sdk/tests/fetch/epoch.rs (2 hunks)
packages/rs-sdk/tests/vectors/contested_resource_identity_votes_not_found/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_resource_vote_states_not_found/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/document_list_document_query/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/document_list_drive_query/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/document_read/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/document_read_no_contract/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/document_read_no_document/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_data_contract_read/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_data_contract_read_not_found/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_data_contracts_1_ok_1_nx/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_data_contracts_2_nx/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_epoch_fetch/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_epoch_fetch_current/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_epoch_fetch_future/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_epoch_list/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_epoch_list_limit/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_identity_balance_read/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_identity_balance_revision_read/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_identity_contract_no_nonce_read/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_identity_public_keys_all_read/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_identity_read/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_identity_read_by_key/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_prefunded_specialized_balance_not_found/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_protocol_version_vote_count/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_protocol_version_votes_limit_2/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_protocol_version_votes_none/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_protocol_version_votes_nx/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_protocol_version_votes_zeros/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)

💤 Files with no reviewable changes (28)

packages/rs-sdk/tests/vectors/contested_resource_identity_votes_not_found/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/contested_resource_vote_states_not_found/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/document_list_document_query/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/document_list_drive_query/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/document_read/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/document_read_no_contract/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/document_read_no_document/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_data_contract_read/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_data_contract_read_not_found/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_data_contracts_1_ok_1_nx/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_data_contracts_2_nx/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_epoch_fetch/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_epoch_fetch_current/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_epoch_fetch_future/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_epoch_list/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_epoch_list_limit/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_identity_balance_read/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_identity_balance_revision_read/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_identity_contract_no_nonce_read/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_identity_public_keys_all_read/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_identity_read/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_identity_read_by_key/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_prefunded_specialized_balance_not_found/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_protocol_version_vote_count/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_protocol_version_votes_limit_2/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_protocol_version_votes_none/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_protocol_version_votes_nx/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_protocol_version_votes_zeros/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json

🧰 Additional context used

🔇 Additional comments (25)

packages/rs-dapi-client/tests/mock_dapi_client.rs (4)

1-2: LGTM: Necessary imports added.

The new imports for Uri, Address, and ExecutionResponse are correctly added and are essential for the changes made in the test function.

Line range hint 15-25: LGTM: Response renamed to inner.

The GetIdentityResponse is now correctly named inner, aligning with the new ExecutionResponse structure. The content of the response remains unchanged, preserving the test's integrity.

32-32: LGTM: Mock expectation updated correctly.

The expectation on the mock DAPI client has been properly updated to use the new execution_response. The use of clone() is appropriate to avoid moving the execution_response.

41-42: LGTM: Assertions updated correctly.

The assertions have been properly updated to compare against the new execution_response. This change correctly reflects the new ExecutionResponse structure and ensures that both result and result2 are consistent with the expected response.

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

36-36: Consider the impact of increased timeout on test execution time and reliability.

The timeout duration has been significantly increased from 500ms to 1400ms. While this change provides more time for the server to respond and aligns with the PR's objectives of enhancing error handling, it may have the following implications:

Increased test execution time, which could slow down the overall test suite.

Potential masking of performance issues that would have been caught with a shorter timeout.

To ensure this change doesn't negatively impact test reliability or execution time, please run the following script:

Consider adjusting the timeout if the results show significant negative impacts.
packages/rs-dapi-client/src/lib.rs (3)
67-67: LGTM: Consistent update of return type in trait implementation.

The change to the return type in the DapiRequest implementation for transport::TransportRequest types is consistent with the modification made to the trait definition.

10-10: LGTM: New executor module added.

The addition of the executor module aligns with the PR objectives of enhancing the SDK with request execution information.

Consider whether this module should be public. If it contains types or functions that should be accessible to users of the crate, you might want to mark it as pub mod executor;. To verify the contents and determine if it should be public, please run:

Line range hint 1-89: Summary: Enhancements to DAPI client with breaking changes.

The changes in this file introduce the new ExecutionResult type and make it accessible through the public API. This aligns with the PR objectives of providing more detailed information about request execution, including address and retry count.

Key points:

New executor module added and its contents made public.

DapiRequest trait and its implementation updated to use ExecutionResult.

These changes will provide users with more detailed information about request execution but introduce breaking changes that will require updates to dependent code.

To ensure all necessary changes have been made consistently, please run:

✅ Verification successful

To ensure all instances of the old Result type in execute functions have been removed, please run the following script:

Verification Successful: No remaining execute functions use the old Result type.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Check for any remaining references to the old Result type
# Expected result: No occurrences of the old Result type in relation to execute methods

rg --type rust 'fn execute.*Result<' packages/rs-dapi-client/src
Length of output: 66

Script:
# 
#!/bin/bash
# Description: Check for execute functions still using the old Result type
# Expected result: No execute functions returning the old Result type

rg --type rust 'fn execute[^;{]*->\s*Result<' packages/rs-dapi-client/src
Length of output: 75
packages/rs-sdk/src/platform/transition/broadcast.rs (2)
47-49: Improved response and error handling, but further refinement needed.

The changes enhance both response and error handling by mapping to inner values, which is consistent with the PR objectives. However, the TODO comment indicates that a more comprehensive approach to handling execution responses and errors is still required.

To ensure consistency in handling execution responses and errors, let's check for similar patterns:
#!/bin/bash
# Description: Search for similar execution response and error handling patterns
rg -U "execute$.*$\.await.*map\(|execution_response|execution_error" --type rust
Would you like assistance in developing a standardized approach for handling execution responses and errors across the codebase?

53-57: Consistent implementation, but consider refactoring to reduce duplication.

The changes maintain consistency with the previous segment, which is commendable. However, the repetition of both the implementation pattern and the TODO comment suggests an opportunity for refactoring.

Consider creating a helper function to encapsulate this common pattern of execution, response mapping, and error handling. This would reduce code duplication and make it easier to implement any future improvements to the error handling strategy.

Here's a suggested refactor:
async fn execute_and_map_inner<T, E>(
    request: impl DapiRequest<T, E>,
    sdk: &Sdk,
) -> Result<T, E> {
    request
        .execute(sdk, RequestSettings::default())
        .await
        .map(|execution_response| execution_response.into_inner())
        .map_err(|execution_error| execution_error.into_inner())
}

// Usage in broadcast_and_wait:
let _ = execute_and_map_inner(request.clone(), sdk).await?;
let response = execute_and_map_inner(request, sdk).await?;
This refactor would centralize the TODO, making it easier to implement improvements across all usages once a better error handling strategy is developed.

Let's check for other potential locations where this refactor could be applied:
#!/bin/bash
# Description: Search for similar execution patterns that could benefit from the refactor
rg -U "execute$.*$\.await.*map\(|execution_response|execution_error" --type rust
packages/rs-sdk/src/platform/transition/top_up_identity.rs (2)

50-51: Improved error handling, but consider further enhancements.

The change to extract the inner error from the execution result is a step in the right direction for better error handling. However, as the TODO comment suggests, there's room for improvement.

Consider implementing a more robust error handling mechanism that provides detailed information about the execution failure, such as retry count and the address used. This aligns with the PR's objective of providing detailed request execution information.

To verify the impact of this change, run the following script:

This will help identify if this error handling pattern is consistently applied across the codebase.

55-59: Consistent error handling improvement, but further refinement needed.

The changes here mirror the improvements made in the previous segment, providing consistency in error handling. The additional step to map the successful response to its inner value is a good improvement for easier access to the response data.

As indicated by the TODO comment, consider implementing a more comprehensive error and response handling mechanism. This could include:

Structured error types that encapsulate retry information and used addresses.

A response wrapper that includes metadata about the execution process.

To verify the consistency of this approach across the codebase, run the following script:

This will help identify if this pattern of handling both successful responses and errors is consistently applied across the codebase.
packages/rs-sdk/src/platform/transition/put_identity.rs (1)
85-87: Improved response and error handling, but consider addressing the TODO comment.

The changes enhance response and error handling by explicitly mapping to inner types, which aligns with the PR objectives. The simplification of the Transport error handling is consistent with the changes mentioned in the summary. However, the TODO comment suggests that further improvements to response and error handling are needed.

To ensure consistency across the codebase, let's check for similar TODO comments:
#!/bin/bash
# Description: Search for similar TODO comments related to handling execution response and errors

rg -i "TODO.*better way to handle execution response and errors" --type rust
Would you like assistance in developing a more comprehensive strategy for handling execution responses and errors?

Also applies to: 92-92
packages/rs-sdk/tests/fetch/epoch.rs (1)

2-3: LGTM: Improved import organization

The separation of VersionedGrpcResponse import enhances code readability and follows good import organization practices.
packages/rs-sdk/src/platform/transition/put_document.rs (1)
114-118: Improved response and error handling, but further enhancements needed.

The changes improve both response and error handling by extracting the inner response and error from the execution result. This aligns with the PR objectives to enhance error handling and introduce the new ExecutionResult type.

To ensure consistency across the codebase, let's check for similar response and error handling patterns:
#!/bin/bash
# Description: Check for similar response and error handling patterns in other files

# Test: Search for similar response and error handling patterns
rg -U --multiline '\.await.*\n.*\.map$\|execution_response\| execution_response\.into_inner\($\).*\n.*\.map_err$\|execution_error\| execution_error\.into_inner\($\)\?'
Consider creating helper functions or macros to standardize these response and error handling patterns across the codebase, addressing the TODO comment for better handling of execution response and errors in the future.
packages/rs-sdk/src/platform/transition/purchase_document.rs (1)

Line range hint 1-180: Summary: Improved error handling aligns with PR objectives

The changes in this file enhance error handling in the purchase_document and wait_for_response methods, which is consistent with the PR's goal of providing more detailed execution information. The modifications improve the SDK's ability to handle and report errors more effectively.

Consider implementing a unified approach for handling execution results across the SDK to ensure consistency and reduce code duplication. This could involve creating a helper function or method that can be used in various parts of the codebase.

To ensure consistency across the codebase, let's check for similar patterns in other files:

This will help verify if the error handling approach is consistent across the SDK and identify any areas that might need similar improvements.
packages/rs-sdk/src/platform/transition/vote.rs (1)
112-116: Consistent error handling improvements, but refactoring opportunity exists.

The changes in both segments enhance error handling by mapping errors and responses to their inner types, which is consistent with the improvements seen earlier. However, there are a few points to consider:

The repetition of similar code in both segments suggests an opportunity for refactoring to reduce duplication.

The TODO comments in both segments indicate that further refinements are needed for handling execution responses and errors.

Consider extracting the common execution and error handling logic into a separate function to reduce code duplication. For example:
async fn execute_request<T>(
    request: impl DapiRequest<T>,
    sdk: &Sdk,
    settings: &RequestSettings,
) -> Result<T, Error> {
    request
        .execute(sdk, settings.clone())
        .await
        .map(|execution_response| execution_response.into_inner())
        .map_err(|execution_error| execution_error.into_inner().into())
}
Then, you can use this function in both places:
let response = execute_request(request, sdk, &settings.request_settings).await?;
To ensure consistency and identify other areas for potential refactoring, let's search for similar patterns:
#!/bin/bash
# Search for similar execution patterns
rg -A 5 "execute$sdk, .+$.await.*map$|execution_response| execution_response\.into_inner\($\).*map_err$|execution_error| execution_error\.into_inner\($\)"
Would you like assistance in implementing a more robust and consistent error handling strategy across the codebase, addressing the TODO comments?

Also applies to: 135-139
packages/rs-sdk/src/platform/fetch.rs (1)

177-179: Consistent error handling improvement, repeated TODO.

The changes in this method mirror those in fetch_with_metadata, maintaining consistency in error handling across both methods. This is a good practice.

As mentioned in the previous comment, consider creating a follow-up task to address the repeated TODO comment and implement a more robust way of handling execution responses and errors for both methods simultaneously.
packages/rs-sdk/src/core/transaction.rs (1)
186-188: Consistent error handling improvement, but further refinement needed.

The changes in this method mirror those in start_instant_send_lock_stream, showing consistency in the error handling approach. However, as indicated by the TODO comment, this is still a temporary solution.

Create a task to implement a more robust error handling mechanism as suggested by the TODO comment.

Add a brief comment explaining the current error handling approach for better code maintainability.

Consider extracting this error handling pattern into a common utility function or macro, as it's used in multiple places and might be needed elsewhere in the codebase.

Example of a potential utility function:
fn handle_execution_result<T, E>(result: Result<ExecutionResponse<T>, ExecutionError<E>>) -> Result<T, Error>
where
    E: std::error::Error,
{
    result
        .map(|execution_response| execution_response.into_inner())
        .map_err(|execution_error| execution_error.into_inner())
        .map_err(|e| Error::DapiClientError(e.to_string()))
}
This function could be used in both places, reducing duplication and centralizing the error handling logic.

To ensure consistency and identify other potential places for refactoring, let's search for similar error handling patterns:
#!/bin/bash
# Search for similar error handling patterns
rg -U "\.map$.*into_inner.*$[\s\n]*\.map_err$.*into_inner.*$" --type rust
packages/rs-sdk/src/platform/fetch_many.rs (1)

190-190: Minor formatting improvement

The removal of the newline in the documentation comment slightly improves readability.
packages/rs-dapi-client/src/executor.rs (1)
41-45: Verify the correctness of the CanRetry implementation for ExecutionError.

The impl<E: CanRetry> CanRetry for ExecutionError<E> delegates the can_retry check to the inner error. Ensure that this aligns with the intended retry logic, especially if ExecutionError might contain additional context that affects whether a retry should occur.

Run the following script to list all implementations of CanRetry and verify their can_retry logic:

This will help ensure that the delegation to self.inner.can_retry() is appropriate in all contexts where ExecutionError is used.

✅ Verification successful

Verified: The CanRetry implementation for ExecutionError correctly delegates the can_retry check to the inner error, ensuring alignment with the intended retry logic.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Find all implementations of `CanRetry` and display their `can_retry` methods.

# Search for all `impl` blocks for `CanRetry` and show the `can_retry` method implementations.
rg -A 5 -e 'impl<.*> CanRetry for' --glob '!target' --glob '!**/tests/**'
Length of output: 967
packages/rs-dapi-client/src/dapi_client.rs (3)

25-26: Ensure comprehensive error context after removing the Address from Transport variant

By removing the Address field from the Transport variant of DapiClientError, it's important to verify that all necessary error context is preserved elsewhere. Make sure that the ExecutionError now adequately captures the Address information for debugging and logging purposes.

248-255: Confirm that ExecutionResponse correctly encapsulates execution details

The construction of ExecutionResponse includes the inner response, retries, and address. Verify that this encapsulation provides all necessary information for downstream consumers and that no critical execution details are omitted.

Line range hint 3-276: Overall enhancement aligns with PR objectives

The changes effectively introduce detailed execution information, including retry counts and addresses, enhancing error handling and logging capabilities. The shift from exponential backoff to a constant delay is well-justified, and the code modifications are coherent and purposeful.

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

31-31: Imports updated appropriately

The addition of ExecutionResult to the imports ensures that the new type is available for use.

packages/rs-dapi-client/src/transport.rs

packages/rs-sdk/src/platform/transition/broadcast.rs

packages/rs-sdk/src/platform/transition/put_identity.rs

packages/rs-sdk/src/platform/transition/transfer_document.rs

packages/rs-dapi-client/src/mock.rs

packages/rs-dapi-client/src/dapi_client.rs

packages/rs-sdk/src/mock/sdk.rs

coderabbitai

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)

packages/rs-dapi-client/src/dapi_client.rs (1)
127-129: LGTM with suggestion: Constant delay retry policy implemented.

The change to ConstantBuilder with a fixed delay of 10 milliseconds aligns with the PR objectives of implementing constant delays between retries. This is appropriate given that requests are directed to various nodes.

Consider making the delay duration configurable, either through the RequestSettings or as a constant at the top of the file. This would allow for easier tuning of the retry behavior without code changes. For example:
const DEFAULT_RETRY_DELAY_MS: u64 = 10;

// In the RequestSettings struct
retry_delay: Duration,

// In the retry policy setup
.with_delay(applied_settings.retry_delay)
This approach provides flexibility to adjust the delay if 10ms proves to be too short in some scenarios.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 5af8822 and 8269c1f.

📒 Files selected for processing (1)

packages/rs-dapi-client/src/dapi_client.rs (9 hunks)

🧰 Additional context used

🔇 Additional comments (7)

packages/rs-dapi-client/src/dapi_client.rs (7)
3-3: LGTM: Improved retry strategy and execution result handling.

The changes in the import statements and type definitions reflect significant improvements:

Switching from ExponentialBuilder to ConstantBuilder indicates a change in the retry strategy, likely to a constant delay between retries.

The addition of AtomicUsize suggests thread-safe counting, which could be used for tracking retries across threads.

New types (ExecutionError, ExecutionResponse, ExecutionResult) indicate a more structured approach to handling responses and errors, which should improve error handling and debugging.

These changes align well with the PR objectives of enhancing request execution information and modifying the retry policy.

Also applies to: 7-7, 16-17

45-45: LGTM: Simplified retry logic for transport errors.

The removal of the second parameter in the Transport match arm of the can_retry method is consistent with the earlier change to the DapiClientError::Transport variant. This simplification makes the retry logic cleaner and easier to understand.

137-139: Implementation of retry counter noted.

The introduction of Arc<AtomicUsize> for counting retries allows for thread-safe tracking of retry attempts. This aligns with the PR objectives of providing more detailed execution information.

As noted in a previous review comment, consider optimizing the usage of Arc<AtomicUsize> if thread safety is not a concern within this context. If it's determined that thread safety is necessary, please disregard the previous optimization suggestion.

269-271: LGTM: Thread-safe retry count incrementing.

The implementation of retry count incrementing in the notification callback is well done. The use of fetch_add with Ordering::AcqRel ensures thread-safe incrementing of the retry counter. This change contributes to accurate tracking of retry attempts, aligning with the PR objectives of providing detailed execution information.

113-113: LGTM: Enhanced execution result type.

The change in the return type to ExecutionResult aligns with the PR objectives of providing more detailed information about request execution. This will allow for better error handling and logging at the SDK level.

As this is a breaking change, please ensure that all code calling this method is updated. Run the following script to identify potential areas that need updating:
#!/bin/bash
# Description: Find potential uses of the old execute method signature

# Test: Search for execute method calls that might need updating
rg '\.execute\s*\(' --type rust
Also, please update the method documentation to reflect this change in the return type and explain the new ExecutionResult structure.

180-184: LGTM: Enhanced error and response handling.

The introduction of ExecutionError and ExecutionResponse structs with retry count and address information significantly improves the context provided for both successful and failed executions. This aligns perfectly with the PR objectives of enhancing request execution information.

To ensure consistent error handling throughout the codebase, please run the following verification script:
#!/bin/bash
# Description: Check for consistent use of ExecutionError and ExecutionResponse

# Test: Search for uses of ExecutionError and ExecutionResponse
rg 'ExecutionError|ExecutionResponse' --type rust

# Test: Search for potential places where error handling might need updating
rg 'DapiClientError::Transport' --type rust
Please review the results to ensure that error handling is consistent and that all relevant parts of the codebase have been updated to use the new ExecutionError and ExecutionResponse structs where appropriate.

Also applies to: 193-196, 251-260

25-26: LGTM: Simplified Transport error variant.

The removal of the Address parameter from the Transport variant of DapiClientError aligns with the PR objectives of restructuring error handling. This simplification should make error handling more straightforward.

To ensure this change doesn't negatively impact error handling elsewhere in the codebase, please run the following verification script:

✅ Verification successful

Verified: Removal of Address parameter is safe.

No remaining uses of the Address field in Transport errors were found. This change aligns with the PR objectives and does not negatively impact error handling elsewhere in the codebase.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Check for any remaining uses of the Address field in Transport errors

# Test: Search for potential uses of Address in Transport errors
rg -i 'transport.*address' --type rust
Length of output: 106162

coderabbitai

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (2)

packages/rs-dapi-client/src/address_list.rs (2)
24-32: LGTM: FromStr implementation for Address

The FromStr implementation for Address is well-structured and follows Rust's idiomatic error handling. It correctly leverages the existing From<Uri> implementation for Address.

Consider adding a brief doc comment explaining the expected string format for clarity:
/// Parses a string representation of an address.
/// The string should be a valid URI.
impl FromStr for Address {
    // ... (rest of the implementation)
}
95-97: LGTM: New InvalidAddressUri variant in AddressListError

The new InvalidAddressUri variant is well-integrated into the AddressListError enum. It correctly derives From<http::uri::InvalidUri> and includes appropriate serialization handling for mocks.

Consider adding a brief doc comment for the new variant to improve documentation:
/// An error occurred while parsing the address URI.
#[error("unable parse address: {0}")]
#[cfg_attr(feature = "mocks", serde(skip))]
InvalidAddressUri(#[from] http::uri::InvalidUri),

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 8269c1f and a9a96a0.

📒 Files selected for processing (4)

packages/rs-dapi-client/src/address_list.rs (4 hunks)
packages/rs-dapi-client/src/mock.rs (8 hunks)
packages/rs-dapi-client/tests/mock_dapi_client.rs (4 hunks)
packages/rs-sdk/src/mock/sdk.rs (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (3)

packages/rs-dapi-client/src/mock.rs
packages/rs-dapi-client/tests/mock_dapi_client.rs
packages/rs-sdk/src/mock/sdk.rs

🧰 Additional context used

🔇 Additional comments (2)

packages/rs-dapi-client/src/address_list.rs (2)

4-4: LGTM: Import added for http module

The new import for http is correctly added and is necessary for the new FromStr implementation and InvalidAddressUri error variant.

Line range hint 1-255: Summary of changes in address_list.rs

The changes in this file enhance the Address and AddressList structures with improved error handling and new functionality:

Added FromStr implementation for Address, allowing string parsing.

Introduced a new InvalidAddressUri variant in AddressListError for better error handling.

Added a TODO to update the From<&str> implementation for AddressList.

These changes align well with the PR objectives of enhancing error handling and improving the overall architecture. The code is well-structured and consistent with Rust best practices.

Consider addressing the suggested documentation improvements and implementing the FromStr trait for AddressList as discussed in the previous comments to further enhance the code quality and consistency.

packages/rs-dapi-client/src/address_list.rs

coderabbitai

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (2)

packages/rs-dapi-client/src/lib.rs (1)
31-38: Enhance example to demonstrate new execution information.

While the example is updated for the new error type, it could be more informative by demonstrating how to access the new execution information (retries and address):
let result = request.execute(&mut client, RequestSettings::default()).await;
match result {
    Ok(execution_response) => {
        println!("Success with {} retries at address {}", 
            execution_response.retries, 
            execution_response.address);
        let response = execution_response.response;
        // Handle response...
    }
    Err(execution_error) => {
        println!("Failed after {} retries at address {}", 
            execution_error.retries, 
            execution_error.address);
        // Handle error...
    }
}
packages/rs-sdk/tests/fetch/data_contract.rs (1)

Line range hint 1-156: Consider adding dedicated tests for execution information.

While the existing tests cover the data contract fetching functionality well, they don't verify the new execution information features introduced in this PR. Consider adding new test cases that specifically validate:

The execution result structure containing retry counts and addresses

Retry behavior with constant delays

Error scenarios with transport failures

Multiple node execution scenarios

This would ensure the new features are thoroughly tested and prevent regressions.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between a9a96a0 and 034bdb2.

📒 Files selected for processing (73)

packages/rs-dapi-client/src/lib.rs (4 hunks)
packages/rs-sdk/src/mock/sdk.rs (3 hunks)
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/data_contract.rs (1 hunks)
packages/rs-sdk/tests/vectors/check_mn_voting_prerequisities/quorum_pubkey-106-41ef25a6baff2c7a3cf388c77c826c1ffa6fb229b31c0802b1808d2c85dbc248.json (0 hunks)
packages/rs-sdk/tests/vectors/check_mn_voting_prerequisities/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_resource_identity_votes_not_found/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_resource_identity_votes_ok/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_resource_identity_votes_ok/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_resource_vote_states_not_found/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_resource_vote_states_ok/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_resource_vote_states_ok/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_resource_vote_states_with_limit/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_resource_vote_states_with_limit/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_resource_voters_for_existing_contestant/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_resource_voters_for_existing_contestant/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_resources_fields_0760ac4854fc8db803bbcbab8709f390bd31511a05e29cd3f170b48ca6b87584/quorum_pubkey-106-41ef25a6baff2c7a3cf388c77c826c1ffa6fb229b31c0802b1808d2c85dbc248.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_resources_fields_0760ac4854fc8db803bbcbab8709f390bd31511a05e29cd3f170b48ca6b87584/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_resources_fields_473cf8e4a270ced75e199e5a3e907b4df4cd66b64365d1ac77c45bcaed443a03/quorum_pubkey-106-41ef25a6baff2c7a3cf388c77c826c1ffa6fb229b31c0802b1808d2c85dbc248.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_resources_fields_473cf8e4a270ced75e199e5a3e907b4df4cd66b64365d1ac77c45bcaed443a03/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_resources_fields_948b5a301af5fc73db7fed418a4fe90f64952b4ddd6b03a7f21d029dc110af50/quorum_pubkey-106-41ef25a6baff2c7a3cf388c77c826c1ffa6fb229b31c0802b1808d2c85dbc248.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_resources_fields_948b5a301af5fc73db7fed418a4fe90f64952b4ddd6b03a7f21d029dc110af50/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_resources_fields_9f57b982b3e3b0286093d8b48ab27b87b22f67a172579913f2fec7a6b5ea31b7/quorum_pubkey-106-41ef25a6baff2c7a3cf388c77c826c1ffa6fb229b31c0802b1808d2c85dbc248.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_resources_fields_9f57b982b3e3b0286093d8b48ab27b87b22f67a172579913f2fec7a6b5ea31b7/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_resources_fields_b7df460e812b958de6e703d8ea325df9aab3448d0409ece3f0baf1d26629e44f/quorum_pubkey-106-41ef25a6baff2c7a3cf388c77c826c1ffa6fb229b31c0802b1808d2c85dbc248.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_resources_fields_b7df460e812b958de6e703d8ea325df9aab3448d0409ece3f0baf1d26629e44f/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_resources_fields_ccb199c48ee58a8bb98742b964cba7bda3b4469b740201d2628f15f926f39347/quorum_pubkey-106-41ef25a6baff2c7a3cf388c77c826c1ffa6fb229b31c0802b1808d2c85dbc248.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_resources_fields_ccb199c48ee58a8bb98742b964cba7bda3b4469b740201d2628f15f926f39347/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_resources_limit/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_resources_limit/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_resources_start_at_value/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_resources_start_at_value/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__2c32c08bee77a2d062db8d1cdb999a841b21043ccc5e7fc5ff1507f9703754a9/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__2c32c08bee77a2d062db8d1cdb999a841b21043ccc5e7fc5ff1507f9703754a9/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__67dc63814e035b20ff705840e19ebb329026b51129b8dd9cd029d26a0a77408b/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__67dc63814e035b20ff705840e19ebb329026b51129b8dd9cd029d26a0a77408b/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__a39a5f7014a616de3d7002bb4f18772c5b5d3526cb2b3c33566bd64512882669/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__a39a5f7014a616de3d7002bb4f18772c5b5d3526cb2b3c33566bd64512882669/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__c71b4f8d63f8a1357936129a226f99e349e7b21b166609cf59062e27ed68a1ee/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__c71b4f8d63f8a1357936129a226f99e349e7b21b166609cf59062e27ed68a1ee/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__cfe3ebd0f17745be80e4348304fe468f3eb8c2d17e23d310582b0b34f2d5bba7/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__cfe3ebd0f17745be80e4348304fe468f3eb8c2d17e23d310582b0b34f2d5bba7/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__fbed6654145b9fba061d182cac5a3879bc9bacb92dc9ae64ed2a07ca2ec0392f/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__fbed6654145b9fba061d182cac5a3879bc9bacb92dc9ae64ed2a07ca2ec0392f/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/document_list_bug_value_text_decode_base58/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/document_list_bug_value_text_decode_base58/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/document_list_document_query/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/document_list_drive_query/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/document_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/document_read_no_contract/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/document_read_no_document/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_contested_resource_voters_for_identity_not_found/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_contested_resource_voters_for_identity_not_found/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_contested_resources_ok/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_contested_resources_ok/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_data_contract_history_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_data_contract_history_read/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json (0 hunks)
packages/rs-sdk/tests/vectors/test_data_contract_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_data_contract_read_not_found/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_data_contracts_1_ok_1_nx/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_data_contracts_2_nx/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_epoch_fetch/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_epoch_fetch_current/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_epoch_fetch_future/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_epoch_list/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_epoch_list_limit/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_identity_balance_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_identity_balance_revision_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_identity_contract_no_nonce_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_identity_public_keys_all_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_identity_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)
packages/rs-sdk/tests/vectors/test_identity_read_by_key/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json (1 hunks)

💤 Files with no reviewable changes (23)

packages/rs-sdk/tests/vectors/check_mn_voting_prerequisities/quorum_pubkey-106-41ef25a6baff2c7a3cf388c77c826c1ffa6fb229b31c0802b1808d2c85dbc248.json
packages/rs-sdk/tests/vectors/contested_resource_identity_votes_ok/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/contested_resource_vote_states_ok/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/contested_resource_vote_states_with_limit/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/contested_resource_voters_for_existing_contestant/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/contested_resources_fields_0760ac4854fc8db803bbcbab8709f390bd31511a05e29cd3f170b48ca6b87584/quorum_pubkey-106-41ef25a6baff2c7a3cf388c77c826c1ffa6fb229b31c0802b1808d2c85dbc248.json
packages/rs-sdk/tests/vectors/contested_resources_fields_473cf8e4a270ced75e199e5a3e907b4df4cd66b64365d1ac77c45bcaed443a03/quorum_pubkey-106-41ef25a6baff2c7a3cf388c77c826c1ffa6fb229b31c0802b1808d2c85dbc248.json
packages/rs-sdk/tests/vectors/contested_resources_fields_948b5a301af5fc73db7fed418a4fe90f64952b4ddd6b03a7f21d029dc110af50/quorum_pubkey-106-41ef25a6baff2c7a3cf388c77c826c1ffa6fb229b31c0802b1808d2c85dbc248.json
packages/rs-sdk/tests/vectors/contested_resources_fields_9f57b982b3e3b0286093d8b48ab27b87b22f67a172579913f2fec7a6b5ea31b7/quorum_pubkey-106-41ef25a6baff2c7a3cf388c77c826c1ffa6fb229b31c0802b1808d2c85dbc248.json
packages/rs-sdk/tests/vectors/contested_resources_fields_b7df460e812b958de6e703d8ea325df9aab3448d0409ece3f0baf1d26629e44f/quorum_pubkey-106-41ef25a6baff2c7a3cf388c77c826c1ffa6fb229b31c0802b1808d2c85dbc248.json
packages/rs-sdk/tests/vectors/contested_resources_fields_ccb199c48ee58a8bb98742b964cba7bda3b4469b740201d2628f15f926f39347/quorum_pubkey-106-41ef25a6baff2c7a3cf388c77c826c1ffa6fb229b31c0802b1808d2c85dbc248.json
packages/rs-sdk/tests/vectors/contested_resources_limit/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/contested_resources_start_at_value/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__2c32c08bee77a2d062db8d1cdb999a841b21043ccc5e7fc5ff1507f9703754a9/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__67dc63814e035b20ff705840e19ebb329026b51129b8dd9cd029d26a0a77408b/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__a39a5f7014a616de3d7002bb4f18772c5b5d3526cb2b3c33566bd64512882669/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__c71b4f8d63f8a1357936129a226f99e349e7b21b166609cf59062e27ed68a1ee/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__cfe3ebd0f17745be80e4348304fe468f3eb8c2d17e23d310582b0b34f2d5bba7/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/contested_rss_vote_state_fields__fbed6654145b9fba061d182cac5a3879bc9bacb92dc9ae64ed2a07ca2ec0392f/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/document_list_bug_value_text_decode_base58/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_contested_resource_voters_for_identity_not_found/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_contested_resources_ok/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json
packages/rs-sdk/tests/vectors/test_data_contract_history_read/quorum_pubkey-106-74bc78bfed100cb1c7da3b1aeeaff1e5767efb0daf93c69cc8294ee246526a09.json

✅ Files skipped from review due to trivial changes (1)

packages/rs-sdk/tests/fetch/contested_resource_identity_votes.rs

🚧 Files skipped from review as they are similar to previous changes (1)

packages/rs-sdk/src/mock/sdk.rs

🧰 Additional context used

🪛 Biome

packages/rs-sdk/tests/vectors/check_mn_voting_prerequisities/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.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-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/contested_resource_identity_votes_ok/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.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-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/contested_resource_vote_states_ok/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.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-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.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-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/contested_resources_fields_0760ac4854fc8db803bbcbab8709f390bd31511a05e29cd3f170b48ca6b87584/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/contested_resources_fields_473cf8e4a270ced75e199e5a3e907b4df4cd66b64365d1ac77c45bcaed443a03/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/contested_resources_fields_948b5a301af5fc73db7fed418a4fe90f64952b4ddd6b03a7f21d029dc110af50/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/contested_resources_fields_9f57b982b3e3b0286093d8b48ab27b87b22f67a172579913f2fec7a6b5ea31b7/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/contested_resources_fields_b7df460e812b958de6e703d8ea325df9aab3448d0409ece3f0baf1d26629e44f/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/contested_resources_fields_ccb199c48ee58a8bb98742b964cba7bda3b4469b740201d2628f15f926f39347/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/contested_resources_limit/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/contested_resources_start_at_value/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.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-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.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-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.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-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.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-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.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-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.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-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.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-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/document_list_document_query/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/document_list_drive_query/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/document_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/document_read_no_contract/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/document_read_no_document/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.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-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/test_contested_resources_ok/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/test_data_contract_history_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/test_data_contract_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.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-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.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-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/test_data_contracts_2_nx/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/test_epoch_fetch/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/test_epoch_fetch_current/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/test_epoch_fetch_future/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/test_epoch_list/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/test_epoch_list_limit/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/test_identity_balance_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/test_identity_balance_revision_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/test_identity_contract_no_nonce_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/test_identity_public_keys_all_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/test_identity_read/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

packages/rs-sdk/tests/vectors/test_identity_read_by_key/quorum_pubkey-106-6599ea71a00d9d0e4aa310f02e0c6e1c1f190ed37d4da4d0346ac6803822fbef.json

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

(parse)

🔇 Additional comments (5)

packages/rs-dapi-client/src/lib.rs (2)

10-10: LGTM: New executor module follows Rust conventions.

The addition of the private executor module aligns with the PR objectives to enhance request execution information handling.

66-68: LGTM: Implementation correctly adapts to trait changes.

The implementation properly delegates to the executor while maintaining all necessary type parameters and bounds.

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

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

The test vector has been updated with a new data contract ID. Please ensure this ID corresponds to a data contract with history enabled in the test environment.

Given that this PR adds request execution information, consider enhancing this test to also verify:

The retry count in the execution result

The node address used for the request

Error scenarios where retries occur

This would help ensure the new execution information features are properly tested.

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

228-229: Consider automating test identity creation.

The TODO comment indicates that manually updating the identity value for test vectors is problematic. This manual process could lead to maintenance overhead and potential test failures when test vectors need regeneration.

Would you like me to help create a task to automate the test identity creation process? This could involve:

Generating a new identity during test setup

Persisting it for consistent test runs

Providing methods to regenerate it when needed

255-255: Verify the new ProTxHash exists on the network.

The ProTxHash has been updated, which is required when regenerating test vectors. Let's verify this masternode exists and is active.

packages/rs-dapi-client/src/lib.rs

coderabbitai

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)

packages/rs-dapi-client/src/address_list.rs (1)
Line range hint 219-228: Consider handling URI parsing errors gracefully.

The current implementation uses expect() which will panic on invalid URIs. Consider using Result to handle errors gracefully:
impl From<&str> for AddressList {
    fn from(value: &str) -> Self {
        let uri_list: Vec<Uri> = value
            .split(',')
-           .map(|uri| Uri::from_str(uri).expect("invalid uri"))
+           .filter_map(|uri| Uri::from_str(uri).ok())
            .collect();

        Self::from_iter(uri_list)
    }
}
This change will silently skip invalid URIs instead of panicking. Alternatively, if you need to handle errors explicitly, you could change the signature to return a Result.
packages/rs-dapi-client/src/mock.rs (1)

262-292: Add tests for mock serialization implementations

While the Mockable implementations for ExecutionResponse and ExecutionError are added, they need corresponding tests to verify the serialization behavior, especially considering the TODO comments about retries and address serialization.

Would you like me to help generate test cases for these implementations?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 034bdb2 and bca5dac.

📒 Files selected for processing (4)

packages/rs-dapi-client/src/address_list.rs (4 hunks)
packages/rs-dapi-client/src/lib.rs (4 hunks)
packages/rs-dapi-client/src/mock.rs (9 hunks)
packages/rs-dapi-client/tests/mock_dapi_client.rs (4 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

packages/rs-dapi-client/src/lib.rs
packages/rs-dapi-client/tests/mock_dapi_client.rs

🧰 Additional context used

📓 Learnings (1)

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

Learnt from: shumkov
PR: dashpay/platform#2259
File: packages/rs-dapi-client/src/address_list.rs:217-217
Timestamp: 2024-10-28T10:33:49.019Z
Learning: Addressing TODO comments in code may be considered out of scope for this project.

🔇 Additional comments (8)

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

4-4: LGTM!

The new import is correctly placed and necessary for handling URI-related errors.

24-32: Well-structured FromStr implementation!

The implementation follows Rust idioms with clean error handling and proper use of the map_err combinator for error type conversion.

93-99: Improved error handling with clear messages!

The error variants are well-defined with appropriate attributes and clear error messages. The new InvalidAddressUri variant properly handles URI parsing failures.

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

16-17: LGTM: Type changes align with execution context requirements

The updated imports and MockResult type definition properly reflect the new execution context requirements, maintaining consistency with the PR's objective to provide detailed request execution information.

Also applies to: 39-41

51-56: LGTM: Method signature properly updated

The expect method's signature has been correctly updated to handle ExecutionResult while maintaining the existing functionality.

77-82: LGTM: Load method properly adapted

The load method has been correctly updated to work with the new MockResult type while maintaining its original functionality.

132-137: LGTM: Error handling properly implements execution context

The error handling correctly wraps the mock error in ExecutionError with appropriate initialization of retries and address fields.

Line range hint 205-222: LGTM: Type renamed for consistency

The ExpectedResult rename properly reflects its broader purpose of handling both responses and errors.

packages/rs-dapi-client/src/mock.rs

shumkov added 7 commits October 21, 2024 19:31

feat(sdk): provide request execution information

7a77ccf

test: fix compilation errors

b85f52e

refactor: rename unwrap to into_inner

6de23ab

refactor: rename response to inner

fba6ef8

test: increase timeout to pass the test

8af6776

fix: into_inner doesn't need CanRetry bound

8e278e3

fix: errored address is none

5af8822

shumkov added this to the 1.4.2 milestone Oct 21, 2024

shumkov marked this pull request as ready for review October 21, 2024 14:08

shumkov requested a review from QuantumExplorer as a code owner October 21, 2024 14:08

coderabbitai bot reviewed Oct 21, 2024

View reviewed changes

shumkov linked an issue Oct 22, 2024 that may be closed by this pull request

Verify is a node provide a fresh data #2267

Closed

Merge branch 'v1.4-dev' into feat/sdk/execution_info

8269c1f

coderabbitai bot reviewed Oct 22, 2024

View reviewed changes

chore: don't use empty uri

a9a96a0

coderabbitai bot reviewed Oct 22, 2024

View reviewed changes

packages/rs-dapi-client/src/address_list.rs Show resolved Hide resolved

shumkov and others added 5 commits October 22, 2024 17:58

docs: fix usage example

920b50d

refactor: remove unused import

067db87

chore: update test vectors

c684b3f

chore: more test vectors

9da89a8

test(sdk): regenerate test vectors

034bdb2

coderabbitai bot reviewed Oct 28, 2024

View reviewed changes

packages/rs-dapi-client/src/lib.rs Outdated Show resolved Hide resolved

packages/rs-dapi-client/src/lib.rs Show resolved Hide resolved

refactor: re-export specific types

1bc31a9

shumkov changed the title ~~feat(sdk): provide request execution information~~ feat(sdk)!: provide request execution information Oct 28, 2024

shumkov added 3 commits October 28, 2024 19:47

fix: compilation error

8c5518f

docs: add missing docs

de7bab3

Merge branch 'v1.4-dev' into feat/sdk/execution_info

bca5dac

coderabbitai bot reviewed Oct 28, 2024

View reviewed changes

lklimek approved these changes Oct 28, 2024

View reviewed changes

packages/rs-dapi-client/src/mock.rs Show resolved Hide resolved

shumkov merged commit ffc2aea into v1.4-dev Oct 28, 2024
29 of 30 checks passed

shumkov deleted the feat/sdk/execution_info branch October 28, 2024 13:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(sdk)!: provide request execution information #2259

feat(sdk)!: provide request execution information #2259

shumkov commented Oct 21, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 21, 2024 •

edited

Loading

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

coderabbitai bot left a comment

coderabbitai bot left a comment

coderabbitai bot left a comment

coderabbitai bot left a comment

coderabbitai bot left a comment

feat(sdk)!: provide request execution information #2259

feat(sdk)!: provide request execution information #2259

Conversation

shumkov commented Oct 21, 2024 • edited by coderabbitai bot Loading

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Summary by CodeRabbit

coderabbitai bot commented Oct 21, 2024 • edited Loading

Walkthrough

Changes

Possibly related PRs

Suggested reviewers

Poem

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

shumkov commented Oct 21, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 21, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)