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

Far and Remotable do unverified local marking rather than WeakMap #2361

Merged
merged 5 commits into from
Feb 14, 2021

Conversation

erights
Copy link
Member

@erights erights commented Feb 8, 2021

Far and Remotable still splice an object into the inheritance chain, but now that object also contains a symbol-named property marking the object as a remotable. Unlike the WeakMap, this is not a reliable brand, so passStyleOf must validate it each time.

Corrected for a previous misunderstanding of Object.entries. I thought it did all enumerable own properties, like .... Actually it only does string-named enumerable own properties. I avoid it when that restriction is inappropriate.

The marshal package no longer exports mustPassByRemote or mustPassByPresence. Instead, clients should use passStyleOf.

I limited Remotable to the current usage pattern: The props must be undefined. The remotable if provided must inherit from null or Object.prototype. The inheritance restriction propagates to Far.

@erights erights self-assigned this Feb 8, 2021
@erights erights force-pushed the narrow-entries branch 3 times, most recently from c5180e3 to 8b9867f Compare February 14, 2021 08:18
@erights
Copy link
Member Author

erights commented Feb 14, 2021

Ready for review!

@michaelfig Please look at the changes to the wallet test. If they are correct now, were they not correct before? If so, I'm unclear on why it was not working before this PR.

@erights erights changed the title Use entries only on non-problematic properties Far and Remotable do unverified local marking rather than WeakMap Feb 14, 2021
@michaelfig
Copy link
Member

@michaelfig Please look at the changes to the wallet test. If they are correct now, were they not correct before?

They were correct before, modulo the fact that the test was using the fakeVatAdmin. That entailed separate import namespaces for the contract and the test, and hence separate getInterface WeakMaps, but without a marshal boundary between the two of them.

So, ZCF's makeHandle('Invitation') did not affect the test's WeakMap and so its interface name was lost. The test relied on that behaviour.

If so, I'm unclear on why it was not working before this PR.

[Assuming you meant "why it was working".]

@dtribble and I had figured out how to test contracts with actual marshal layers between them. I think that would have made the test always behave as you expect.

Copy link
Member

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I reviewed all the changes, since I had some familiarity with the plan to proceed with them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants