[DO NOT MERGE] Sketch of fix for #268 #737
Conversation
*This is a types-only change.* Before this change, we used `any` for a Hypercore peer's type. After this change, we use a new type, `HypercorePeer`, designed to match [Hypercore's `Peer` class][0]. Ideally, we would put this in `@digidem/types` or DefinitelyTyped. For now, I think this is an improvement over the `any`s we currently use. I did this because I'm working with peer-related code and found the `any`s difficult to work with. [0]: https://github.com/holepunchto/hypercore/blob/de993dc051301f03570df0eb7929c06286745563/lib/replicator.js#L331
| // not yet synced role records. | ||
| return NO_ROLE | ||
| } | ||
| async getRole(deviceId, options = {}) { |
There was a problem hiding this comment.
I chose to add new behavior to Roles, as I felt it was a clean place to put it. Let me know if that seems wrong.
|
The original issue is about keys that are being sent to peers, where-as this sketch seems to be focussed on what to do with keys that are received. E.g. we don't want to send the hypercore keys for anything other than the auth cores unless we actually validate that a peer is part of a project. Currently as long as a peer has the project key, we send keys for all cores. We also send "haves". We want to change behaviour so that only auth core keys are sent to peers that have the project key, and then only send keys for cores in other namespaces once we evaluate whether that peer is authorized to sync that namespace. Similarly with sending "haves". This means that we need to check the capabilities and roles before sending this data, and we also need to listen for updates to capabilities and roles, because as auth cores sync, this data can update/change. Did you see the draft PR for this https://github.com/digidem/mapeo-core-next/pull/390/files? I think that is roughly a reasonable approach, but there are probably a few errors in it and I don't think tests were actually passing. |
|
Closing because we don't intend to incorporate this. |
I recommend reviewing this with whitespace changes disabled.
I'm unsure of the right approach for #268. This incomplete PR sketches out what I think it should look like.