Skip to content

feat: complete empty vs provider-fail in recover() - #9938

Open
pranishnepal wants to merge 1 commit into
masterfrom
pranishnepal/wcn-3036-bitgojs-honest-empty-vs-provider-failed-in-recover-remaining
Open

pranishnepal wants to merge 1 commit into
masterfrom
pranishnepal/wcn-3036-bitgojs-honest-empty-vs-provider-failed-in-recover-remaining

Conversation

@pranishnepal

Copy link
Copy Markdown
Contributor

Context

This PR is a follow-up to PR #9932 (which implemented honest "empty vs provider-failed" classification in recover() for SOL, XLM, and IOTA). It completes the migration across all 17 remaining coin modules (and their abstract bases) that still conflated wallet-empty states with node/explorer/RPC failures.


Key Refinement Highlights

Beyond the mechanical retypes, this PR addresses critical edge cases and bugs discovered during rigorous review of the uncommitted state:

  • Cosmos 404 Dead-Code Bug Fix (abstract-cosmos): superagent v9 rejects on non-2xx (including 404). The existing base code and overrides (injective, islm, zeta) checked response.status === 404 downstream, which was dead code (empty accounts threw generic provider errors). Fixed by catching the 404 in getAccountFromNode's catch block and properly throwing ErrorNoInputToRecover.
  • Underfunded vs. Empty Splits (ICP & RUNE): Split balance - fee <= 0 checks (which historically collapsed empty and underfunded) into two separate paths: balance <= 0 throws ErrorNoInputToRecover, while 0 < balance <= fee throws a plain Error (needs gas).
  • RUNE TypeError Guard: Added an empty balances array guard in rune.ts to prevent a raw TypeError on balances[0].amount for empty accounts, routing it to ErrorNoInputToRecover.
  • XRP Missing Empty Site: Classified "Does not have Trustline with ${issuer}" as ErrorNoInputToRecover (empty of that token), aligning it with the "Does not have funds to recover" zero-balance check.
  • RPC/Query Helper Wraps (SUI/TON/TRX): Wrapped shared low-level helpers (makeRPC for SUI, getFeeEstimate for TON, recoveryPost/Get for TRX) in RecoveryProviderError to comprehensively cover all node/explorer boundaries.

Verification & Testing

Negative tests asserting by type (ErrorNoInputToRecover / RecoveryProviderError) have been added or upgraded for all affected coins, and verified green

This is a follow-up to PR #9932
(#9932)
which introduced honest "empty vs provider-failed" classification in
recover(). This commit closes the loop by applying the same fix to all
17 remaining coin modules (and their abstract bases) that still
conflated the two.

Key changes across SUI, ADA, NEAR, POLYX, DOT, Substrate base, Cosmos
base (and subclasses), XTZ, ICP, VET, HBAR, TRX, ALGO, TON, RUNE, STX,
and XRP:
- Retype verifiably-empty throws to ErrorNoInputToRecover
- Wrap provider query boundaries in RecoveryProviderError
- Convert scan loop empty string-match checks to ErrorNoInputToRecover
- Add type-based negative assertions and provider-failure tests per coin

Ticket: WCN-3036
@linear-code

linear-code Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

WCN-3036

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Unit tests are failing on Node 26.x (Current release line, non-blocking). This is not an LTS version yet, so it does not block merge, but it signals an incompatibility to fix before Node 26.x becomes LTS.

View run

@pranishnepal
pranishnepal marked this pull request as ready for review October 10, 2026 15:04
@pranishnepal
pranishnepal requested review from a team as code owners October 10, 2026 15:04

This branch has not been deployed

No deployments
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.

1 participant