Conversation
Migrates InstantSend lock retrieval from custom RPC endpoints (trpc.digitalcash.dev / rpc.digitalcash.dev) to use the standard DAPI gRPC subscription method subscribeToTransactionsWithProofs. Changes: - Add @dashevo/dapi-client and @dashevo/dashcore-lib dependencies - Rewrite DAPIClient to use bloom filter subscription approach - Create bloom filter for txid and subscribe to transaction stream - Parse InstantSend lock messages and match by txid - Add TypeScript declarations for external CommonJS modules - Update vite.config.ts for CommonJS module handling This removes dependency on third-party RPC endpoints and uses the official DAPI infrastructure for InstantSend lock retrieval.
📝 WalkthroughWalkthroughThis pull request integrates a new subscription-based DAPI client library, replacing the previous RPC polling approach. It adds two dependencies, implements a bloom-filter-based transaction lock detection system via streaming subscriptions, includes comprehensive TypeScript type definitions, and updates Vite configuration to handle CommonJS/ESM module interoperability. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant BDAPI as BridgeDAPIClient
participant DAPIService as DAPI Service
participant Blockchain as Blockchain Network
App->>BDAPI: waitForInstantSendLock(txid, timeout)
BDAPI->>BDAPI: createBloomFilterForTxid(txid)
BDAPI->>BDAPI: getBestBlockHeight()
BDAPI->>DAPIService: subscribeToTransactionsWithProofs(bloomFilter, {fromBlockHeight})
DAPIService->>Blockchain: Open streaming subscription
loop Monitor incoming transactions
Blockchain-->>DAPIService: Transaction with proof
DAPIService-->>BDAPI: Stream data event
BDAPI->>BDAPI: parseInstantLockTxid(islockBytes)
alt Matching txid found
BDAPI->>App: Return Uint8Array (Promise resolves)
else Continue monitoring
BDAPI->>BDAPI: Wait for next data event
end
end
alt Timeout exceeded
BDAPI->>App: Reject Promise
end
App->>BDAPI: disconnect()
BDAPI->>DAPIService: Close connection
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 2
🤖 Fix all issues with AI agents
In `@src/api/dapi.ts`:
- Around line 143-146: The variable `stream` is typed as ReturnType<typeof
client.core.subscribeToTransactionsWithProofs> but that captures the Promise
type; update the annotation so `stream` reflects the resolved StreamResponse —
e.g., change its type to Awaited<ReturnType<typeof
client.core.subscribeToTransactionsWithProofs>> (or, if you prefer, use `any`
since `client` is `any`) and ensure the code uses the resolved `stream` after
awaiting client.core.subscribeToTransactionsWithProofs; the fix targets the
`stream` declaration in the Promise block where
`subscribeToTransactionsWithProofs` is awaited.
In `@vite.config.ts`:
- Around line 20-23: The define block currently replaces all process.env
references with an empty object which breaks usage of process.env.BASE_PATH;
update vite.config.ts to stop globally polyfilling 'process.env' and instead
either (a) define only the needed variable ('process.env.BASE_PATH') so the
build injects the real BASE_PATH value, or (b) change your code to read the
value via import.meta.env.BASE_PATH and remove the broad 'process.env' define;
locate the define object in vite.config.ts to implement the targeted change and
ensure BASE_PATH is preserved at build time.
🧹 Nitpick comments (3)
src/types/external.d.ts (1)
27-32: Consider returningthisfor method chaining.The
on()method typically returnsthis(or the stream itself) to allow method chaining, which is a common pattern in Node.js streams.Suggested improvement
interface StreamResponse { - on(event: 'data', callback: (data: unknown) => void): void; - on(event: 'error', callback: (error: Error) => void): void; - on(event: 'end', callback: () => void): void; + on(event: 'data', callback: (data: unknown) => void): this; + on(event: 'error', callback: (error: Error) => void): this; + on(event: 'end', callback: () => void): this; cancel(): void; }src/api/dapi.ts (2)
13-29: Type duplication is acceptable but could diverge.These local interface definitions duplicate types from
src/types/external.d.ts. While this is a reasonable approach for type safety with dynamically loaded CommonJS modules, be aware that changes inexternal.d.tswon't automatically sync here.Consider adding a comment linking to the ambient declarations or consolidating types.
218-224: Consider tracking stream errors for better diagnostics.Currently, stream errors are logged but otherwise ignored, relying on the timeout to handle failures. This could make debugging difficult if transient errors occur repeatedly.
Optional improvement
+ let lastError: Error | null = null; + stream.on('error', (error: Error) => { if (!resolved) { console.error('Stream error:', error); + lastError = error; + onProgress?.(`Stream error: ${error.message}`); // Don't reject immediately on stream errors, the timeout will handle it // Some stream errors are recoverable } });Then include
lastErrorin the timeout rejection message for context.
| return new Promise<Uint8Array>((resolve, reject) => { | ||
| let stream: ReturnType<typeof client.core.subscribeToTransactionsWithProofs> | null = null; | ||
| let timeoutId: ReturnType<typeof setTimeout> | null = null; | ||
| let resolved = false; |
There was a problem hiding this comment.
Incorrect type annotation for stream variable.
The subscribeToTransactionsWithProofs method returns Promise<StreamResponse>, but after awaiting it on line 175, stream holds the resolved StreamResponse, not the Promise. The current ReturnType<...> captures the Promise type incorrectly.
Proposed fix
return new Promise<Uint8Array>((resolve, reject) => {
- let stream: ReturnType<typeof client.core.subscribeToTransactionsWithProofs> | null = null;
+ let stream: { on: Function; cancel: () => void } | null = null;
let timeoutId: ReturnType<typeof setTimeout> | null = null;
let resolved = false;Or simply use any since the client is already typed as any.
🤖 Prompt for AI Agents
In `@src/api/dapi.ts` around lines 143 - 146, The variable `stream` is typed as
ReturnType<typeof client.core.subscribeToTransactionsWithProofs> but that
captures the Promise type; update the annotation so `stream` reflects the
resolved StreamResponse — e.g., change its type to Awaited<ReturnType<typeof
client.core.subscribeToTransactionsWithProofs>> (or, if you prefer, use `any`
since `client` is `any`) and ensure the code uses the resolved `stream` after
awaiting client.core.subscribeToTransactionsWithProofs; the fix targets the
`stream` declaration in the Promise block where
`subscribeToTransactionsWithProofs` is awaited.
| define: { | ||
| // Polyfill for process.env in browser | ||
| 'process.env': {}, | ||
| }, |
There was a problem hiding this comment.
Potential issue: process.env polyfill may break BASE_PATH usage.
The define: { 'process.env': {} } on line 20-22 replaces all process.env references with an empty object at build time. However, line 4 uses process.env.BASE_PATH, which will now always resolve to undefined since the polyfill doesn't preserve environment variables.
Consider using a more targeted approach:
Proposed fix
define: {
- // Polyfill for process.env in browser
- 'process.env': {},
+ // Polyfill for process.env in browser while preserving BASE_PATH
+ 'process.env': {
+ BASE_PATH: JSON.stringify(process.env.BASE_PATH || '/'),
+ },
},Alternatively, if BASE_PATH is set at build time via Vite's env handling, you could use import.meta.env.BASE_PATH instead.
📝 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.
| define: { | |
| // Polyfill for process.env in browser | |
| 'process.env': {}, | |
| }, | |
| define: { | |
| // Polyfill for process.env in browser while preserving BASE_PATH | |
| 'process.env': { | |
| BASE_PATH: JSON.stringify(process.env.BASE_PATH || '/'), | |
| }, | |
| }, |
🤖 Prompt for AI Agents
In `@vite.config.ts` around lines 20 - 23, The define block currently replaces all
process.env references with an empty object which breaks usage of
process.env.BASE_PATH; update vite.config.ts to stop globally polyfilling
'process.env' and instead either (a) define only the needed variable
('process.env.BASE_PATH') so the build injects the real BASE_PATH value, or (b)
change your code to read the value via import.meta.env.BASE_PATH and remove the
broad 'process.env' define; locate the define object in vite.config.ts to
implement the targeted change and ensure BASE_PATH is preserved at build time.
2 participants
Migrates InstantSend lock retrieval from custom RPC endpoints (trpc.digitalcash.dev / rpc.digitalcash.dev) to use the standard DAPI gRPC subscription method subscribeToTransactionsWithProofs.
Changes:
This removes dependency on third-party RPC endpoints and uses the official DAPI infrastructure for InstantSend lock retrieval.
Summary by CodeRabbit
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.