fix(sdk): failed to deserialize consensus error

[shumkov]

Issue being fixed or feature implemented

When SDK receives consensus error we get another error:

Protocol error: platform deserialization error: unable to deserialize ConsensusError : UnexpectedVariant { type_name: "ConsensusError", allowed: Range { min: 0, max: 4 }, found: 65 }

What was done?

• Use base64 decoded bytes

How Has This Been Tested?

• Covered with unit tests

Breaking Changes

None

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 Dependencies

  • Added assert_matches library version 1.5.0 to support testing

• Testing

  • Enhanced error handling test coverage
  • Added multiple unit tests for error scenarios, including AlreadyExists and consensus errors
  • Improved error deserialization and fallback mechanisms

[coderabbitai]

Walkthrough

This pull request introduces improvements to error handling in the Rust SDK. A new dependency assert_matches is added to Cargo.toml, and the error.rs file is updated with enhanced error deserialization logic for DapiClientError. The changes include adding unit tests to verify error handling scenarios, specifically focusing on AlreadyExists and consensus error transformations.

Changes

File	Change Summary
packages/rs-sdk/Cargo.toml	Added assert_matches = "1.5.0" dependency
packages/rs-sdk/src/error.rs	- Updated DapiClientError deserialization logic - Added new test module with multiple test functions for error handling

Sequence Diagram

sequenceDiagram
    participant SDK as Rust SDK
    participant Error as Error Handler
    participant Metadata as Consensus Metadata
    
    SDK->>Error: Process DapiClientError
    Error->>Metadata: Convert to bytes
    Metadata-->>Error: Byte representation
    Error->>Error: Attempt deserialization
    alt Deserialization Successful
        Error-->>SDK: Specific Error Variant
    else Deserialization Fails
        Error-->>SDK: Generic Error Message
    end

Loading

Possibly related PRs

• feat(sdk)!: detect stale nodes #2254: Modifies Error enum in the same file with a new error variant.
• fix(dashmate): status command fails if drive is not running #2364: Introduces a new error class related to service status checks.

Suggested reviewers

• QuantumExplorer

Poem

🐰 In the realm of Rust, where errors dance,
A new dependency takes its stance.
assert_matches joins the fray,
Testing errors in a robust way.
SDK's resilience now enhanced with care! 🛡️

---

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

• X
• Mastodon
• Reddit
• LinkedIn

🪧 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 generate docstrings to generate docstrings for this PR. (Beta)
• @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: 0

🧹 Nitpick comments (2)

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

91-109: Consider introducing a specialized error variant or detailed message where indicated by the TODO.

You have a TODO at lines 107-108 hinting at creating a specific error to handle invalid consensus error encodings. Instead of defaulting to Self::Generic(...), introducing a dedicated variant like Error::InvalidConsensusError(...) can improve clarity and make error handling more robust across the codebase.

---

234-255: Supplement coverage of fixture-based ConsensusError with negative test scenarios.

While this fixture-based test is valuable, consider adding a scenario where the fixture data is corrupted or truncated, ensuring that the deserialization fails gracefully and returns the correct fallback error path. This can confirm the defensive behavior in real-world edge cases.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6d637fe and f5cbff9.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (2)

• packages/rs-sdk/Cargo.toml (1 hunks)
• packages/rs-sdk/src/error.rs (2 hunks)

🔇 Additional comments (3)

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

186-194: Confirm coverage scope for the AlreadyExists variant.

The test checks if the AlreadyExists variant is returned when the error code is AlreadyExists. Consider adding an assertion about the error message itself to ensure it matches any expected message format. This can help detect unexpected message changes or translations in the future.

---

196-232: Good usage of assert_matches to validate consensus error transformations.

The test correctly validates that valid bytes produce the expected ConsensusError, ensuring robust error handling. This is a good demonstration of using assert_matches. Keep up the comprehensive testing approach to ensure new error types don't regress.

packages/rs-sdk/Cargo.toml (1)

59-59: New test dependency is appropriate.

The assert_matches crate is well-suited for pattern-matching-based assertions and keeps tests more expressive and concise. This addition aligns with Rust testing best practices.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

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

91-109: LGTM! Robust error handling implementation.

The improved error handling for consensus error deserialization is well-structured with:

• Proper byte conversion before deserialization
• Appropriate fallback paths for deserialization failures
• Debug logging for troubleshooting

Consider addressing the TODO comment.

The TODO comment on line 107 suggests introducing a specific error for invalid consensus error encoding cases.

Would you like me to help create a specific error variant for this case? This would improve error handling granularity and make it easier for clients to handle this specific failure mode.

---

170-257: LGTM! Comprehensive test coverage.

The test module is well-structured with:

• Clear test organization in submodules
• Specific test cases for each error scenario
• Good use of fixtures for real-world cases
• Proper use of assert_matches for cleaner assertions

Consider documenting the test fixture.

The base64 encoded test fixture on line 236 would benefit from a comment explaining what consensus error case it represents and how it was generated.

Add a comment before the fixture:

+        // This fixture represents a consensus error for an IdentityAssetLockProofLockedTransactionMismatch
+        // Generated from a real-world example where...
         let consensus_error_bytes = base64::engine::general_purpose::STANDARD.decode("ATUgJOJEYbuHBqyTeApO/ptxQ8IAw8nm9NbGROu1nyE/kqcgDTlFeUG0R4wwVcbZJMFErL+VSn63SUpP49cequ3fsKw=").expect("decode base64");

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f5cbff9 and 4022345.

📒 Files selected for processing (1)

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

[pauldelucia]

Tested in Dash Evo Tool

I pushed this fix to test/testWithoutSpan2 as well without the additional tests 8a000df