Skip to content

Commit

Permalink
fix(marshal): serialize empty objects as data, not pass-by-reference
Browse files Browse the repository at this point in the history
Finally change the serialization of a plain empty object, i.e. `harden({})`,
from a pass-by-reference "marker" to plain pass-by-copy data. We think we've
identified all the places where an empty object was used represent a marker,
and changed them to use `Far(iface, {})` instead, which triggers
pass-by-reference.

refs #2018
  • Loading branch information
warner committed Mar 12, 2021
1 parent 86440ca commit aeee1ad
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 24 deletions.
9 changes: 0 additions & 9 deletions packages/marshal/src/marshal.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,15 +289,6 @@ function isPassByCopyRecord(val) {
const descs = getOwnPropertyDescriptors(val);
const descKeys = ownKeys(descs);

// Empty non-array objects must be registered with Far/Remotable, or Data
// This causes a warning for now, eventually it will become an
// error, then (TODO) it will go back to pass-by-copy.
// See https://github.com/Agoric/agoric-sdk/issues/2018
if (descKeys.length === 0 && proto === objectPrototype) {
// console.log(`--- @@marshal: empty object without Data/Far/Remotable`);
// assert.fail(X`empty object without Data/Far/Remotable`);
return false;
}
for (const descKey of descKeys) {
if (typeof descKey === 'symbol') {
return false;
Expand Down
6 changes: 0 additions & 6 deletions packages/marshal/test/test-marshal-stringify.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,6 @@ test('marshal stringify errors', t => {
t.throws(() => stringify(Far('y', {})), {
message: /Marshal's stringify rejects presences and promises .*/,
});

// This is due to https://github.com/Agoric/agoric-sdk/issues/2018
// and should no longer be an error once this is fixed.
t.throws(() => stringify(harden({})), {
message: /Marshal's stringify rejects presences and promises .*/,
});
});

test('marshal parse errors', t => {
Expand Down
12 changes: 3 additions & 9 deletions packages/marshal/test/test-marshal.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export const roundTripPairs = harden([
[1, 2],
],
[{ foo: 1 }, { foo: 1 }],
[{}, {}],
[
{ a: 1, b: 2 },
{ a: 1, b: 2 },
Expand Down Expand Up @@ -503,13 +504,7 @@ test('records', t => {
);

// harden({})
// old: pass-by-ref without complaint
// interim1: pass-by-ref with warning
// interim2: rejected
// final: pass-by-copy without complaint
t.deepEqual(ser(build()), noIface); // old+interim1
// t.throws(() => ser(harden({})), { message: /??/ }, 'unmarked empty object rejected'); // int2
// t.deepEqual(ser(build()), emptyData); // final
t.deepEqual(ser(build()), emptyData);

// Data({key1: 'data'})
// old: not applicable, Data() not yet added
Expand All @@ -518,8 +513,7 @@ test('records', t => {
const key1Data = { body: JSON.stringify({ key1: 'data' }), slots: [] };
t.deepEqual(ser(build('enumStringData', 'data')), key1Data);

// Serialized data should roundtrip properly. The first case here will
// trigger a warning during interim1.
// Serialized data should roundtrip properly
t.deepEqual(unser(ser(harden({}))), {});
t.deepEqual(unser(ser(harden({ key1: 'data' }))), { key1: 'data' });

Expand Down

0 comments on commit aeee1ad

Please sign in to comment.