Skip to content

Comments

revert(sdk): deserialization error due to outdated contract cache#3114

Open
shumkov wants to merge 1 commit intov3.1-devfrom
revert-3052-fix/sdk/outdated-data-contract-cache
Open

revert(sdk): deserialization error due to outdated contract cache#3114
shumkov wants to merge 1 commit intov3.1-devfrom
revert-3052-fix/sdk/outdated-data-contract-cache

Conversation

@shumkov
Copy link
Collaborator

@shumkov shumkov commented Feb 19, 2026

Reverts #3052

Summary by CodeRabbit

  • Bug Fixes

    • Improved protocol error messaging and handling for clearer error representation.
  • Refactor

    • Simplified proof and document fetch operations through internal code restructuring.
    • Removed contract update methods from context provider interfaces.
    • Streamlined error handling flows across verification and fetch operations.

@github-actions github-actions bot added this to the v3.1.0 milestone Feb 19, 2026
@github-actions
Copy link

✅ gRPC Query Coverage Report

================================================================================
gRPC Query Coverage Report - NEW QUERIES ONLY
================================================================================

Total queries in proto: 53
Previously known queries: 47
New queries found: 6

================================================================================

New Query Implementation Status:
--------------------------------------------------------------------------------
✓ getAddressInfo                                /home/runner/work/platform/platform/packages/rs-sdk/src/platform/query.rs
✓ getAddressesBranchState                       /home/runner/work/platform/platform/packages/rs-sdk/src/platform/address_sync/mod.rs
✓ getAddressesInfos                             /home/runner/work/platform/platform/packages/rs-sdk/src/platform/fetch_many.rs
✓ getAddressesTrunkState                        /home/runner/work/platform/platform/packages/rs-sdk/src/platform/query.rs
✓ getRecentAddressBalanceChanges                /home/runner/work/platform/platform/packages/rs-sdk/src/platform/query.rs
✓ getRecentCompactedAddressBalanceChanges       /home/runner/work/platform/platform/packages/rs-sdk/src/platform/query.rs

================================================================================
Summary:
--------------------------------------------------------------------------------
New queries implemented: 6 (100.0%)
New queries missing: 0 (0.0%)

Total known queries: 53
  - Implemented: 50
  - Not implemented: 2
  - Excluded: 1

Not implemented queries:
  - getConsensusParams
  - getTokenPreProgrammedDistributions

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 19, 2026

📝 Walkthrough

Walkthrough

This PR removes the update_data_contract method from the ContextProvider trait and all its implementations across the codebase, refactors protocol error handling to use structured string messages, and significantly restructures document fetch operations to use a retry-driven inlined pattern instead of separate request functions.

Changes

Cohort / File(s) Summary
ContextProvider trait method removal
packages/rs-context-provider/src/provider.rs, packages/rs-sdk-trusted-context-provider/src/provider.rs, packages/rs-sdk/src/mock/provider.rs, packages/wasm-sdk/src/context_provider.rs
Removed the update_data_contract method from the ContextProvider trait definition and all implementations, eliminating the ability to dynamically update cached data contracts.
Protocol error representation refactoring
packages/rs-drive-proof-verifier/src/error.rs, packages/rs-drive-proof-verifier/src/proof.rs, packages/rs-drive-proof-verifier/src/unproved.rs
Reworked protocol error handling to use structured ProtocolError { error: String } variant instead of wrapping ProtocolError(ProtocolError), and updated all error mapping paths to convert errors to string messages.
Mock SDK proof error expectations removal
packages/rs-sdk/src/mock/sdk.rs
Removed the proof_error_expectations field, expect_fetch_proof_error method, and related test simulation logic for deserialization errors in MockDashPlatformSdk.
Document fetch logic refactoring
packages/rs-sdk/src/platform/fetch.rs, packages/rs-sdk/src/platform/fetch_many.rs
Restructured document fetch operations from using separate fetch_request/fetch_many_request helper functions to inline retry-driven closures with integrated parsing, address/retry propagation, and error handling.
Query and test cleanup
packages/rs-sdk/src/platform/documents/document_query.rs, packages/rs-sdk/tests/fetch/mock_document_contract_refresh.rs, packages/rs-sdk/tests/fetch/mod.rs
Removed the clone_with_contract method from DocumentQuery and deleted the entire mock_document_contract_refresh test module along with its module import, effectively removing contract refresh retry logic from tests.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • QuantumExplorer

Poem

🐰 Contracts no longer dance in cache,
Errors now wear strings with panache,
Fetch flows inlined, clean and tight,
Tests retire—the logic feels right! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: reverting PR #3052 which addressed deserialization errors from an outdated contract cache. The summary of changes confirms the removal of update_data_contract methods and related error handling, aligning with a revert operation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch revert-3052-fix/sdk/outdated-data-contract-cache

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/rs-sdk/src/platform/fetch_many.rs (1)

340-340: Remove commented-out code.

Line 340 has a leftover commented-out line that appears to be debug/development residue.

🧹 Proposed cleanup
-            // let object: Option<BTreeMap<K,Document>> = sdk
-            let documents = sdk
+            let documents = sdk
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/rs-sdk/src/platform/fetch_many.rs` at line 340, Remove the leftover
commented-out debug line "let object: Option<BTreeMap<K,Document>> = sdk" in
fetch_many.rs (the commented declaration near the fetch_many logic); simply
delete that commented line so the file contains no dead/commented-out
development residue while leaving surrounding code and function behavior
unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/rs-sdk/src/platform/fetch_many.rs`:
- Line 340: Remove the leftover commented-out debug line "let object:
Option<BTreeMap<K,Document>> = sdk" in fetch_many.rs (the commented declaration
near the fetch_many logic); simply delete that commented line so the file
contains no dead/commented-out development residue while leaving surrounding
code and function behavior unchanged.

@PastaPastaPasta
Copy link
Member

Why?

@shumkov shumkov requested a review from lklimek February 23, 2026 09:20
@shumkov
Copy link
Collaborator Author

shumkov commented Feb 23, 2026

@PastaPastaPasta because it's actually not fixing the problem. Actual fix is here #3071

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants