refactor(bytes): one codec module for base64/hex/byte normalization - #371
Conversation
Thirteen files carried their own base64 decoder, four their own hex decoder, and six a private 'normalize whatever the SDK returned' helper, with subtly different rules (some tried base64 before hex and silently mis-decoded hex key data, one returned an empty array on failure). lib/bytes.ts is now the single source: bytesToBase64/base64ToBytes, bytesToHex/hexToBytes, bytesToBase64Url, bytesEqual, normalizeBytes (lenient, null on failure) and requireBytes (throws with a field label). Strings are tried as hex first, since hex key data is also syntactically valid base64 but never the other way round. Private-feed document transforms now use requireBytes, so a field that cannot be decoded surfaces as a query error instead of an empty array that the decryptor later reports as REVOKED. lib/crypto no longer reaches into lib/services for getPublicKey; it uses crypto/keys directly. DM service drops its Buffer usage and 60-line public-key extractor in favour of the shared codec. The 54-line credit-transfer debug block in tip-service, which re-fetched the identity and logged a WIF prefix on every tip, is removed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
🕓 Ready for review — 32 ahead in queue (commit 2e8765b) |
Second PR of the anti-slop series (after #370). One codec module for the byte conversions that were re-implemented across the tree.
What was there
parseInt(substr, 16)loops)They did not agree. Most tried base64 before hex, which mis-decodes hex identity-key data since a 66-character hex string is also syntactically valid base64. One returned an empty
Uint8Arrayon failure with a warning, which the private-feed decryptor downstream reported as "your access has been revoked".lib/cryptoreached intolib/servicesthrough dynamic imports just to callgetPublicKey.What replaces it
lib/bytes.ts, 140 lines, withlib/bytes.test.ts:bytesToBase64/base64ToBytes,bytesToHex/hexToBytes,bytesToBase64Url,bytesEqual,isHexStringnormalizeBytes(value): acceptsUint8Array,number[], a JSON-serialised NodeBuffer, hex strings, base64 strings; returnsnullon anything else. Hex is tried first for the reason above (real base64 of random bytes is never all hex digits).requireBytes(value, label): same, but throws with the field name.Decoders return
Uint8Array<ArrayBuffer>so results go straight to Web Crypto without a cast.Behaviour changes worth knowing
requireBytes. A field that cannot be decoded now fails the query (caught by the existing per-methodcatch, logged, returnsnull/[]) rather than producing an empty array that surfaces later as a REVOKED message. Every replaced site was checked to sit inside such a catch.tip-service.sendTipis removed. It re-fetched the identity, derived a keypair, and logged the WIF prefix and derived public key on every tip.lib/crypto/key-validation.tsandkey-derivation.tsimportgetPublicKeyfromcrypto/keysinstead of dynamically importing the private-feed crypto service. No import cycle:keys.tsdepends only onhash.ts,wif.tsandbytes.ts.Bufferusage is gone.Wire formats are unchanged. The base64 loops replaced are byte-for-byte the same operation, so existing DMs, key backups and the state-transition retry cache all decode as before.
Review
Independently reviewed against the base: no confirmed bugs. One plausible note (a bad document in a paginated private-feed list now fails the whole page instead of one entry, but
documentToPlainObjectkeeps byte fields asUint8Array, so strings never reach those sites in practice). The reviewer's cosmetic nits (an unused exportedBytesalias, two now-redundantas ArrayBuffercasts) are folded into the next PR, which touches those files anyway.Validation
npm run lint,npx tsc --noEmit,npm run test(60 specs),npx knip --include files,dependencies,unlisted,npm run buildall clean locally.🤖 Generated with Claude Code