Health check fix#1584
Merged
Merged
Conversation
getDLWalletId() was searching for WalletType 14 but chia-blockchain defines DATA_LAYER as 11 (and has since 2022). This meant the entire transaction health and recovery system — rejected tx detection, auto-clearing, DL unconfirmed tx gating — was silently disabled on real nodes. The original integration test was tautological: it stubbed type 14 in the mock response then asserted CADT found type 14. It also ran under USE_SIMULATOR=true which short-circuits getDLWalletId() to return '2' without ever hitting the RPC path. - Extract findDLWalletInResponse() pure function with named constant CHIA_WALLET_TYPE_DATA_LAYER = 11, testable without simulator bypass - Add __test_resetDLWalletCache() to prevent cross-test cache leakage - Replace tautological test with findDLWalletInResponse unit tests that validate the constant value and reject adjacent type values - Add live-api test (wallet-health.live.spec.js) that calls the real Chia wallet RPC get_wallets and verifies CADT's constant matches - Run wallet-health live test in both v1 and v2 CI jobs as an early sanity check before the longer org-creation tests
- Fall back on 403 (not just 404) when v2 is disabled in v1-only CI - Move wallet-health test after org creation so the DataLayer wallet exists (Chia auto-creates it on first datalayer interaction)
fix: correct DataLayer wallet type constant from 14 to 11
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Medium Risk
Touches wallet RPC discovery logic used by health checks and potentially other runtime paths; incorrect enum handling or caching behavior could affect wallet availability reporting in production.
Overview
Fixes DataLayer wallet discovery by switching the
get_walletslookup to the correct chia-blockchainWalletType.DATA_LAYERvalue (11), and extracting the parsing logic into a new pure helperfindDLWalletInResponse.Adds test hooks (
__test_resetDLWalletCache) and updates/extends integration coverage around wallet-id discovery, plus introduces a new live test (wallet-health.live.spec.js) that validates both CADT’s/health/walletresponse and the underlying wallet RPCget_walletstypes. CI live workflows now run this new wallet health sanity check in both v1 and v2 live test jobs via the newtest:v2:live:wallet-healthscript.Reviewed by Cursor Bugbot for commit 374c9f3. Bugbot is set up for automated code reviews on this repo. Configure here.