Skip to content

fix(sdk): remove Document fetch_many override referencing removed parse_proof#3179

Merged
lklimek merged 1 commit into
v3.1-devfrom
fix/sdk-fetch-many-parse-proof
Mar 4, 2026
Merged

fix(sdk): remove Document fetch_many override referencing removed parse_proof#3179
lklimek merged 1 commit into
v3.1-devfrom
fix/sdk-fetch-many-parse-proof

Conversation

@shumkov
Copy link
Copy Markdown
Collaborator

@shumkov shumkov commented Mar 4, 2026

Issue being fixed or feature implemented

Fixes compilation failures introduced by revert commit d820fe2 (#3114) across multiple CI checks: dash-sdk linting/tests, wasm-sdk linting/tests, rs-sdk-ffi tests, iOS build, and check-each-feature.

What was done?

The revert commit d820fe2 brought back a custom Document FetchMany::fetch_many implementation that called Sdk::parse_proof, which was removed in #3141. This caused compilation failures in all crates depending on dash-sdk.

Removed the custom fetch_many override entirely. The trait's default fetch_many_with_metadata_and_proof implementation (which correctly uses parse_proof_with_metadata_and_proof) handles this case identically.

Also removed the now-unnecessary #[async_trait::async_trait] attribute on the impl block, matching the convention of other FetchMany impls that don't override methods.

How Has This Been Tested?

  • cargo check -p dash-sdk passes
  • cargo clippy -p dash-sdk passes with no warnings

Breaking Changes

None. The default trait implementation provides the same behavior.

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

… parse_proof method

The revert commit d820fe2 brought back a custom Document FetchMany::fetch_many
implementation that called Sdk::parse_proof, which was removed in #3141. This
caused compilation failures across dash-sdk, wasm-sdk, rs-sdk-ffi, and iOS builds.

Remove the custom override entirely so the trait's default
fetch_many_with_metadata_and_proof implementation (which correctly uses
parse_proof_with_metadata_and_proof) is used instead.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@shumkov shumkov requested a review from QuantumExplorer as a code owner March 4, 2026 16:22
@github-actions github-actions Bot added this to the v3.1.0 milestone Mar 4, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 4, 2026

✅ 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
Copy Markdown
Contributor

coderabbitai Bot commented Mar 4, 2026

📝 Walkthrough

Walkthrough

The pull request removes the async fetch_many implementation from the FetchMany<Identifier, Documents> trait impl for Document. This eliminates approximately 38 lines including query extraction, retry logic, response parsing, and ExecutionResponse handling. The trait type association persists, but the concrete fetch operation is removed.

Changes

Cohort / File(s) Summary
Async FetchMany Removal
packages/rs-sdk/src/platform/fetch_many.rs
Deleted async fetch_many method from FetchMany<Identifier, Documents> impl, including document query extraction, retry loop, and response parsing logic. Type Request association remains but concrete fetch path is eliminated.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A rabbit hopped through code so dense,
Removing paths that made no sense,
The fetch was trimmed, the impl stayed,
Simpler logic, neatly made! 🐰✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: removing a Document fetch_many override that was broken due to a removed parse_proof dependency.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/sdk-fetch-many-parse-proof

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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 4, 2026

✅ DashSDKFFI.xcframework built for this PR.

SwiftPM (host the zip at a stable URL, then use):

.binaryTarget(
  name: "DashSDKFFI",
  url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
  checksum: "a0d374aae45489d3b077919cf6e218d1affb8f94ae471234b76460fa96c46b4b"
)

Xcode manual integration:

  • Download 'DashSDKFFI.xcframework' artifact from the run link above.
  • Drag it into your app target (Frameworks, Libraries & Embedded Content) and set Embed & Sign.
  • If using the Swift wrapper package, point its binaryTarget to the xcframework location or add the package and place the xcframework at the expected path.

@lklimek lklimek merged commit 44dbf4d into v3.1-dev Mar 4, 2026
48 checks passed
@lklimek lklimek deleted the fix/sdk-fetch-many-parse-proof branch March 4, 2026 16:56
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.

2 participants