radicle-dev / radicle-link

The second iteration of the Radicle code collaboration protocol.
Other
421 stars 39 forks source link

identities: updated delegation causes replication failure #711

Closed FintanH closed 2 years ago

FintanH commented 3 years ago

We have the following steps:

Error:

Revision 7bb4bbce268d89a62e1962f5dded7a9f68c9e665 of 4b84e8407d602317b01f22207908dbea1d9d4e17 not in ancestry path of eada5419806f643d402c92b790c9b471098f1e81

The error originates from updated_person, which seems to not consider the fact that the Person in the delegate is a fast-forward of the Person that Peer 2 knows about at the top-level. Instead, it only checks that the Person delegate was a previous version of the Person at the top-level.

Should this check be done for fast-forwards as well and do the following if it is?

self.as_person().verify(known.revision)
kim commented 3 years ago

I think this is related to #709, which, however, doesn't actually fix the root cause. At some point, it did work that a fetch would update the top-level person-typed namespace, such that the symref can never be ahead or behind. I'm not sure why or when this broke, but it is possible that we shouldn't rely on it anyways: there is still a TODO which punts on the problem that technically more top-level namespaces need to be updated, because the tracking graph may have changed.

A possible fix could thus be that #709 resets the top-level rad/id to the local remotes/../rad/self iff the latter can be fast-forwarded onto the former. I also think that the fetch turns any symrefs into direct refs if they get updated as a result of the fetch, so the symrefs need to be forced always.

FintanH commented 3 years ago

At some point, it did work that a fetch would update the top-level person-typed namespace, such that the symref can never be ahead or behind.

I think in this case because the rad/id of the Person is replicated before the Project it doesn't get a chance to create the symref, because it will fail the verification with the above error before it gets to the symref creation point.

Hmmm, actually it seems we've had a similar conversation before here: https://github.com/radicle-dev/radicle-link/issues/420.

A possible fix could thus be that #709 resets the top-level rad/id to the local remotes/../rad/self iff the latter can be fast-forwarded onto the former. I also think that the fetch turns any symrefs into direct refs if they get updated as a result of the fetch, so the symrefs need to be forced always.

Ok, I can look into this on top of #709 :)

kim commented 3 years ago

I think in this case because the rad/id of the Person is replicated before the Project it doesn't get a chance to create the symref, because it will fail the verification with the above error before it gets to the symref creation point.

Ah yes... peer 2 already has the top-level namespace, that's why. We would need to do another roundtrip to first fetch everything in rad/ids in order to make sure that the top-level is up-to-date.

That's annoying.

I think what I proposed could still be viable -- after the Peek, the top-level can be reset to the corresponding rad/ids/XXX if and only if that'd be a fast-forward.

This may render the identity invalid, however, so we would need to verify it before replicating it again (and not replicate if it became invalid). In this case, the project would also fail to verify -- which has dubious semantics.

So I'm not sure. Maybe the correct thing to do is to only verify the project against the state of rad/ids. If both rad/namespaces/XXX/refs/rad/id and refs/rad/ids/XXX verify and are reachable from each other, the former can be reset to the more recent one, and the latter turned into a symref. If the latter verifies, but the former doesn't, then the project is in some kind of limbo state.

FintanH commented 2 years ago

Is there anything blocking this @kim? Or is it a matter of you working on replication v3?