Repository navigation
fix(sdk): restore JS DAPI client node discovery after Core V24 - #5609
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Update dependency constraints and lockfile .pnp.cjs, packages/bench-suite/package.json, packages/dapi/package.json, packages/dash-spv/package.json, packages/dashmate/package.json, packages/js-dapi-client/package.json, packages/js-dash-sdk/package.json, packages/platform-test-suite/package.json, packages/wallet-lib/package.json, packages/wasm-dpp/package.json |
The listed package manifests now require @dashevo/dashcore-lib ~0.26.1. The lockfile resolves version 0.26.1 and records its @noble/hashes dependency. |
DAPI client testnet addresses
| Layer / File(s) | Summary |
|---|---|
Replace testnet address whitelist packages/js-dapi-client/lib/networkConfigs.js, packages/js-dapi-client/lib/dapiAddressProvider/createDAPIAddressProviderFromOptions.js |
The testnet whitelist changes from 22 addresses to 42. Comments describe the whitelist and state that default discovery fails with NoAvailableAddressesError if no listed addresses remain in the masternode list. |
Priority:
Estimated code review effort: 2 (Simple) | ~8 minutes
Change: Bug fix
Suggested reviewers: shumkov, thepastaclaw
Merge Risk: 
db886
-
The dependency upgrade and testnet whitelist are internally consistent and introduce no confirmed merge-blocking risk.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: restoring JS DAPI client node discovery after Dash Core V24. This matches the whitelist refresh and dashcore-lib upgrade. Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files. (10 skipped: 9 unsupported, 1 too large.)
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
|
🔍 Review in progress — actively reviewing now (commit db88654) · triage: normal |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
The refreshed whitelist entries are correctly formatted for the existing DAPI provider, but the reviewed head still pins every relevant workspace to dashcore-lib ~0.22. The pinned parser rejects Core V24 CbTx v4 payloads before whitelist filtering runs, so the PR's stated goal of restoring default discovery is not functional until the compatible dashcore-lib release and corresponding lockfile/cache updates are added. The changed whitelist also retains stale documentation and a known fail-hard behavior when the snapshot becomes outdated.
🔴 1 blocking | 🟡 2 suggestion(s)
1 finding(s) not shown inline (the lines are not part of this PR's diff)
🟡 Suggestion: An expired whitelist snapshot still causes a complete discovery failure
packages/js-dapi-client/lib/dapiAddressProvider/SimplifiedMasternodeListDAPIAddressProvider.js:58-63
When the configured snapshot has no match in the current SML, the provider replaces the complete address set with an empty array and default discovery fails with NoAvailableAddressesError. This is the same failure mode that required the current refresh and will recur when testnet masternode membership changes again. If the hard whitelist is intentionally retained as a trust boundary, track the required rotation or fallback design; silently leaving this known failure mode untracked makes the fix temporary by construction.
source: glm-5.3-flash (phase1-reviewer: general)
Review provenance
Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
lowbygpt-6.1-sol(effort low) — The change is a small, contained refresh of the testnet DAPI address whitelist, with straightforward discovery behavior and no changes to parsing, consensus, cryptography, or funds handling. - Phase 1 reviewers:
glm-5.3-flash— general (completed, effort high); agentphase1-reviewer - Phase 1 model:
glm-5.3-flash— zai quota: 5h 96% left, weekly 96% left; passed overgemini-3.8-flash-high(antigravity quota lookup failed) - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort medium); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/js-dapi-client/lib/networkConfigs.js`:
- [BLOCKING] packages/js-dapi-client/lib/networkConfigs.js:17-18: Whitelist refresh cannot restore discovery while dashcore-lib remains on 0.22
The whitelist is applied only after the simplified masternode list has been parsed. At this head, packages/js-dapi-client and the other affected workspaces still depend on @dashevo/dashcore-lib ~0.22.0, and the pinned parser rejects the additional Core V24 CbTx v4 payload bytes with 'raw payload is bigger than expected'. As a result, default testnet discovery continues retrying at the parser stage and never reaches this refreshed whitelist. Add the released dashcore-lib version containing the required CbTx and ExtAddr fixes consistently across the affected workspaces, then update yarn.lock and the Yarn cache before treating this discovery fix as complete.
- [SUGGESTION] packages/js-dapi-client/lib/networkConfigs.js:12-16: Whitelist comment no longer describes the refreshed list
The comment still says that PoSe is unavailable and that the list contains reliable DCG masternodes, but the regenerated array is produced by selecting every enabled masternode that passes versionCheck from the documented endpoint. This makes the comment misleading precisely where the snapshot's maintenance behavior is documented. Update this comment and the duplicate comment in createDAPIAddressProviderFromOptions.js to describe the current snapshot and its regeneration requirement.
In `packages/js-dapi-client/lib/dapiAddressProvider/SimplifiedMasternodeListDAPIAddressProvider.js`:
- [SUGGESTION] packages/js-dapi-client/lib/dapiAddressProvider/SimplifiedMasternodeListDAPIAddressProvider.js:58-63: An expired whitelist snapshot still causes a complete discovery failure
When the configured snapshot has no match in the current SML, the provider replaces the complete address set with an empty array and default discovery fails with NoAvailableAddressesError. This is the same failure mode that required the current refresh and will recur when testnet masternode membership changes again. If the hard whitelist is intentionally retained as a trust boundary, track the required rotation or fallback design; silently leaving this known failure mode untracked makes the fix temporary by construction.
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
The refreshed testnet whitelist and its comments are internally consistent, but discovery still treats the snapshot as a hard allow-list. When the snapshot no longer intersects the current SML, the provider installs an empty address list and default discovery fails with NoAvailableAddressesError; this previously reported suggestion remains valid. The dashcore-lib V24 parser prerequisite is intentionally deferred in the draft and is tracked by the upstream #318/#319 work.
🟡 1 suggestion(s)
1 finding(s) not shown inline (the lines are not part of this PR's diff)
🟡 Suggestion: An expired whitelist snapshot still causes a complete discovery failure
packages/js-dapi-client/lib/dapiAddressProvider/SimplifiedMasternodeListDAPIAddressProvider.js:58-63
The refreshed snapshot restores discovery only while at least one current valid masternode matches it. Once the snapshot becomes stale, this filter produces an empty array, setAddresses([]) removes every candidate, and the subsequent live-address lookup fails even though the SML may contain reachable evonodes. Because this provider is used by default testnet discovery, the same complete outage will recur after the next snapshot rotation. Preserve the whitelist as a preference by falling back to updatedAddresses when the intersection is empty, or track a concrete follow-up for that recovery behavior.
let filteredAddresses = updatedAddresses;
if (this.addressWhiteStrings.length > 0) {
filteredAddresses = updatedAddresses.filter((dapiAddress) => (
this.addressWhiteStrings.includes(dapiAddress.toString())
));
if (filteredAddresses.length === 0) {
// Whitelist snapshot went stale; don't let discovery fail completely.
filteredAddresses = updatedAddresses;
}
}
source: glm-5.3-flash (phase1-reviewer: general); gpt-6.1-sol (phase2-reviewer: general, architecture-layering)
Review provenance
Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
lowbygpt-6.1-sol(effort low) — This is a small, contained JavaScript configuration and address-provider adjustment whose behavior is straightforward to verify, while the described dashcore-lib dependency fix is not part of the diff. - Phase 1 reviewers:
glm-5.3-flash— general (completed, effort high); agentphase1-reviewer - Phase 1 model:
glm-5.3-flash— zai quota: 5h 95% left, weekly 96% left; passed overgemini-3.8-flash-high(antigravity quota lookup failed) - Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort medium); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/js-dapi-client/lib/dapiAddressProvider/SimplifiedMasternodeListDAPIAddressProvider.js`:
- [SUGGESTION] packages/js-dapi-client/lib/dapiAddressProvider/SimplifiedMasternodeListDAPIAddressProvider.js:58-63: An expired whitelist snapshot still causes a complete discovery failure
The refreshed snapshot restores discovery only while at least one current valid masternode matches it. Once the snapshot becomes stale, this filter produces an empty array, `setAddresses([])` removes every candidate, and the subsequent live-address lookup fails even though the SML may contain reachable evonodes. Because this provider is used by default testnet discovery, the same complete outage will recur after the next snapshot rotation. Preserve the whitelist as a preference by falling back to `updatedAddresses` when the intersection is empty, or track a concrete follow-up for that recovery behavior.
Out-of-scope follow-up suggestions (1)
These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.
- Mainnet DAPI seed TLS certificates require infrastructure renewal — The supplied live evidence reports expired certificates for multiple mainnet seed endpoints and an unresponsive fourth seed. This prevents new default mainnet clients from bootstrapping independently of this whitelist and V24 change, so it should be handled by the infrastructure team in a separate issue.
- Follow-up: File an infrastructure issue to renew the mainnet seed certificates and audit seed reachability.
|
Re the 🟡 stale-snapshot fallback suggestion in the re-review: I'm leaving it out of this PR. 🤖 Posted autonomously by Claude on behalf of pasta. |
None of the 22 whitelisted testnet addresses is an enabled evonode any
more, so SimplifiedMasternodeListDAPIAddressProvider filtered out the
whole masternode list and DAPIClient({ network: 'testnet' }) failed with
NoAvailableAddressesError. Regenerate the list with the command in the
comment: the 41 evonodes that are currently enabled and pass the
version check.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The whitelist has been a snapshot of all enabled testnet evonodes rather than a hand-picked set of DCG nodes since #2816; say so, and say what happens when it goes stale. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
5cf1b7d to
eb8708f
Compare
0.22.0 fails on every post-V24 testnet masternode list diff (CbTx v4, ExtAddr entries), so DAPI address discovery can't start. 0.26.1 supports the V24 wire formats; all nine workspace packages move together. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Your move: thepastaclaw left review threads unresolved; resolve them. |
github-actions
Bot
added
the
waiting-self-review
|
Your move: thepastaclaw left review threads unresolved; resolve them. |
|
Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
waiting-bots
Issue being fixed or feature implemented
Since Dash Core V24 activated on testnet (block 1,578,000, 2026-10-23),
new DAPIClient({ network: 'testnet' })cannot reach any node unless it is given explicitdapiAddresses. dash-bridge had to hard-code addresses to work around this (dashpay/dash-bridge#53). Mainnet hits the parser part of this at its own V24 fork.Reproduced live with the published
@dashevo/dapi-client5.0.1, using default discovery andcore.getBestBlockHeight(). There are three failures, each hidden behind the one before:ExtNetInfo. fix: parse Core V24 masternode list diffs dashcore-lib#319 also fixes a regression in 0.25.0, which caps LLMQ_60_75 at 24 active quorums. Mainnet and testnet both carry 32, so bumping to 0.25.0 alone would break mainnet too.NoAvailableAddressesError: No available addresses. None of the 22 addresses innetworkConfigs.testnet.dapiAddressesWhiteListis an enabled testnet evonode any more, soSimplifiedMasternodeListDAPIAddressProviderfilters out the whole list. This rot happened before V24; the parser failure was hiding it.What was done?
networkConfigs.js: regenerated the testnet whitelist with the command in its comment. It now holds 41 evonodes that are enabled and pass the version check, all on Platform HTTP port 1443.@dashevo/dashcore-libfrom~0.22.0to~0.26.1in every workspace package that uses it: dapi, bench-suite, dashmate, js-dapi-client, wallet-lib, dash-spv, js-dash-sdk, platform-test-suite and wasm-dpp. They all stay on one copy, and wallet-lib and the SDK also see post-V24 blocks and coinbases. 0.26.1 adds Core V24 support (fix: support Core V24 coinbase payloads dashcore-lib#318–externalIp option error #323) and one new dependency,@noble/hashes. Platform doesn't use any of the APIs whose behaviour changed (ProTx payload classes, binary masternode list diffs). The SDK's asset lock is a version 1 payload with a standard P2PKH output, which still validates.yarn.lock,.pnp.cjsand.yarn/cacheare updated to match.How Has This Been Tested?
Live testnet, using the published
@dashevo/dapi-client5.0.1 with default discovery andcore.getBestBlockHeight():NoAvailableAddressesError1582348(the tip)mnlistdiffcaptures as regression fixtures.@dashevo/dash-spvtests pass locally with 0.26.1 (63 passing). The other affected packages' tests need a builtwasm-dpp, so CI covers them.Known issues, not fixed here
seed-1/-2/-3.mainnet.networks.dash.org:443expired on Sep 18 2026, Mar 27 2025 and Sep 24 2026.seed-4doesn't answer, andseed-1.pshenmic.devdoesn't resolve, on testnet either. Sonew DAPIClient()on mainnet can't bootstrap today, independent of V24. This needs infrastructure work, not code.getIp()(thecore_p2phost) plusplatformHTTPPort. That's correct for every testnet v3 evonode today, but not for evonodes that publish Platform on a different host or a domain. dashcore-lib 0.26.1 addsSimplifiedMNListEntry.getPlatformHTTPSEndpoint()for that; switching the provider to it is a follow-up.platform.getStatus()throws on testnet because the response omitsstate_sync. It's out of scope here.Breaking Changes
None.
Checklist:
structure.rs, regeneratedgrovedb-structure.json, and checked the structure viewer link posted on this pull requestFor repository code-owners and collaborators only
🤖 Generated with Claude Code
PR Hygiene ·
db88654/skip-botsproceeds without the ones not yet reported/self-reviewed.pnp.cjs,.yarn/cache/@dashevo-dashcore-lib-npm-0.22.0-9a6dd273b9-ac9e268f6e.zip,.yarn/cache/@dashevo-dashcore-lib-npm-0.26.1-a5ae7b63b3-da3a8914ef.zipand 11 more) — QuantumExplorer or shumkovdashmate(packages/dashmate/package.json) — ktechmidas or shumkovjs-wasm-sdk(packages/js-dash-sdk/package.json) — shumkovWhen every merge requirement is met, the
PR Hygienecheck passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.Summary by CodeRabbit