refactor(dapi-client)!: expose Uint8Array instead of Buffer in public API#3675
Closed
PastaPastaPasta wants to merge 2 commits into
Closed
refactor(dapi-client)!: expose Uint8Array instead of Buffer in public API#3675PastaPastaPasta wants to merge 2 commits into
PastaPastaPasta wants to merge 2 commits into
Conversation
…ton/fetch/promisify shims
Non-breaking cleanup pass; package stays CJS, no public API changes, no consumer changes required.
dapi-client: replace winston with a minimal console-backed logger that preserves the same API (.error/.warn/.info/.verbose/.debug/.silly/.getForId). Drop node-fetch and the lib/test/bootstrap setimmediate shim — Node 18+ has both globally. Drop the https.Agent self-signed-TLS branch from requestJsonRpc (was Node-only; consumers wanting this must configure NODE_TLS_REJECT_UNAUTHORIZED at the app layer). Inline lodash/sample in ListDAPIAddressProvider. Add engines.node >=18.18. Remove dependencies: winston, node-fetch, lodash, bs58 (unused), node-inspect-extracted (unused). Remove devDeps: setimmediate.
dapi-grpc: inline the promisify shim in core/v0/web/CorePromiseClient.js and platform/v0/web/PlatformPromiseClient.js so the browser bundle no longer requires Node's util module. Both files document the shim so a future codegen regen does not silently reintroduce require('util').
… API
Adds lib/utils/bytes.js helper (hexToBytes/bytesToHex/base64ToBytes/bytesToBase64/concatBytes/bytesEqual) and converts all Buffer.* call sites in dapi-client lib/ to Uint8Array, with corresponding test updates. Package stays CJS.
Production exceptions where Buffer is retained: BlockHeadersReader passes Buffer to dashcore-lib's BlockHeader (its BufferReader needs .readInt32LE), and GetIdentitiesContractKeysResponse passes Buffer to wasm-dpp's Identifier.from (it explicitly requires Node Buffer).
createGrpcTransportError now handles both raw bytes (grpc-js path) and base64 strings (grpc-web path) for drive-error-data-bin, stack-bin, and dash-serialized-consensus-error-bin metadata fields, restoring the dual-format behavior that Buffer.from(x, 'base64') used to provide implicitly.
Test updates: spec files that construct expected protobuf requests now wrap .toBuffer() with new Uint8Array(...) to match production's normalization (sinon calledOnceWithExactly distinguishes Buffer from plain Uint8Array).
Breaking change for direct consumers: response object byte fields are now Uint8Array. Callers that do response.field.toString('hex') will fail — use bytesToHex(response.field) from lib/utils/bytes instead. Buffer.isBuffer(response.field) now returns false; use response.field instanceof Uint8Array.
Test results: dapi-client 315/315, wallet-lib 377/377, js-dash-sdk 60/60 — downstream consumers continue passing without modification (they exercise dapi-client mostly via mocks).
Contributor
|
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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (75)
✨ 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 |
Member
Author
|
Closing — reopening as upstream→upstream so CI runs against secrets/runners. New PR link will be added shortly. |
Member
Author
|
Reopened as #3680 (upstream→upstream so CI runs against repo secrets/runners). |
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
Replaces
BufferwithUint8Arrayacross@dashevo/dapi-client's public API. Adds a smalllib/utils/bytes.jshelper for hex/base64/concat operations. Package stays CJS — that conversion happens in PR 3.This is PR 2 of 5 in the stacked series. Depends on #3674.
Why
Removes
Bufferfrom the dapi-client surface so consumers don't need aBufferpolyfill in browser bundles.Bufferis Node-specific;Uint8Arrayworks everywhere.What changes
dapi-client lib/
lib/utils/bytes.js:hexToBytes,bytesToHex,base64ToBytes,bytesToBase64,concatBytes,bytesEqual(CJS exports).Buffer.*call sites converted per the translation table in the commit message.Bufferis retained:BlockHeadersReader→new BlockHeader(Buffer.from(header))because dashcore-lib'sBufferReaderneeds.readInt32LE.GetIdentitiesContractKeysResponse→Identifier.from(Buffer.from(entry.getIdentityId()))because wasm-dpp'sIdentifierexplicitly requires NodeBuffer.createGrpcTransportError
Restored dual-format handling that
Buffer.from(x, 'base64')provided implicitly:typeof x === 'string' ? base64ToBytes(x) : new Uint8Array(x)for the three metadata-bin fields (grpc-js sends raw bytes; grpc-web sends base64 strings).Tests
Spec files that construct expected protobuf requests now wrap
.toBuffer()withnew Uint8Array(...)to match production's normalization. Sinon'scalledOnceWithExactlyis strict about Buffer-vs-Uint8Array distinction.Test plan
yarn workspace @dashevo/dapi-client run test:unit— 315 passingyarn workspace @dashevo/wallet-lib run test:unit— 377 passing (no regression)yarn workspace dash run test:unit— 60 passing (no regression)Downstream consumers (wallet-lib, js-dash-sdk) exercise dapi-client mostly through mocks, so they didn't need updates here.
Breaking changes
Public response objects now expose
Uint8Arrayinstead ofBuffer. Consumer code patterns that need updating:response.field.toString('hex')bytesToHex(response.field)response.field.toString('base64')bytesToBase64(response.field)Buffer.isBuffer(response.field)response.field instanceof Uint8Arrayresponse.field.slice(a, b)response.field.subarray(a, b)In Node,
Buffer extends Uint8Array, so anything that just reads bytes (passes tocrypto, writes to a stream, length checks, indexed access) continues to work identically.Stack