-
Notifications
You must be signed in to change notification settings - Fork 79
feat(ocapn): CBOR alternative encoding #3033
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: master
Are you sure you want to change the base?
Conversation
1c5f113 to
4d057a5
Compare
4d057a5 to
cd93ea0
Compare
gibson042
left a comment
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.
These comments are limited to documentation; I haven't looked at any implementing code.
packages/ocapn/docs/cbor-encoding.md
Outdated
| | Target (in-band) | Record marker | `27([280("target")])` | | ||
| | Promise (in-band) | Record marker | `27([280("promise")])` | | ||
| | Error (in-band) | Record with message | `27([280("error"), "TypeError"])` | |
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.
CBOR tag number 27 suggests (if not requires) that the first element be a string ("The typename [in array [typename, constructargs...]] is usually a class name, or another string that indicates the name of the type"), and I think we should honor that expectation.
| | Target (in-band) | Record marker | `27([280("target")])` | | |
| | Promise (in-band) | Record marker | `27([280("promise")])` | | |
| | Error (in-band) | Record with message | `27([280("error"), "TypeError"])` | | |
| | Target (in-band) | Record marker | `27(["target"])` | | |
| | Promise (in-band) | Record marker | `27(["promise"])` | | |
| | Error (in-band) | Record with message | `27(["error", "TypeError"])` | |
| OCapN messages contain "passable" data that may include references to remote | ||
| objects (targets), promises, and errors. To enable efficient message forwarding | ||
| through intermediaries without re-serialization, references are encoded using | ||
| **in-band markers** within an **embedded CBOR body**, with **parallel arrays** | ||
| mapping each marker to its CapTP table position. |
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.
So references don't bear any identifying metadata such as a slot index, and the idea is instead to use straightforward counting with repeats as necessary, such that targets [remotable1, remotable2] vs. [remotable1, remotable1] have the same in-band representation but target arrays that vary like e.g.
-[27(["desc:export", 1), 27(["desc:export", 2)]
+[27(["desc:export", 1), 27(["desc:export", 1)]`?
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.
should be merely [27(["desc:export"]), 27(["desc:export"])] in-band.
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.
Right, that covers "same in-band representation"... I'm asking about what the (presumably out-of-band) target arrays would look like for [remotable1, remotable2] vs. [remotable1, remotable1].
| The body of passable data is encoded as **Tag 24** (Encoded CBOR data item) | ||
| wrapping a byte string. This standard CBOR tag indicates that the byte string | ||
| contains valid CBOR data. |
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.
What is the structure of the envelope containing this body and some number of reference arrays? Is it itself a CBOR array or map, are there CBOR tags relevent to that level, etc.?
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.
The body is a CBOR encoded array. There is no additional tag, as writ.
bc494f5 to
de7e849
Compare
packages/ocapn/docs/cbor-encoding.md
Outdated
| A reference to a remote object (target) in the CapTP tables. | ||
|
|
||
| ``` | ||
| 27([280("target")]) |
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.
why a target string here? why not a CBOR tag number?
(likewise promise and error markers)
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.
Using a CBOR tag number would require a trip to IANA. 27 is a CBOR tag number we’re using to denote a Syrup record, which is analogous to a single-tier Cap’n Web array, e.g., ["date",ms]. CBOR tag number 280 indicates a symbol. So, this is a lot like the existing <desc:import-object> family of descriptors, but we’re collapsing all the targets, promises, and errors to place-holders and all other distinctions moving to slots, as is consistent with Endo’s marshaling. We could add back a gratuitous slot index to each of these, which would make it easier for a human to parse the CBOR diagnostic format.
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.
Using a CBOR tag number would require a trip to IANA.
'nuff said.
de7e849 to
bac01ff
Compare
| case 'number-prefix': | ||
| // Could be integer or float - check further or read as appropriate | ||
| const num = reader.readInteger(); // or readFloat64() |
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.
Wouldn't readInteger() misbehave if the value is actually floating point, and readFloat64() misbehave if the value is actually an integer? I see no value in conflating the two distinct types via a single value from peekTypeHint.
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.
These methods assert the type matches the expected type by direct observation. This is an artifact of our desire to, in many case, avoid reïfying certain values, and in particular, reïfying symbols as strings when it suits us (often).
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.
The explanation is sloppy. Working on that.
| | Error | Record with string label | `27(["error", "message"])` | | ||
| | Target (in-band) | Record marker | `27(["target"])` | | ||
| | Promise (in-band) | Record marker | `27(["promise"])` | | ||
| | Error (in-band) | Record with message | `27(["error", "TypeError"])` | |
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.
Why does Error appear twice?
| | 0 | Unsigned int | Length prefixes only (not for OCapN integers) | | ||
| | 1 | Negative int | Length prefixes only (not for OCapN integers) | |
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.
"not for OCapN integers"?
| ### Error | ||
|
|
||
| An error is a Record with string label "error" containing a message string. | ||
|
|
||
| ``` | ||
| D8 1B # Tag 27 (Record) | ||
| 82 # Array of 2 elements | ||
| 65 # Text string, 5 bytes | ||
| 65 72 72 6F 72 # "error" | ||
| <message string> | ||
| ``` | ||
|
|
||
| **Diagnostic**: `27(["error", "Error message here"])` |
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.
This seems inconsistent with Quick Reference—is the element after "error" a message or a type?
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.
My intention is that the second value in the array is a message and that it be carried in-band, while the respective identifier be carried out-of-band. There should actually be a third element with an OCapN Struct with additional data.
Description
This change introduces an experimental CBOR codec as an alternative to Syrup for OCapN message serialization. The implementation enables OCapN messages to be parsed by any RFC 8949 compliant CBOR decoder, allowing more of the ecosystem's existing tools to come to bear and making the wire format strictly a compact, machine-readable format.
New: CBOR Codec Implementation
New: CBOR Diagnostic Notation Codec
A text-based codec for human-readable CBOR representation:
src/cbor/diagnostic/encode.js- CBOR bytes → diagnostic stringsrc/cbor/diagnostic/decode.js- Diagnostic string → JavaScript valuessrc/cbor/diagnostic/util.js- Hex conversion and comparison helpersThis enables writing test cases in readable form and debugging encoding issues.
New: Codec Interface Abstraction
src/codec-interface.jsdefines abstractOcapnReaderandOcapnWriterinterfaces that both Syrup and CBOR implement. This enables:Codec Architecture: Dependency Injection
Applications import only the codec they need:
This replaces a hypothetical central factory, ensuring unused implementations are tree-shaken.
Test Infrastructure Enhancements
cbornpm packageDocumentation
docs/cbor-encoding.md- Complete specification covering:docs/codec-usage.md- Usage patterns for both codecssrc/cbor/README.md- Implementation overviewsrc/syrup/README.md- Updated with codec IDSecurity Considerations
desc:sig-envelopedesign wraps signed content in embedded CBOR (Tag 24) to enable signature verification against bytewise representationScaling Considerations
Documentation Considerations
The
docs/cbor-encoding.mdspecification is comprehensive and self-contained. Users wanting to implement an OCapN codec in another language should be able to do so from this document alone.Testing Considerations
Existing Syrup tests parameterized to cover CBOR.
Inludes tests that verify that cross-reference legibility of generated CBOR messages with an off-the-shelf, generic CBOR implementation.
Round-trip and canonicalization tests.
Compatibility Considerations
None.
Upgrade Considerations
None.