feat: add devnet support#25
Merged
Merged
Conversation
Bridge now supports devnets (e.g. devnet-tadi) alongside testnet/mainnet. Users can pick a preset devnet or enter custom config (DAPI addresses, Insight URL, optional RPC URL) which is persisted to localStorage. Platform connections to devnets use EvoSDK with custom masternode addresses and trusted mode. IS lock retrieval uses a dual-stack approach: JSON-RPC and DAPI subscription race in parallel for mainnet/testnet (first wins), DAPI subscription only for devnets which have no JSON-RPC proxy.
- Collapse 4 duplicated waitForInstantSendLock call sites in main.ts - Move hash160 derivation inside DAPISubscriptionClient - Flatten Promise/IIFE nesting; remove resolved flag - Eagerly construct dapi-client in constructor - Set modal input values via DOM properties (XSS-safe) - Warn on unknown network fallback to testnet
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📝 WalkthroughWalkthroughThis PR introduces custom devnet support and replaces direct JSON-RPC InstantSend lock polling with a dual-backend service that races JSON-RPC and DAPI subscription requests. It systematically widens network type parameters from literal unions to strings, enabling support for arbitrary network names persisted via localStorage, and integrates the new infrastructure through client initialization, bridge operations, and UI components. ChangesDevnet and InstantSend infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/islock.ts`:
- Around line 39-45: The two backend calls (jsonRpcPromise and dapiPromise from
jsonRpcClient.waitForInstantSendLock and
subscriptionClient.waitForInstantSendLock) keep running after Promise.any
resolves; add cancellation so the loser is stopped. Create separate
AbortController (or similar cancel token) per call, pass their signals into
waitForInstantSendLock (or extend those methods to accept a signal), await
Promise.any, then in the then/catch cleanup call abort() on the controller(s)
for any promise that did not win (or call the clients' cancel/unsubscribe
methods if available) so the polling/subscription is terminated immediately.
Ensure waitForInstantSendLock implementations honor the signal or add a cancel
API if needed.
In `@src/config.ts`:
- Around line 93-98: Prevent custom devnets from overriding built-in networks by
rejecting any save or delete that uses a reserved name; in saveCustomDevnet (and
the corresponding custom-devnet remove/update functions around the 106-126 area)
add a guard against reserved names (e.g., "mainnet" and "testnet" or a
RESERVED_NETWORK_NAMES Set) before mutating localStorage or NETWORK_REGISTRY,
and return/throw an error or no-op when the name is reserved; use
loadCustomDevnets, CUSTOM_DEVNETS_KEY and NETWORK_REGISTRY to determine existing
custom entries but never allow replacing entries whose name is in the reserved
set.
- Around line 72-87: Update loadCustomDevnets to more strictly validate
deserialized objects before returning them: in the filter/type-guard for
NetworkConfig (function loadCustomDevnets) add checks that c.minFee and
c.dustThreshold are numbers, that c.dapiAddresses is an array and
c.dapiAddresses.every(a => typeof a === 'string'), and keep the existing checks
(name, type==='devnet', insightApiUrl, addressPrefix, wifPrefix, platformHrp).
Also ensure JSON.parse is wrapped so failures fall through to returning []
(preserve the try/catch behavior) so malformed localStorage entries never get
inserted into the registry.
In `@src/main.ts`:
- Around line 317-320: The devnet toggle handler uses devnetMenu.style.display
=== 'none' which fails when the initial display is an empty string set by CSS;
update the click handler for devnetToggle to check the actual computed
visibility (use getComputedStyle(devnetMenu).display === 'none') or better:
toggle a CSS class (e.g., 'hidden') on devnetMenu via
devnetMenu.classList.toggle('hidden') so the first click reliably opens the
menu; modify the listener attached to devnetToggle accordingly (refer to
devnetToggle.addEventListener and devnetMenu).
- Around line 132-135: switchNetwork currently replaces clients without tearing
down the old IslockService, leaking subscriptions; before calling
initClients(network) retrieve the existing IslockService instance from global
state (or wherever it’s stored), call its shutdown/close/disconnect/unsubscribe
method (the actual API on IslockService) to stop subscriptions, clear that
reference, then proceed to updateState(setNetwork(...)) and call
initClients(network) to create and store the new instance; reference the
switchNetwork function, initClients function, and the IslockService instance
name used in your state to locate where to add the teardown.
In `@src/platform/client.ts`:
- Around line 41-50: The code silently falls back to EvoSDK.testnetTrusted()
when config.type === 'devnet' but config.dapiAddresses is missing/empty; change
this to fail fast by validating the devnet config before returning a client: in
the branch that checks config.type === 'devnet', verify that
config.dapiAddresses is a non-empty array and if not throw an Error (or reject)
with a clear message (e.g., "devnet selected but dapiAddresses missing"),
otherwise construct and return new EvoSDK({...}) as before; update any callers
expecting a client to handle the thrown error.
In `@src/ui/components.ts`:
- Around line 277-281: The devnetOptions map is using escapeHtml(d.name)
(text-safe) for both the button label and the data-network attribute, which can
break the quoted attribute if the devnet name contains a quote; update the
mapping in the devnetOptions block so the button text still uses
escapeHtml(d.name) but the data-network attribute uses an attribute-safe escape
(e.g., an escapeAttr or encodeAttribute function that escapes quotes and other
attribute-special characters) when producing data-network="${...}"; adjust the
same change for the other similar block (lines referenced in the comment) and
keep references to escapeHtml, devnetOptions, state.network and devnets when
making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f44c1aec-321d-4163-9f66-736e778bb8cb
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
index.htmlpackage.jsonsrc/api/dapi-subscription.tssrc/api/dapi.tssrc/api/islock.tssrc/config.tssrc/crypto/hd.tssrc/crypto/keys.tssrc/main.tssrc/platform/client.tssrc/platform/contract.tssrc/platform/dpns.tssrc/platform/identity.tssrc/types.tssrc/types/external.d.tssrc/ui/components.tssrc/ui/state.tstsconfig.jsonvite.config.ts
Comment on lines
+39
to
+45
| // Race JSON-RPC polling against DAPI subscription — first success wins | ||
| const jsonRpcPromise = this.jsonRpcClient.waitForInstantSendLock(txid, timeoutMs, onRetry); | ||
| const dapiPromise = this.subscriptionClient.waitForInstantSendLock(txid, publicKey, utxo, timeoutMs); | ||
|
|
||
| try { | ||
| return await Promise.any([jsonRpcPromise, dapiPromise]); | ||
| } catch (error) { |
There was a problem hiding this comment.
Cancel the losing backend after Promise.any resolves.
The losing request keeps running (polling or subscription stream) until its own timeout. This adds unnecessary network load and leaves transient resources alive longer than needed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/islock.ts` around lines 39 - 45, The two backend calls
(jsonRpcPromise and dapiPromise from jsonRpcClient.waitForInstantSendLock and
subscriptionClient.waitForInstantSendLock) keep running after Promise.any
resolves; add cancellation so the loser is stopped. Create separate
AbortController (or similar cancel token) per call, pass their signals into
waitForInstantSendLock (or extend those methods to accept a signal), await
Promise.any, then in the then/catch cleanup call abort() on the controller(s)
for any promise that did not win (or call the clients' cancel/unsubscribe
methods if available) so the polling/subscription is terminated immediately.
Ensure waitForInstantSendLock implementations honor the signal or add a cancel
API if needed.
Comment on lines
+72
to
+87
| function loadCustomDevnets(): NetworkConfig[] { | ||
| try { | ||
| const stored = localStorage.getItem(CUSTOM_DEVNETS_KEY); | ||
| if (!stored) return []; | ||
| const parsed = JSON.parse(stored); | ||
| if (!Array.isArray(parsed)) return []; | ||
| return parsed.filter( | ||
| (c): c is NetworkConfig => | ||
| c && | ||
| typeof c.name === 'string' && | ||
| c.type === 'devnet' && | ||
| typeof c.insightApiUrl === 'string' && | ||
| typeof c.addressPrefix === 'number' && | ||
| typeof c.wifPrefix === 'number' && | ||
| typeof c.platformHrp === 'string' | ||
| ); |
There was a problem hiding this comment.
Harden custom-devnet deserialization validation before registry load.
loadCustomDevnets() currently accepts records without validating required numeric fields used downstream (minFee, dustThreshold) and without validating dapiAddresses. A malformed localStorage entry can load into the registry and later fail at runtime.
Suggested fix
return parsed.filter(
(c): c is NetworkConfig =>
c &&
typeof c.name === 'string' &&
c.type === 'devnet' &&
typeof c.insightApiUrl === 'string' &&
typeof c.addressPrefix === 'number' &&
typeof c.wifPrefix === 'number' &&
- typeof c.platformHrp === 'string'
+ typeof c.minFee === 'number' &&
+ typeof c.dustThreshold === 'number' &&
+ typeof c.platformHrp === 'string' &&
+ Array.isArray(c.dapiAddresses) &&
+ c.dapiAddresses.every((a: unknown) => typeof a === 'string')
);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/config.ts` around lines 72 - 87, Update loadCustomDevnets to more
strictly validate deserialized objects before returning them: in the
filter/type-guard for NetworkConfig (function loadCustomDevnets) add checks that
c.minFee and c.dustThreshold are numbers, that c.dapiAddresses is an array and
c.dapiAddresses.every(a => typeof a === 'string'), and keep the existing checks
(name, type==='devnet', insightApiUrl, addressPrefix, wifPrefix, platformHrp).
Also ensure JSON.parse is wrapped so failures fall through to returning []
(preserve the try/catch behavior) so malformed localStorage entries never get
inserted into the registry.
Comment on lines
+93
to
+98
| export function saveCustomDevnet(config: NetworkConfig): void { | ||
| const customs = loadCustomDevnets().filter((c) => c.name !== config.name); | ||
| customs.push(config); | ||
| localStorage.setItem(CUSTOM_DEVNETS_KEY, JSON.stringify(customs)); | ||
| NETWORK_REGISTRY.set(config.name, config); | ||
| } |
There was a problem hiding this comment.
Block overriding built-in network names in custom devnet saves.
Line 93 allows saving any name, so a custom entry can replace testnet/mainnet in NETWORK_REGISTRY. That can silently reroute core flows under a trusted devnet config.
Suggested fix
+const RESERVED_NETWORK_NAMES = new Set(['testnet', 'mainnet', 'devnet-tadi']);
+
export function saveCustomDevnet(config: NetworkConfig): void {
+ if (config.type !== 'devnet') {
+ throw new Error('Only devnet configs can be saved as custom networks');
+ }
+ if (RESERVED_NETWORK_NAMES.has(config.name)) {
+ throw new Error(`"${config.name}" is reserved and cannot be overridden`);
+ }
const customs = loadCustomDevnets().filter((c) => c.name !== config.name);
customs.push(config);
localStorage.setItem(CUSTOM_DEVNETS_KEY, JSON.stringify(customs));
NETWORK_REGISTRY.set(config.name, config);
}As per coding guidelines, "Centralize network configuration (testnet/mainnet) in config.ts, with network selection determined by URL parameter ?network=mainnet (defaults to testnet)".
Also applies to: 106-126
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/config.ts` around lines 93 - 98, Prevent custom devnets from overriding
built-in networks by rejecting any save or delete that uses a reserved name; in
saveCustomDevnet (and the corresponding custom-devnet remove/update functions
around the 106-126 area) add a guard against reserved names (e.g., "mainnet" and
"testnet" or a RESERVED_NETWORK_NAMES Set) before mutating localStorage or
NETWORK_REGISTRY, and return/throw an error or no-op when the name is reserved;
use loadCustomDevnets, CUSTOM_DEVNETS_KEY and NETWORK_REGISTRY to determine
existing custom entries but never allow replacing entries whose name is in the
reserved set.
Comment on lines
+132
to
+135
| function switchNetwork(network: string): void { | ||
| updateState(setNetwork(state, network)); | ||
| initClients(network); | ||
| } |
There was a problem hiding this comment.
Disconnect previous IslockService instance during network switch.
Line 134 replaces the client without cleaning up the previous subscription client. Repeated switches can leak open connections until GC/timeouts.
Suggested fix
-function switchNetwork(network: string): void {
+async function switchNetwork(network: string): Promise<void> {
+ await islockService?.disconnect().catch(() => {});
updateState(setNetwork(state, network));
initClients(network);
}- switchNetwork(network);
+ void switchNetwork(network);As per coding guidelines, "Use main.ts as the entry point for UI orchestration and global state management, initializing network configuration and coordinating the bridge workflow".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main.ts` around lines 132 - 135, switchNetwork currently replaces clients
without tearing down the old IslockService, leaking subscriptions; before
calling initClients(network) retrieve the existing IslockService instance from
global state (or wherever it’s stored), call its
shutdown/close/disconnect/unsubscribe method (the actual API on IslockService)
to stop subscriptions, clear that reference, then proceed to
updateState(setNetwork(...)) and call initClients(network) to create and store
the new instance; reference the switchNetwork function, initClients function,
and the IslockService instance name used in your state to locate where to add
the teardown.
Comment on lines
+317
to
+320
| devnetToggle.addEventListener('click', (e) => { | ||
| e.stopPropagation(); | ||
| devnetMenu.style.display = devnetMenu.style.display === 'none' ? 'flex' : 'none'; | ||
| }); |
There was a problem hiding this comment.
Devnet menu toggle can require an extra click on first open.
The toggle checks devnetMenu.style.display === 'none', but initial value is often '' when hidden via CSS class. First click may set 'none' again instead of opening.
Suggested fix
devnetToggle.addEventListener('click', (e) => {
e.stopPropagation();
- devnetMenu.style.display = devnetMenu.style.display === 'none' ? 'flex' : 'none';
+ const isHidden = getComputedStyle(devnetMenu).display === 'none';
+ devnetMenu.style.display = isHidden ? 'flex' : 'none';
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| devnetToggle.addEventListener('click', (e) => { | |
| e.stopPropagation(); | |
| devnetMenu.style.display = devnetMenu.style.display === 'none' ? 'flex' : 'none'; | |
| }); | |
| devnetToggle.addEventListener('click', (e) => { | |
| e.stopPropagation(); | |
| const isHidden = getComputedStyle(devnetMenu).display === 'none'; | |
| devnetMenu.style.display = isHidden ? 'flex' : 'none'; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main.ts` around lines 317 - 320, The devnet toggle handler uses
devnetMenu.style.display === 'none' which fails when the initial display is an
empty string set by CSS; update the click handler for devnetToggle to check the
actual computed visibility (use getComputedStyle(devnetMenu).display === 'none')
or better: toggle a CSS class (e.g., 'hidden') on devnetMenu via
devnetMenu.classList.toggle('hidden') so the first click reliably opens the
menu; modify the listener attached to devnetToggle accordingly (refer to
devnetToggle.addEventListener and devnetMenu).
Comment on lines
+41
to
50
| if (config.type === 'devnet' && config.dapiAddresses?.length) { | ||
| return new EvoSDK({ | ||
| addresses: config.dapiAddresses, | ||
| network: 'testnet', | ||
| trusted: true, | ||
| ...options, | ||
| }); | ||
| } | ||
|
|
||
| return EvoSDK.testnetTrusted(options); |
There was a problem hiding this comment.
Fail fast on invalid devnet config instead of silently using testnet.
When config.type is devnet but dapiAddresses is empty/missing, this currently falls back to EvoSDK.testnetTrusted(). That can route platform operations to the wrong network under a devnet selection.
Suggested fix
function createPlatformSdk(network: PlatformNetwork): EvoSDK {
const options = { settings: PLATFORM_REQUEST_SETTINGS };
const config = getNetwork(network);
if (config.type === 'mainnet') {
return EvoSDK.mainnetTrusted(options);
}
- if (config.type === 'devnet' && config.dapiAddresses?.length) {
- return new EvoSDK({
- addresses: config.dapiAddresses,
- network: 'testnet',
- trusted: true,
- ...options,
- });
+ if (config.type === 'devnet') {
+ if (!config.dapiAddresses?.length) {
+ throw new Error(`Devnet "${network}" is missing DAPI addresses`);
+ }
+ return new EvoSDK({
+ addresses: config.dapiAddresses,
+ network: 'testnet',
+ trusted: true,
+ ...options,
+ });
}
return EvoSDK.testnetTrusted(options);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/platform/client.ts` around lines 41 - 50, The code silently falls back to
EvoSDK.testnetTrusted() when config.type === 'devnet' but config.dapiAddresses
is missing/empty; change this to fail fast by validating the devnet config
before returning a client: in the branch that checks config.type === 'devnet',
verify that config.dapiAddresses is a non-empty array and if not throw an Error
(or reject) with a clear message (e.g., "devnet selected but dapiAddresses
missing"), otherwise construct and return new EvoSDK({...}) as before; update
any callers expecting a client to handle the thrown error.
Comment on lines
+277
to
+281
| const devnetOptions = devnets | ||
| .map((d) => { | ||
| const safe = escapeHtml(d.name); | ||
| return `<button class="devnet-option ${state.network === d.name ? 'active' : ''}" data-network="${safe}">${safe}</button>`; | ||
| }) |
There was a problem hiding this comment.
Escape data-network for attribute context (not text context).
escapeHtml() is text-safe, but this value is also injected into a quoted attribute. A devnet name containing " can break data-network and inject markup/script.
🔧 Proposed fix
+function escapeHtmlAttribute(str: string): string {
+ return str
+ .replace(/&/g, '&')
+ .replace(/"/g, '"')
+ .replace(/'/g, '&`#39`;')
+ .replace(/</g, '<')
+ .replace(/>/g, '>');
+}
+
const devnetOptions = devnets
.map((d) => {
- const safe = escapeHtml(d.name);
- return `<button class="devnet-option ${state.network === d.name ? 'active' : ''}" data-network="${safe}">${safe}</button>`;
+ const safeText = escapeHtml(d.name);
+ const safeAttr = escapeHtmlAttribute(d.name);
+ return `<button class="devnet-option ${state.network === d.name ? 'active' : ''}" data-network="${safeAttr}">${safeText}</button>`;
})
.join('');Also applies to: 283-294
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ui/components.ts` around lines 277 - 281, The devnetOptions map is using
escapeHtml(d.name) (text-safe) for both the button label and the data-network
attribute, which can break the quoted attribute if the devnet name contains a
quote; update the mapping in the devnetOptions block so the button text still
uses escapeHtml(d.name) but the data-network attribute uses an attribute-safe
escape (e.g., an escapeAttr or encodeAttribute function that escapes quotes and
other attribute-special characters) when producing data-network="${...}"; adjust
the same change for the other similar block (lines referenced in the comment)
and keep references to escapeHtml, devnetOptions, state.network and devnets when
making the change.
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.
Summary
Adds devnet support so the bridge can be used on networks other than testnet/mainnet. Users can pick the hardcoded
devnet-tadipreset or enter a custom devnet config (DAPI addresses, Insight URL, optional RPC URL, optional faucet) which is persisted to localStorage. Custom devnets can also be loaded via?network=<name>URL param.Platform connection: For devnets, uses
new EvoSDK({ addresses, network: 'testnet', trusted: true, ... })to point at custom HP masternodes.IS lock retrieval: Dual-stack approach.
digitalcash.dev) against DAPI subscription viaPromise.any— first success wins.subscribeToTransactionsWithProofs, ported from the WIP branchfeat/remove-insight-wip.Dependencies added:
@dashevo/dapi-client,@dashevo/dashcore-lib,vite-plugin-node-polyfills. These are Node-heavy packages that significantly increase bundle size (~22MB). Upstream cleanup of these packages for browser use is being investigated separately; this PR accepts the polyfill cost for now to unblock devnet usage.Trade-offs:
trusted: truefor custom devnets means responses aren't proof-verified — acceptable since users control which devnet they connect to.Test plan
npm run buildpasses (TypeScript + Vite production build)?network=devnet-tadiURL param loads devnet correctly (purple DEVNET-TADI badge)🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes