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

docs(patterns): explain invariants among our orders #1590

Merged
merged 6 commits into from
Sep 12, 2023

Conversation

erights
Copy link
Contributor

@erights erights commented May 20, 2023

So far, just porting Agoric/agoric-sdk#4435 to endo. Starting out very stale.

@erights erights self-assigned this May 20, 2023
@erights erights changed the title docs(patterns): migrate agoric-sdk#4435 docs(patterns): explain invariants among our orders May 21, 2023
@erights erights force-pushed the markm-document-store-invariants branch from 9b5292b to 71080e1 Compare June 6, 2023 03:09
@erights erights requested a review from gibson042 August 28, 2023 06:38
@erights
Copy link
Contributor Author

erights commented Aug 28, 2023

@gibson042 what in here is still valuable?

@erights erights force-pushed the markm-document-store-invariants branch from 71080e1 to 2aaa42a Compare August 28, 2023 06:38
Copy link
Contributor

@gibson042 gibson042 left a comment

Choose a reason for hiding this comment

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

@gibson042 what in here is still valuable?

I think there's a lot of value, especially in marshal-vs-patterns-level.md. But I would request that it be linked to from the README, and also that we minimize duplicated information (though I don't have a preference for pointing into types.js vs. pointing out from it into Markdown).

packages/patterns/docs/marshal-vs-patterns-level.md Outdated Show resolved Hide resolved
packages/patterns/docs/marshal-vs-patterns-level.md Outdated Show resolved Hide resolved
packages/patterns/docs/marshal-vs-patterns-level.md Outdated Show resolved Hide resolved

## `kindOf` *vs* `passStyleOf`

For every passable value except Tagged, `kindOf(p) === passStyleOf(p)`. They differ only when `passStyleOf(p) === 'tagged'`. For those, either the `kindOf` level recognizes the Tagged as encoding one of the higher level types defined by the `kindOf` level, or it does not. If it does not, then `kindOf(p) === undefined`. Only the `passStyleOf` level is assumed for universal interoperability. Different participants may encode and recognize different extensions using the same Tagged extension point. If a participant that does not recognize a particular Tagged according to a higher layer it uses, it must still treat it as a valid Tagged object, which must still round trip. Alice might send a Tagged she recognizes to Bob, who does not, but who sends it on to Carol who does.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is super useful, but makes me wonder even more about whether we should incorporate unordered set and/or map into the passStyleOf level as suggested in #1739. It would mean that full ordering could be applied even to unrecognized tagged types, although that ordering might differ from kind-specific ordering.

However, this is certainly not the best place for that discussion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand this. We do apply full rank orderings to unrecognized tag types, in that rankCompare just does lexicographic ordering of Taggeds, independent of any higher level recognition. Do I misunderstand the question?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re #1739 , from that thread, I think we resolved that we should just use unicode / utf-8 ordering for rankCompare, and therefore keyCompare, and avoid using UTF-16 / CESU-8 ordering for anything at these levels of abstraction. Does that help clarify anything here?

To make that switch, we'll need to ensure that our stores (heap, virtual, and durable) sort their keys in a way that remains consistent with rankCompare. This may conflict with existing durable data, which will be an interesting problem.

Attn @FUDCo @warner

packages/patterns/docs/marshal-vs-patterns-level.md Outdated Show resolved Hide resolved
packages/patterns/docs/marshal-vs-patterns-level.md Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

The content of this file isn't really comprehensible to me. I don't know if there's a good way to present it other than prose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please identify what you don't understand. But I suspect that anything here not comprehensible to you probably doesn't make sense and should be deleted ;)

@gibson042 gibson042 force-pushed the markm-document-store-invariants branch 2 times, most recently from 8c3ebee to f12d975 Compare September 10, 2023 19:29
@gibson042
Copy link
Contributor

@erights This is ready for you, but you won't be able to submit an actual review since you initially opened it. I think we'll eventually also want to document the specifics of rank order (such as special number values, strings, and using Pareto order to translate subset-like relationships in key comparison) in @endo/marshal.

@gibson042 gibson042 marked this pull request as ready for review September 10, 2023 19:34
Copy link
Contributor Author

@erights erights left a comment

Choose a reason for hiding this comment

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

Virtual LGTM, thanks! Awesome to see this!

Now that I've virtually approved, IIUC, you can actually approve and then merge, yes?

(If possible, would be nice to see this merged before ocapn mtg tomorrow)

@erights erights force-pushed the markm-document-store-invariants branch from f12d975 to 0934951 Compare September 11, 2023 22:08
@gibson042 gibson042 force-pushed the markm-document-store-invariants branch from 0934951 to a4ee548 Compare September 12, 2023 18:22
@gibson042 gibson042 merged commit f891df0 into master Sep 12, 2023
14 checks passed
@gibson042 gibson042 deleted the markm-document-store-invariants branch September 12, 2023 18:34
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