Repository navigation
fix(sdk-coin-ada): stop using cbor's sync encode statics for CIP8 structures - #10089
Conversation
…uctures `cbor`'s synchronous statics (`Encoder.encode`/`encodeOne`/`encodeCanonical`) only return the first chunk read from the encoder stream, so on Node.js 26 the encoded bytes are truncated to one byte and any subsequent decode fails with "unexpected end of input". That broke the CIP8 utils unit tests and the COSE encode/decode helpers as soon as Node.js 26 became a required CI matrix entry (WCN-3753). Encode by collecting the encoder's `data` events instead: that puts the stream in flowing mode, so chunks are released synchronously as the encoder writes. Draining buffered output with `read()` does not work — past the readable high-water mark `push()` returns false and the encoder's chunked `push()` chains short-circuit, truncating large values; the regression test added here (a 64 KB CIP8 payload, i.e. encoding above the encoder chunk size) fails under that implementation on Node 22.23.2, 24.15.0 and 26.12.1. Verified locally: @bitgo/sdk-coin-ada unit tests 195 passing (exit 0) on Node 22.23.2, 24.15.0 and Node 26.12.1, plus a COSE round-trip smoke test for messages up to 1 MB on all three versions. Ticket: CECHO-2580
|
|
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
@claude please review. |
|
Claude finished @nayandas190's task in 27s —— View job Review of #10089 (already merged)Verdict: looks good. I have one thing to confirm and a few minor notes. The fix is sound. To confirm
Minor notes
I only read the code and haven't run the tests, so the results in the PR description are unverified by me. |
Description
@bitgo/sdk-coin-adabuilds its CIP8 / COSE payloads withcbor's synchronous statics (Encoder.encode/encodeOne/encodeCanonical) inmodules/sdk-coin-ada/src/lib/messages/cip8/utils.ts. Internally those funnel throughEncoder._encodeAll, which returns a singleread()chunk; on Node.js 26 the encoder emits its output in 1-byte chunks, so the encoding was silently truncated to one byte and any decode failed withunexpected end of input.encodeCborhelper that collects the encoder'sdataevents. Attachingdataputs the Transform in flowing mode, so chunks are released synchronously while the encoder writes.encodeCbordocumenting why draining buffered output withread()is not an option (past the high-water markpush()returns false and cbor's chunkedpush()chains short-circuit, truncating large values).Minimal repro of the underlying bug:
Split out of #10088 (statics bot-token change) so it can land independently: Node.js 26 is a required CI matrix entry since #10043 (WCN-3753), and
unit-test-changedruns changed packages plus their dependents —@bitgo/sdk-coin-adadepends on@bitgo/statics, which is why the 26.x job in #10088 surfaced this.Issue Number
CECHO-2590
Type of change
How Has This Been Tested?
yarn --cwd modules/sdk-coin-ada unit-teston Node 22.23.2, 24.15.0 and 26.12.1 — 195 passing, exit 0 (was 192 passing / 2 failing on Node 26 before the fix)pushAny+ drainread()alternative, which is why that approach was not usedChecklist: