-
Notifications
You must be signed in to change notification settings - Fork 5
feat(pass-style): narrow byteArray to plain frozen Uint8Array #475
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: llm-c6b70e8
Are you sure you want to change the base?
Changes from all commits
3d0062b
deff99f
164e6bc
100006c
ab785dd
573d547
644b680
ef1049c
ea03b53
50700d2
8e3bd31
fbcab22
554c5e4
739cbc2
b28bb1f
093456a
3995de4
d13469b
2f71e02
015821d
66b2dc3
a658421
0a6a0f9
90e2bda
a9ee41e
3f99db5
8c87a8f
cd31fb5
9117001
fea8312
004f4bd
f7f7a27
2419e75
fc2238d
0984dd8
bf40a3b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| --- | ||
| '@endo/base64': patch | ||
| --- | ||
|
|
||
| `@endo/base64` now encodes a frozen `Uint8Array` byteArray passable (issue | ||
| #573) correctly, bringing it to parity with its `@endo/hex` twin. | ||
|
|
||
| `jsEncodeBase64` and `encodeBase64` accept a `Uint8Array` (the narrowed | ||
| byteArray shape) and gate on `ArrayBuffer.isView`, the committed | ||
| genuine-vs-emulated distinguisher: a genuine view (mutable or immutable | ||
| buffer) is read in place, while an emulated `@endo/immutable-arraybuffer` | ||
| wrapper — a plain object reporting `isView === false`, whose `bytes[i]` reads | ||
| `undefined` — is thawed into a mutable `Uint8Array` first. | ||
| `encodeBase64` also dispatches to the native `Uint8Array.prototype.toBase64` | ||
| intrinsic (or the legacy `globalThis.Base64.encode` XS binding) only for | ||
| genuine views, whose bytes the native code can read; an emulated wrapper falls | ||
| through to the pure-JavaScript polyfill. Previously the polyfill's | ||
| integer-indexed read silently produced all-zero output for an emulated | ||
| byteArray, and the native path had no such guard. Not reached by an in-repo | ||
| passable today, but the byteArray narrowing that reached `@endo/hex` did not | ||
| reach its twin. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| --- | ||
| '@endo/marshal': minor | ||
| --- | ||
|
|
||
| A `byteArray` (a plain frozen `Uint8Array` backed by an immutable | ||
| `ArrayBuffer`) is now serializable through the capdata, smallcaps, | ||
| encode-passable, and marshal-justin codecs. | ||
|
|
||
| - **capdata**: byteArray encodes as `{"@qclass":"byteArray","data":"<hex>"}`. | ||
| - **smallcaps**: byteArray encodes as `"*<hex>"`. The reserved `*` prefix is | ||
| now assigned to byteArray. | ||
| - **encode-passable**: byteArray encodes as | ||
| `a<encodeBigInt(byteLength)>:<hex>`. The Elias-delta length prefix gives | ||
| shortlex ordering (matching `compareRank`) with no arbitrary size cap, and | ||
| every character in the body is safe inside both `legacyOrdered` and | ||
| `compactOrdered` array framings. | ||
| - **marshal-justin**: renders byteArray as | ||
| `frozenBytes(decodeHex("<hex>"))`. | ||
|
|
||
| Hex conversion uses `@endo/hex` (`encodeHex` / `decodeHex`), and the decoded | ||
| `Uint8Array` is converted into a passable byteArray with | ||
| `@endo/immutable-arraybuffer`'s `frozenBytes`. | ||
|
|
||
| Syrup already supported this value; no change required there. | ||
|
|
||
| Deploy sequencing: producers should not emit byteArrays until decoders are | ||
| upgraded. Older decoders reject the new encodings (unknown `@qclass`, | ||
| unknown smallcaps prefix, unknown encode-passable prefix); consumers must | ||
| ship the new decoder before producers begin emitting byteArray values. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kriscendobot @kriscendobot , none of these protocol version questions need to be resolved for this PR to make progress. Please move this question to a separate issue on endo-but-for-bots. Attn @kriskowal @gibson042 @mhofman
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved these protocol-versioning questions to a separate issue as requested: #584. It captures the concrete case (the new |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| --- | ||
| '@endo/immutable-arraybuffer': minor | ||
| '@endo/bytes': major | ||
| '@endo/marshal': major | ||
| '@endo/ocapn': patch | ||
| '@endo/thixotrope': patch | ||
| --- | ||
|
|
||
| Consolidate the immutable byte utilities onto a single shared implementation | ||
| exported from `@endo/immutable-arraybuffer`, and rename them to `frozenBytes` | ||
| (previously `@endo/bytes`' `bytesToImmutable`) and `thawedBytes` (previously | ||
| `bytesFromImmutable`). `frozenBytes` wraps a `Uint8Array` view's contents in a | ||
| hardened frozen `Uint8Array` backed by an immutable `ArrayBuffer` (a | ||
| `'byteArray'` passable); `thawedBytes` copies such a value back out into a fresh | ||
| mutable `Uint8Array`. Importing the package's new main entry installs the shim | ||
| as a side effect, since `frozenBytes` depends on it; the bare install remains | ||
| the separate `@endo/immutable-arraybuffer/shim.js` export. | ||
|
|
||
| Breaking (no backward compatibility is preserved): | ||
|
|
||
| - `@endo/bytes` no longer exports `./to-immutable.js` (`bytesToImmutable`) or | ||
| `./from-immutable.js` (`bytesFromImmutable`). Import `frozenBytes` and | ||
| `thawedBytes` from `@endo/immutable-arraybuffer` instead. `@endo/bytes` keeps | ||
| `./concat-immutables.js` (`concatImmutables`), now implemented on the shared | ||
| utilities. | ||
| - `@endo/marshal`'s `decodeToJustin` now emits `frozenBytes(decodeHex(...))` | ||
| instead of `bytesToImmutable(decodeHex(...))` for byteArray values, so a | ||
| Justin evaluation environment must bind `frozenBytes` rather than | ||
| `bytesToImmutable`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| --- | ||
| '@endo/immutable-arraybuffer': minor | ||
| 'ses': patch | ||
| --- | ||
|
|
||
| Add freezable TypedArray emulation for immutable-ArrayBuffer-backed views. | ||
|
|
||
| After loading `@endo/immutable-arraybuffer/shim.js`, constructing a TypedArray | ||
| from an emulated immutable `ArrayBuffer` produces an emulated freezable wrapper | ||
| whose mutator methods (`copyWithin`, `fill`, `reverse`, `set`, `sort`) throw | ||
| `TypeError`, whose `buffer` getter returns the immutable wrapper rather than | ||
| the underlying genuine buffer, and which can be frozen via `Object.freeze`. | ||
| The wrapper inherits directly from `T.prototype` with no intermediate prototype. | ||
|
|
||
| The genuine-buffer constructor path (passing a regular mutable `ArrayBuffer`) | ||
| is unchanged: the result is a normal writable TypedArray view. | ||
|
|
||
| `ses`: the permits walk accepts the shim-installed `%TypedArrayPrototype%` | ||
| slots without complaint; no new permit rows are required. | ||
|
|
||
| The shim commits to a single emulated-vs-genuine fidelity loss, | ||
| `ArrayBuffer.isView`: an emulated freezable wrapper is a plain ordinary object | ||
| and reports `ArrayBuffer.isView === false`, whereas a genuine view (mutable, or | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorporated. The changeset now reads (head model |
||
| native-immutable) reports `true`. This is the one distinguisher downstream | ||
| clients (`@endo/bytes` / `@endo/pass-style`) are entitled to rely on, and it is | ||
| pinned by a regression test so an accidental change trips a test rather than | ||
| corrupting a consumer. | ||
|
|
||
| The shim also repairs the emulated wrapper's `[Symbol.toStringTag]`: it replaces | ||
| the genuine `this`-sensitive `%TypedArrayPrototype%[Symbol.toStringTag]` getter | ||
| with a wrapper that amplifies an emulated wrapper to its hidden genuine | ||
| TypedArray, so `Object.prototype.toString.call(emulatedView)` now reads | ||
| `'[object Uint8Array]'` — matching a genuine view — instead of | ||
| `'[object Object]'`. This is a getter-wrapper fix, not a `[Symbol.toStringTag]` | ||
| data property (the wrapper still carries no own tag), so the getter and | ||
| `Object.prototype.toString` agree. `[Symbol.toStringTag]` is therefore no longer | ||
| an emulated-vs-genuine distinguisher; `ArrayBuffer.isView` remains the single | ||
| committed distinguisher. A brand check that captures this getter after the shim | ||
| installs (e.g. `@endo/harden`'s `isTypedArray`) will classify an emulated wrapper | ||
| as a TypedArray and route it through the `freezeTypedArray` path, which is benign | ||
| (the wrapper has no own integer-indexed properties, so it freezes without | ||
| throwing in either capture order). | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refresh this.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in b2e7121 (after stack cleanup). The changeset now describes the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should mention the new ascii package.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed. The changeset now includes a bullet for |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| --- | ||
| '@endo/pass-style': major | ||
| '@endo/bytes': major | ||
| '@endo/patterns': patch | ||
| '@endo/marshal': patch | ||
| '@endo/ocapn': patch | ||
| '@endo/ocapn-noise': patch | ||
| --- | ||
|
|
||
| Narrow the `byteArray` pass style to plain frozen `Uint8Array` only. | ||
|
|
||
| The `byteArray` pass-style brand check previously accepted both raw | ||
| immutable `ArrayBuffer` values and plain frozen `Uint8Array` values | ||
| backed by an immutable `ArrayBuffer`. It now accepts only the latter | ||
| shape: a plain frozen `Uint8Array` whose backing buffer is a plain | ||
| frozen immutable `ArrayBuffer`. Raw immutable `ArrayBuffer` values | ||
| are no longer recognised as `byteArray`; the `ByteArray` TypeScript | ||
| alias is now `Uint8Array` (was `ArrayBuffer`). | ||
|
|
||
| The emulated-vs-genuine distinction the narrowed brand check draws — an | ||
| emulated `@endo/immutable-arraybuffer` wrapper versus a genuine | ||
| integer-indexed `Uint8Array` view — is committed to a single fidelity | ||
| loss, `ArrayBuffer.isView`: an emulated wrapper is a plain object and is | ||
| not a view, a genuine view (mutable or native-immutable) is. `@endo/pass-style`'s | ||
| `byteArray` brand check discriminates on `ArrayBuffer.isView` (a non-view | ||
| must carry zero own indexed properties, a genuine view exactly | ||
| `length`-many matching the buffer), which is strictly more precise than | ||
| accepting either count unconditionally. `@endo/bytes`'s `compareBytes` | ||
| likewise gates on `ArrayBuffer.isView`, indexing a genuine view in place | ||
| and copying only a non-view (emulated) wrapper or bare buffer. The | ||
| integer-indexed-read (`view[i] === undefined`) and `[Symbol.toStringTag]` | ||
| (`'[object Object]'`) behaviors of an emulated wrapper are incidental | ||
| consequences of its plain-object shape, not separately committed fidelity | ||
| losses. | ||
|
|
||
| `@endo/bytes`: the immutable-byte adapters — consolidated into | ||
| `frozenBytes` and `thawedBytes` and re-homed in | ||
| `@endo/immutable-arraybuffer` (see the consolidation changeset) — take | ||
| on the narrowed shape. `frozenBytes(view)` now wraps the immutable | ||
| `ArrayBuffer` produced by `sliceToImmutable` in a fresh frozen | ||
| `Uint8Array` before hardening; the return type is now `Uint8Array` | ||
| (was `ArrayBuffer`). `thawedBytes` accepts the new shape | ||
| (`ArrayBufferView`) in addition to the prior `ArrayBufferLike`. | ||
| `concatImmutables` returns a `Uint8Array` rather than an | ||
| `ArrayBuffer`, and accepts either shape on input. `bytesEqual` now | ||
| gates on `ArrayBuffer.isView` like its `compareBytes` sibling: it | ||
| compares a genuine view in place and thaws a non-view (emulated) wrapper | ||
| or bare buffer into a mutable `Uint8Array` first. Previously it indexed | ||
| its arguments directly, so two distinct equal-length emulated byteArrays | ||
| read `undefined` at every position and compared equal, while an | ||
| emulated-vs-genuine pair compared unequal. | ||
|
|
||
| `@endo/marshal`: the byteArray rank-compare's `ArrayBuffer.prototype` | ||
| dispatch arm becomes dead code and is removed. Values arrive as a frozen | ||
| `Uint8Array` backed by an immutable `ArrayBuffer`. On the emulated | ||
| `@endo/immutable-arraybuffer` path such a wrapper has no integer-indexed | ||
| own properties, so the bytes are read by first copying each wrapper into | ||
| a genuine mutable `Uint8Array` (via `slice`, which the shim amplifies) | ||
| and then delegating the equal-length lexicographic comparison to | ||
| `@endo/bytes`'s `compareBytes`, deduplicating the byte-comparison loop. | ||
|
|
||
| `@endo/patterns`: the `byteArray` matcher's `TypeFromPattern` and | ||
| `getMatcherKind` types resolve to `Uint8Array` (was `ArrayBuffer`). | ||
|
|
||
| `@endo/ocapn`: the syrup `writeBytestring` types (and the crypto, | ||
| codec, client, cbor, and bytewise-compare byte params throughout the | ||
| package) narrow to `Uint8Array`; no function is typed to accept both a | ||
| buffer and a buffer view. Where a codec dispatcher still tolerates a raw | ||
| `ArrayBuffer` from an older peer, that buffer is normalized to a | ||
| `Uint8Array` at the boundary rather than propagated into the callee's | ||
| signature. The hub's `hexFromBytes`/`swissnumHex` helpers and | ||
| `attachSession`'s `powers.identity` handshake fields | ||
| (`sessionId`/`peerPublicKeyQ`/`selfPrivateKeyBytes`) narrow to `Uint8Array` | ||
| the same way: `hexFromBytes` gates on `ArrayBuffer.isView` (like | ||
| `@endo/bytes`' `toIndexableUint8`), reading a genuine view in place and | ||
| copying only an emulated `@endo/immutable-arraybuffer` wrapper — the shape a | ||
| `frozenBytes`/`makeSessionId` session id takes — so no handshake or | ||
| gift-handoff signature is typed to accept both a buffer and a buffer view. | ||
| The byteArray-shaped branded | ||
| client types (`SessionId`, `SwissNum`, `PublicKeyId`) change from | ||
| `ArrayBufferLike & {_brand}` to `Uint8Array & {_brand}`. Printable | ||
| swissnum strings are encoded with canonical `@endo/ascii` before immutable | ||
| wrapping. Decoder paths keep non-ASCII swissnums as bytes rather than | ||
| coercing them through the WHATWG `ascii` decoder. The CBOR | ||
| diagnostic-notation `equals`/`diagnosticEquals` helper's byte comparison | ||
| now gates on `ArrayBuffer.isView` as well, thawing an emulated wrapper | ||
| before indexing; previously (like the pre-fix `asUint8`) it trusted | ||
| `instanceof Uint8Array` and read `undefined` from an emulated wrapper, so | ||
| distinct equal-length byteArrays compared equal (latent — diagnostic | ||
| notation has no wire consumers). | ||
|
|
||
| `@endo/ocapn-noise`: adapt to the narrowed `byteArray`. Its `asUint8` | ||
| helper previously trusted `instanceof Uint8Array` and returned the value | ||
| as-is, which broke the peer-key comparison in the crossed-hellos | ||
| handshake on the emulated `@endo/immutable-arraybuffer` path: the | ||
| decoded public key arrives as a frozen `Uint8Array` wrapper with no | ||
| integer-indexed own properties, so `peerBytes[i]` read `undefined` and | ||
| every byte compared unequal. It now discriminates on `ArrayBuffer.isView` | ||
| (as `@endo/immutable-arraybuffer`'s `thawedBytes` does), copying an emulated | ||
| wrapper into a genuine mutable `Uint8Array`. The stale | ||
| `OcapnNoiseSession.sessionId` type is updated from `ArrayBufferLike` to | ||
| the now-`Uint8Array`-shaped `SessionId`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| --- | ||
| '@endo/pass-style': patch | ||
| --- | ||
|
|
||
| `passStyleOf` no longer blames mutability when a non-`Uint8Array` typed array | ||
| is rejected. The `byteArray` pass style accepts only a whole-buffer | ||
| `Uint8Array` over an immutable `ArrayBuffer`; a typed array of any other | ||
| element type is rejected for its element type, not its mutability. The | ||
| late fall-through guard previously reported every unclaimed genuine | ||
| `TypedArray` with the "Cannot pass mutable typed arrays" message, which | ||
| misleads for a genuinely frozen non-`Uint8Array` typed array over an | ||
| immutable buffer (reachable on a native Immutable-ArrayBuffer engine, and on | ||
| the shim leg under unsafe harden taming) — mutability is not the problem | ||
| there. That case now reports "Cannot pass typed arrays other than Uint8Array". | ||
| A `Uint8Array` still reports the mutable message, since it only reaches that | ||
| guard backed by a mutable buffer (an immutable-backed one is always accepted). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| --- | ||
| '@endo/bytes': patch | ||
| --- | ||
|
|
||
| Deduplicate the byte-order/equality/concat readers' identical | ||
| `toIndexableUint8` helper onto one shared `./src/to-indexable-uint8.js` | ||
| module. `compareBytes`, `bytesEqual`, and `concatBytes` previously each carried | ||
| a byte-for-byte identical copy of the helper that reads a byteArray in place | ||
| when it is a genuine `ArrayBuffer.isView` and copies the emulated | ||
| `@endo/immutable-arraybuffer` wrapper into a fresh mutable `Uint8Array` | ||
| otherwise. No behavior changes. `to-string.js`' `toDecodable` is intentionally | ||
| left separate: it keys on `.immutable` (not `isView`) because | ||
| `TextDecoder.decode` rejects every immutable-backed view, so it must copy even | ||
| a genuine immutable view that the indexed readers read in place. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kriscendobot , I am glad we decided on shortlex for rankCompare, and therefore also for encode-passable. However, @endo/bytes, perhaps in some outstanding PR, defines and exports a compare over genuine Uint8Arrays that is a lexical compare. That's not necessarily a problem. But it does raise the cognitive overhead of the system as a whole. If there's no reason to prefer lexicographic over shortlex for that compare, can we switch it to shortlex?
Attn @kriskowal @gibson042
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @erights — shortlex stays for
rankCompare/encode-passable; agreed.On
@endo/bytes'compareBytes(it is new in this PR —packages/bytes/src/compare.js— so this is the outstanding PR you had in mind): there is a reason it is bytewise-lexicographic rather than shortlex, and it isn't the marshal rank path. It is the OCapN/Syrup wire layer, where "bytewise" / "octet" ordering is spec-mandated, andcompareBytesis that primitive. Three consumers require it:compareBytes(ocapn/src/syrup/js-representation.js).ocapn/src/codecs/subtypes.js).makeSessionIdsorts the two peer IDs "based on their octets" to agree with reference implementations (ocapn/src/cryptography.js).All three are interop points with other OCapN implementations; on unequal-length inputs shortlex and lexicographic disagree, so switching
compareBytesto shortlex would diverge from the wire spec there.The marshal
byteArrayrank path is unaffected either way:rankOrder.jsdoes its own shortlex length pre-check and only delegates the equal-length tie-break tocompareBytes, where lexicographic and shortlex coincide (it says as much in the comment there). Soencode-passable/compareRankstay shortlex regardless of whatcompareBytesdoes.So the two orderings genuinely live at different layers — bytewise for the OCapN wire format, shortlex for marshal rank — which is the cognitive overhead you're pointing at. Rather than change
compareBytes' semantics (which breaks Syrup/OCapN interop), two ways to lower that overhead:@endo/bytesinto the OCapN/Syrup layer that needs it, so@endo/bytesdoesn't export a lexicographic compare at all.I'll defer to you, @kriskowal, and @gibson042 on which — happy to land either in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kriscendobot
That makes sense to me, thanks.