fix(rs-sdk): withdrawals orderBy bug (#2409) by QuantumExplorer · Pull Request #3536 · dashpay/platform

QuantumExplorer · 2026-04-24T15:51:38Z

Issue being fixed or feature implemented

Integration-test reproducer for #2409 — documents fetched from the withdrawals system contract with range/in where clauses return an empty array, and the reporter also noted asymmetric behavior between asc and desc orderBy directions.

Draft because the tests are currently failing on mainnet and the exact assertion shape still needs tuning against a dataset that actually exhibits the bug.

What was done?

Added packages/rs-sdk/tests/fetch/withdrawals_orderby.rs with two tests:

withdrawals_orderby_owner_id_asc_vs_desc — simple orderBy: [["$ownerId", asc|desc]], no where. Mirrors the UI screenshot from the bug report.
withdrawals_orderby_issue_2409_range_plus_in — exact query from the issue: where: [['transactionIndex', 'in', [0..5]], ['status', '>', 0]] with matching multi-field orderBy.

The module is gated on --features network-testing in tests/fetch/mod.rs so it only compiles/runs when explicitly invoked, and the SDK is built directly via SdkBuilder + TrustedHttpContextProvider with a hardcoded mainnet DAPI address list (the same list rs-sdk-ffi uses for its trusted-mode default) — no .env required.

How Has This Been Tested?

Run locally against mainnet:

cargo test -p dash-sdk --no-default-features --features network-testing \
  --test main withdrawals_orderby -- --nocapture

Current findings on mainnet (v3.1-dev):

Simple query: asc and desc each return 100 docs with identical sets — no asymmetry visible (likely because total mainnet withdrawal count ≤ 100, so limit=100 saturates both directions).
Range+in query: asc and desc both return 0 docs — reproduces the "empty array" half of Search for Data Contract Documents partially not working #2409, but not the asc-vs-desc asymmetry (the asymmetry may only be reproducible on testnet, where the reporter observed it, or with a dataset whose transactionIndex values fall inside [0..5]).

Open questions for reviewers / follow-up:

Should the second test target testnet instead, matching the reporter's environment?
Should the assertions be flipped from assert_ne!(asc_set, desc_set) to assert!(!result.is_empty()), which is a more direct bug signal?
Two mainnet DAPI nodes in the hardcoded list (157.66.81.162, 178.215.237.134) currently serve expired TLS certs; requests retried through other nodes so tests still run, but the list probably deserves a refresh (separate issue).

Breaking Changes

None — test-only addition, gated on a non-default feature.

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

Adds network-testing-only integration tests in `packages/rs-sdk/tests/fetch/` that hit live mainnet DAPI and exercise the withdrawals system contract with paired asc/desc queries: - `withdrawals_orderby_owner_id_asc_vs_desc`: simple `orderBy: $ownerId` with no `where`, mirroring the UI screenshot from the bug report. - `withdrawals_orderby_issue_2409_range_plus_in`: the exact query from the issue — `transactionIndex in [0..5]` + `status > 0`, orderBy on `[status, transactionIndex]`. The tests are currently failing on mainnet: - The simple query returns 100 docs in both directions with identical sets (no asymmetry visible; likely because total withdrawal count on mainnet is ≤100). - The range+in query returns 0 docs in both directions — reproducing the "empty array" half of #2409, though not the asc/desc asymmetry. Pushing as a draft so the assertion shape / dataset choice can be iterated on before this is suitable as a regression gate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-04-24T15:51:46Z

Warning

Rate limit exceeded

@QuantumExplorer has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 56 minutes and 37 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 56 minutes and 37 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2c6123a6-7a04-4bb8-aa69-cfd8496a3dba

📥 Commits

Reviewing files that changed from the base of the PR and between 32edf1c and ced8573.

📒 Files selected for processing (2)

packages/rs-drive/src/query/mod.rs
packages/rs-drive/tests/query_tests.rs

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch claude/eloquent-sutherland-d141c9

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.}

…document queries (#2409) Replaces the mainnet-dependent rs-sdk reproducer with two deterministic rs-drive integration tests and fixes the two underlying bugs in `DriveDocumentQuery::get_non_primary_key_path_query`: 1. `recursive_create_query` was called with `order_by = None` in the `last_clause == None` branch, so queries with no `where` clauses that used an index purely for ordering silently coerced `desc` back to the index's intrinsic direction. Forward `Some(&self.order_by)` to match the sibling branch that already did so. 2. When both `in` and `range` clauses were present, `last_clause` was unconditionally set to the `in` clause regardless of each field's position in the chosen index. Queries whose `in` was on a leaf-side property (e.g. `status > 0 AND transactionIndex in [..]` on the `[status, transactionIndex]` index) built a path query whose primary query operated on one field while the path terminated at another, returning `[]`. Pick `last_clause` vs `subquery_clause` based on each field's index position instead. New tests (`packages/rs-drive/tests/query_tests.rs`): - `test_withdrawals_query_orderby_asc_vs_desc_owner_id_limit_10_of_15` - `test_withdrawals_query_range_plus_in_issue_2409` All 33 tests in `query_tests.rs` pass, including existing withdrawal tests with hardcoded root-hash and expected-ID assertions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

thepastaclaw · 2026-04-24T17:12:16Z

✅ Review complete (commit ced8573)

codecov · 2026-04-24T17:26:13Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.19%. Comparing base (2b657d8) to head (ced8573).
⚠️ Report is 421 commits behind head on v3.1-dev.

Additional details and impacted files

@@            Coverage Diff            @@
##           v3.1-dev    #3536   +/-   ##
=========================================
  Coverage     88.18%   88.19%           
=========================================
  Files          2474     2474           
  Lines        298763   298738   -25     
=========================================
- Hits         263477   263461   -16     
+ Misses        35286    35277    -9

Components	Coverage Δ
dpp	`87.94% <ø> (+<0.01%)`	⬆️
drive	`87.36% <100.00%> (+<0.01%)`	⬆️
drive-abci	`89.94% <ø> (+<0.01%)`	⬆️
sdk	`∅ <ø> (∅)`
dapi-client	`∅ <ø> (∅)`
platform-version	`∅ <ø> (∅)`
platform-value	`92.26% <ø> (ø)`
platform-wallet	`∅ <ø> (∅)`
drive-proof-verifier	`55.66% <ø> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

thepastaclaw

Code Review

Both Claude and Codex reviewed the rs-drive diff (orderBy direction + in/range index position fix and its regression test) and surfaced no actionable findings. The change is a localized correctness fix in document query construction with a targeted reproducer test, and no blocking issues, suggestions, or nitpicks remain after validation.

Reviewed commit: ced8573

shumkov · 2026-04-29T11:59:39Z

@QuantumExplorer please update PR title

github-actions Bot added this to the v3.1.0 milestone Apr 24, 2026

QuantumExplorer marked this pull request as ready for review April 24, 2026 17:11

QuantumExplorer requested a review from shumkov as a code owner April 24, 2026 17:11

thepastaclaw reviewed Apr 25, 2026

View reviewed changes

shumkov approved these changes Apr 29, 2026

View reviewed changes

QuantumExplorer changed the title ~~test(rs-sdk): withdrawals orderBy bug reproducer (#2409)~~ fix(rs-sdk): withdrawals orderBy bug (#2409) Apr 29, 2026

QuantumExplorer merged commit b4a0e71 into v3.1-dev Apr 29, 2026
40 checks passed

QuantumExplorer deleted the claude/eloquent-sutherland-d141c9 branch April 29, 2026 16:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(rs-sdk): withdrawals orderBy bug (#2409)#3536

fix(rs-sdk): withdrawals orderBy bug (#2409)#3536
QuantumExplorer merged 2 commits into
v3.1-devfrom
claude/eloquent-sutherland-d141c9

QuantumExplorer commented Apr 24, 2026

Uh oh!

coderabbitai Bot commented Apr 24, 2026 •

edited

Loading

Rate limit exceeded

Uh oh!

thepastaclaw commented Apr 24, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Apr 24, 2026 •

edited

Loading

Uh oh!

thepastaclaw left a comment

Uh oh!

shumkov commented Apr 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

QuantumExplorer commented Apr 24, 2026

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Uh oh!

coderabbitai Bot commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Uh oh!

thepastaclaw commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

shumkov commented Apr 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented Apr 24, 2026 •

edited

Loading

thepastaclaw commented Apr 24, 2026 •

edited

Loading

codecov Bot commented Apr 24, 2026 •

edited

Loading