Skip to content
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

switch to new marshalling format everywhere #6822

Closed
6 tasks done
warner opened this issue Jan 20, 2023 · 6 comments
Closed
6 tasks done

switch to new marshalling format everywhere #6822

warner opened this issue Jan 20, 2023 · 6 comments
Assignees
Labels
enhancement New feature or request vaults_triage DO NOT USE
Milestone

Comments

@warner
Copy link
Member

warner commented Jan 20, 2023

What is the Problem Being Solved?

During today's show-and-tell presentation by @samsiegart , @erights noticed that the chain-storage published data was still using the old @qclass serialization format. We switched SwingSet to use the serializeBodyFormat: 'smallcaps' format in f289ec0 (#6326), but there are other places in agoric-sdk which use makeMarshal to serialize data, which are still using the old format. I found the following with rgjs -w makeMarshal |grep -v test |grep -v SwingSet | grep -v import:

cache/src/store.js:  const { serialize: keyToJsonable } = makeMarshal(valToSlot);
casting/src/defaults.js:    unserialize: makeMarshal(undefined, slotToVal).unserialize,
cosmic-swingset/src/chain-main.js:      const marshaller = makeMarshal();
deploy-script-support/src/code-gen.js:const { serialize } = makeMarshal();
notifier/src/storesub.js:  marshaller = makeMarshal(undefined, undefined, {
vats/src/lib-board.js:    ...makeMarshal(val => {
vats/src/lib-board.js:    ...makeMarshal(
wallet/api/src/lib-dehydrate.js:  const { serialize: dehydrate, unserialize: hydrate } = makeMarshal(
wallet/api/src/lib-wallet.js:  const { unserialize: fillInSlots } = makeMarshal(noOp, identitySlotToValFn, {
wallet/api/src/marshal-contexts.js:    ...makeMarshal(valToSlot, slotToVal),
wallet/api/src/marshal-contexts.js:    fromBoard: makeMarshal(valToSlot.fromBoard, slotToVal.fromBoard, {
wallet/api/src/marshal-contexts.js:    fromMyWallet: makeMarshal(valToSlot.fromMyWallet, slotToVal.fromMyWallet, {

The basic fix is to add serializeBodyFormat: 'smallcaps' to the options bag, but since the chain already has IAVL-tree data in the old format (which won't go away during the bulldozer upgrade), we must make sure the client-side tools that follow that data will be able to handle both. makeMarshal is tolerant of both, but if we have any other code that thinks it knows about the serialization details, that code will need to change.

We had originally planned to drop read-side support for the old format from makeMarshal after completing the transition of all clients. However the presence of old-format IAVL data on the chain may prevent us from doing that, or perhaps preventing the client code from upgrading to a newer version of makeMarshal.

BTW, makeMarshal lives in the Endo project/repository.

Test plan

Most of these should work automatically by the Casting abstraction.

@warner warner added the enhancement New feature or request label Jan 20, 2023
@mhofman
Copy link
Member

mhofman commented Feb 6, 2023

Please consider also doing #6879 / endojs/endo#1478 if/when doing this.

@dckc
Copy link
Member

dckc commented Feb 16, 2023

I just made #7013 to explore changing the board marshaller and hence smart wallet etc.

As to the others...

cache/src/store.js: const { serialize: keyToJsonable } = makeMarshal(valToSlot);

I'm not sure about that one... I suspect it's not used. @michaelfig ?

casting/src/defaults.js: unserialize: makeMarshal(undefined, slotToVal).unserialize,

That's unserialize-only; it will Just Work.

cosmic-swingset/src/chain-main.js: const marshaller = makeMarshal();

That one goes to chainStorage. I should add it to #7013 .

deploy-script-support/src/code-gen.js:const { serialize } = makeMarshal();

The output of that one is fed immediately to decodeToJustin, so it's invisible. (might be worth upgrading anyway)

notifier/src/storesub.js: marshaller = makeMarshal(undefined, undefined, {

adding that to #7013

vats/src/lib-board.js: ...makeMarshal(val => {
vats/src/lib-board.js: ...makeMarshal(

That's where I started.

wallet/api/src/lib-dehydrate.js: const { serialize: dehydrate, unserialize: hydrate } = makeMarshal(
wallet/api/src/lib-wallet.js: const { unserialize: fillInSlots } = makeMarshal(noOp, identitySlotToValFn, {

I think those are only used in the ag-solo wallet.

wallet/api/src/marshal-contexts.js: ...makeMarshal(valToSlot, slotToVal),
wallet/api/src/marshal-contexts.js: fromBoard: makeMarshal(valToSlot.fromBoard, slotToVal.fromBoard, {
wallet/api/src/marshal-contexts.js: fromMyWallet: makeMarshal(valToSlot.fromMyWallet, slotToVal.fromMyWallet, {

I suspect marshal-contexts.js is dead code. I should try pruning it to be sure.

@dckc dckc self-assigned this Apr 17, 2023
@warner
Copy link
Member Author

warner commented May 1, 2023

On one hand, smaller serialized data means fewer bytes published to IAVL, which will speed up validators (slightly) and speed up client access (slightly), which are good.

On the other hand, asking off-chain developers to use a custom deserializer for the data they fetch from the chain is a drag. OT3H they pretty much have to do that for the old format too, since JSON doesn't even support BigInts (which are pervasive in the data we publish), but at least plain JSON.parse on the old format doesn't throw, whereas the deliberate leading # in the smallcaps format (used to distinguish between old and new) raises the entry bar to even get started experimenting with a decoder (which we know folks will do anyways, no matter how well we document the format).

I think that, if it doesn't cause insurmountable problems for the front end, that we should make this change while we still can (i.e. before the release).

@dckc
Copy link
Member

dckc commented May 1, 2023

warner added a commit that referenced this issue May 3, 2023
This changes every `makeMarshal()` instance which performs object
graph serialization to add the `{serializeBodyFormat: 'smallcaps'}`
option. All emitted data will be in the "smallcaps" format, which is
more concise (less overhead). For example, `1n` (a BigInt) is encoded
as `#"+1"` instead of `{"@qclass":"bigint","digits":"1"}`.

The old format was JSON except with those special `@qclass` objects to
capture everything that JSON could not, including object
references ("slots"). The new format is a leading `#` prefix, plus
JSON, except that all the special cases are expressed as strings.

Most significantly, this changes the format of the data we publish to
`chainStorage`.  The old format encoded simple data as mostly JSON, so
bespoke parsers mostly worked, but even BigInts required special
handling. Off-chain followers who perform RPC queries to the chain are
highly encouraged to use the `@endo/marshal` library (or equivalent)
to parse the data. This library can understand old-format data too,
such as chain data published before this upgrade.

Notable changes:

* Many ad-hoc serializers were replaced with a real `makeMarshal`,
  which requires hardened inputs, so some new `harden()` calls were
  added
* Some marshallers which only perform unserialization (`fromCapData`)
  were not modified
* All unit tests which compared serialized data against
  manually-constructed examples had their examples updated
* Previously, the `makeMockChainStorageRoot()` utility parsed
  Far/Remotable objects into an object with `{ iface }`. Now, it is
  unserialized into actual Remotables. Both `t.deepEqual` and
  `t.snapshot` will correctly compare the `iface` values of two Far
  objects, so tests should simply construct a `Far(iface)` and include
  it in the specimen data. As a result, many `t.snapshot`-style tests
  had their snapshot data changed.

closes #6822

Co-authored-by: Chip Morningstar <[email protected]>
warner added a commit that referenced this issue May 3, 2023
This changes every `makeMarshal()` instance which performs object
graph serialization to add the `{serializeBodyFormat: 'smallcaps'}`
option. All emitted data will be in the "smallcaps" format, which is
more concise (less overhead). For example, `1n` (a BigInt) is encoded
as `#"+1"` instead of `{"@qclass":"bigint","digits":"1"}`.

The old format was JSON except with those special `@qclass` objects to
capture everything that JSON could not, including object
references ("slots"). The new format is a leading `#` prefix, plus
JSON, except that all the special cases are expressed as strings.

Most significantly, this changes the format of the data we publish to
`chainStorage`.  The old format encoded simple data as mostly JSON, so
bespoke parsers mostly worked, but even BigInts required special
handling. Off-chain followers who perform RPC queries to the chain are
highly encouraged to use the `@endo/marshal` library (or equivalent)
to parse the data. This library can understand old-format data too,
such as chain data published before this upgrade.

Notable changes:

* Many ad-hoc serializers were replaced with a real `makeMarshal`,
  which requires hardened inputs, so some new `harden()` calls were
  added
* Some marshallers which only perform unserialization (`fromCapData`)
  were not modified
* All unit tests which compared serialized data against
  manually-constructed examples had their examples updated
* Previously, the `makeMockChainStorageRoot()` utility parsed
  Far/Remotable objects into an object with `{ iface }`. Now, it is
  unserialized into actual Remotables. Both `t.deepEqual` and
  `t.snapshot` will correctly compare the `iface` values of two Far
  objects, so tests should simply construct a `Far(iface)` and include
  it in the specimen data. As a result, many `t.snapshot`-style tests
  had their snapshot data changed.

refs #6822

Co-authored-by: Chip Morningstar <[email protected]>
warner added a commit that referenced this issue May 3, 2023
This changes every `makeMarshal()` instance which performs object
graph serialization to add the `{serializeBodyFormat: 'smallcaps'}`
option. All emitted data will be in the "smallcaps" format, which is
more concise (less overhead). For example, `1n` (a BigInt) is encoded
as `#"+1"` instead of `{"@qclass":"bigint","digits":"1"}`.

The old format was JSON except with those special `@qclass` objects to
capture everything that JSON could not, including object
references ("slots"). The new format is a leading `#` prefix, plus
JSON, except that all the special cases are expressed as strings.

Most significantly, this changes the format of the data we publish to
`chainStorage`.  The old format encoded simple data as mostly JSON, so
bespoke parsers mostly worked, but even BigInts required special
handling. Off-chain followers who perform RPC queries to the chain are
highly encouraged to use the `@endo/marshal` library (or equivalent)
to parse the data. This library can understand old-format data too,
such as chain data published before this upgrade.

Notable changes:

* Many ad-hoc serializers were replaced with a real `makeMarshal`,
  which requires hardened inputs, so some new `harden()` calls were
  added
* Some marshallers which only perform unserialization (`fromCapData`)
  were not modified
* All unit tests which compared serialized data against
  manually-constructed examples had their examples updated
* Previously, the `makeMockChainStorageRoot()` utility parsed
  Far/Remotable objects into an object with `{ iface }`. Now, it is
  unserialized into actual Remotables. Both `t.deepEqual` and
  `t.snapshot` will correctly compare the `iface` values of two Far
  objects, so tests should simply construct a `Far(iface)` and include
  it in the specimen data. As a result, many `t.snapshot`-style tests
  had their snapshot data changed.

refs #6822

Co-authored-by: Chip Morningstar <[email protected]>
@dckc
Copy link
Member

dckc commented May 6, 2023

I gather @warner and @FUDCo have done their bit and @samsiegart is checking the dapps.

@ivanlei
Copy link
Contributor

ivanlei commented May 10, 2023

All done!

@ivanlei ivanlei closed this as completed May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request vaults_triage DO NOT USE
Projects
None yet
Development

No branches or pull requests

6 participants