#35224 wallet: importdescriptors should output the request on error

ReviewabilityReviewsAgreementSize
#35224 kevkevinpal wallet: importdescriptors should output the request on errorReady0Crickets1 + 3 tests
  • Help users identify which descriptor failed when batch-importing descriptors with a missing timestamp
  • Prevents having to guess which item caused the entire batch import call to abort
  • CI is failing, so verify whether the test failure is related before doing an in-depth review
  • No review verdicts recorded
  • No substantive comments or reviews from external contributors yet
  • Awaiting initial review
  • 1 lines added or modified outside tests
  • 3 lines added or modified in tests
  • 3 lines removed in total
  • 2 files, 1 commits
Wallet#78 of 92
P3 · bug fix
  • P3 because it improves error diagnostics for a specific failure mode during batch descriptor imports
  • Provides modest usability value to operators debugging import errors without affecting core wallet logic
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Updates the `importdescriptors` RPC error message when a timestamp is missing to include the serialized JSON of the rejected request. A functional test is updated to check the new error string.

Problem: When batch-importing multiple descriptors via `importdescriptors`, omitting a required timestamp throws a generic RPC error (`Missing required timestamp field for key`) that fails the entire call without indicating which descriptor in the batch caused the error.

Evidence of importance:

  • Author description: "When importdescriptors is given multiple requests, a missing timestamp still fails the whole call with a static error... That does not say which request failed."

Risk of a mistake: low (wallet, rpc_api, tests). Only changes the descriptive text in an RPC error message and adjusts an associated functional test.

Changed since the previous assessment: Dependency #34861 has been merged into master, removing the pending dependency requirement.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • reviewer: initial review
  • reviewer: review of the PR 2026-10-10

Agreement

Crickets

no substantive comment either way

Thread notes: Initial record. No external reviewers have commented yet; author noted an unrelated 32-bit ARM CI failure. / Initial record. No external participants or reviews yet; author noted an unrelated CI failure.

Objections, suggestions, and questions: none recorded.

Files

5 lines under test/bench/ci.

  • test/functional/wallet_importdescriptors.py +3/-2
  • src/wallet/rpc/backup.cpp +1/-1

Card

The short text the ranking pass compares across PRs.

This PR modifies importdescriptors to include the serialized JSON of the offending request when throwing an RPC error for a missing timestamp. This helps users importing multiple descriptors at once identify which item in the array failed. No other participants have commented on the issue.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-25T01:14, confidence high, input hash 6e1fbfdd268eef86
thread read through 2026-10-10 (1 statements), head ad0c67a91e
labels: Wallet

Categories

Wallet

P3 · bug fix

Improves batch descriptor import error handling by including the rejected request in the error message. When multiple descriptors are imported together, omitting a timestamp previously threw a static error that failed the call without identifying which descriptor caused the failure.

Membership: Carries the Wallet label and modifies descriptor import error reporting in src/wallet/rpc/backup.cpp.

Factors: security/stability 0, bug 1, performance 0, user value 1, leverage 0