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

Relative objects in wasm #947

Merged
merged 4 commits into from
Jul 14, 2023
Merged

Relative objects in wasm #947

merged 4 commits into from
Jul 14, 2023

Conversation

graydon
Copy link
Contributor

@graydon graydon commented Jul 13, 2023

This is a much simpler approach to the "object read-access-control" problem covered in PRs #940 and #930 (and previously #132). The idea here is:

  1. We split object references as before in two types differentiated by their low bit, but now they're "absolute" and "relative". Relative references indirect through a context-local table that just holds absolute references.
  2. We only really care about this issue when dealing with wasm contracts. Local-testing / native contracts have no isolation / complete system visibility anyways, there's no point really worrying about "who can see what".
  3. Therefore the wasm VM argument-marshalling path is a natural point to do the relative-to-absolute translation. We translate relative references to absolute on the way in to the host, and absolute to relative on the way out. Wasm never gets to see any absolute references. If they forge one, we refuse it on the way in.

(There is a slight downside in the current implementation which is that if you return the same absolute reference multiple times we'll create new relative references on each return. If this is annoying we can use a hashtable or something instead of a vector. But the vector has both performance and conceptual simplicity on its side, and in any case the user shouldn't care about specific object references, and I expect most contracts do not run long enough or return the same object references repeatedly enough for this to matter either way)

@graydon graydon requested review from sisuresh and a team as code owners July 13, 2023 08:28
@graydon graydon force-pushed the relative-objects branch 3 times, most recently from 1ce9c6b to 54578f7 Compare July 13, 2023 23:22
@graydon graydon requested a review from dmkozh July 14, 2023 01:05
@graydon graydon force-pushed the relative-objects branch 2 times, most recently from e2c82fb to 059cce1 Compare July 14, 2023 01:06
@graydon
Copy link
Contributor Author

graydon commented Jul 14, 2023

@dmkozh I believe this is ready to go now, and is as minimal as I can make it. I added a test and localized the changes to the host (and actually just the VM frame type). At this point more of the diff is just legibility change (I changed the VM frame type from a tuple to a struct) than anything else.

soroban-env-host/src/test/hostile.rs Outdated Show resolved Hide resolved
@graydon graydon merged commit d099f93 into main Jul 14, 2023
8 checks passed
@graydon graydon deleted the relative-objects branch July 14, 2023 20:56
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.

3 participants