Skip to content

Commit

Permalink
fix(compartment-mapper): fix #2407 include symbol-named properties (#…
Browse files Browse the repository at this point in the history
…2408)

Closes: #2407 
Refs: Agoric/agoric-sdk#9874
Agoric/agoric-sdk#9431
Agoric/agoric-sdk#9781 #2377

## Description

While working on Agoric/agoric-sdk#9874 , I
found that it still failed to endow `passStyleOf` via a symbol-named
property as recently agree.

Turns out there was still a lurking `Object.keys` on the compartment
endowment pathway that needs to be changed to enumerate all enumerable
own properties, whether string-named or symbol-named.

### Security Considerations

None

### Scaling Considerations

For this PR, none. But it enables liveslots to endow its `passStyleOf`
to the compartments it makes, which has the scaling benefits explained
at Agoric/agoric-sdk#9781

### Documentation Considerations

For the normal developer, none. Once endowed, `passStyleOf` will have
performance that is less surprising.

### Testing Considerations

Agoric/agoric-sdk#9874 both patches in the
equivalent of this PR, uses it to endow `passStyleOf`, and test that it
has done so, across both node and XS.

However, after this fix, Agoric/agoric-sdk#9874
works on the Node host. But it still fails on the XS host for
undiagnosed reasons, so there may be another bug like #2407 still
lurking somewhere. Until diagnosed, it isn't clear if that remaining
problem is an endo problem or an agoric-sdk problem.

### Compatibility and Upgrade Considerations

Except for code meant only to test this, `passStyleOf` should not have
any observable difference. We have not attempted to make an other use of
symbol-named global properties, so there should be no compat or upgrade
issues.
  • Loading branch information
erights authored Aug 22, 2024
1 parent 8fc0ef7 commit 2b799f8
Showing 1 changed file with 12 additions and 3 deletions.
15 changes: 12 additions & 3 deletions packages/compartment-mapper/src/policy.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import {
policyLookupHelper,
} from './policy-format.js';

const { create, entries, values, assign, keys, freeze } = Object;
const { create, entries, values, assign, freeze, getOwnPropertyDescriptors } =
Object;
const { ownKeys } = Reflect;
const q = JSON.stringify;

/**
Expand All @@ -19,7 +21,9 @@ const q = JSON.stringify;
export const ATTENUATORS_COMPARTMENT = '<ATTENUATORS>';

/**
* Copies properties (optionally limited to a specific list) from one object to another.
* Copies properties (optionally limited to a specific list) from one object
* to another. By default, copies all enumerable, own, string- and
* symbol-named properties.
*
* @param {object} from
* @param {object} to
Expand All @@ -28,7 +32,12 @@ export const ATTENUATORS_COMPARTMENT = '<ATTENUATORS>';
*/
const selectiveCopy = (from, to, list) => {
if (!list) {
list = keys(from);
const descs = getOwnPropertyDescriptors(from);
list = ownKeys(from).filter(
key =>
// @ts-expect-error TypeScript still confused about a symbol as index
descs[key].enumerable,
);
}
for (let index = 0; index < list.length; index += 1) {
const key = list[index];
Expand Down

0 comments on commit 2b799f8

Please sign in to comment.