librad: provider announced non-existent rev

[geigerzaehler]

We’re seeing the following warning after receiving a gossip announcement for the rad/id path:

Aug 17 14:22:18.569  WARN librad::net::peer::storage: provider announced non-existent rev,
  provider: hynnmiewkxb6uoxnnjknx6n5a6mhsco3fzybp314nffrmcss5gwo5w,
  announced: Payload { urn: Urn { id: Oid(05458e67eb40d82a2eaee912de43bf0dbf27d6c5), path: Some(RefLike("rad/id")) }, rev: Some(Git(681fbbb93e614f969996765dce7d610add298900)), origin: None }

After doing some debugging, it looks like Storage::has_fetch() looks up the transformed URN

refs/namespaces/hnrkyktcqc9iwbsbkf4zq1rs6eq9o5x3845no/refs/remotes/hynnmiewkxb6uoxnnjknx6n5a6mhsco3fzybp314nffrmcss5gwo5w/heads/rad/id

Note the extra heads component.

It looks like the urn_context function in the storage module is not correct.

[kim]

Yeah, I think announcing rad/id is wrong -- there is no way on the receiving end to distinguish between if that is supposed to refer to refs/heads/rad/id or refs/rad/id without hardcoding special knowledge about rad/*. The normal DWIM rules are that path-not-starting-with-refs/ is relative to refs/heads/, which is what we do.

[FintanH]

Yeah, I think announcing rad/id is wrong

So basically, if there's an update to the identity and it's to be announced do we just announce without a path set? Or in other words, how should an update to an identity be announced?

[kim]

 Yeah, I think announcing rad/id is wrong

So basically, if there's an update to the identity and it's to be announced do we just announce without a path set? Or in other words, how should an update to an identity be announced?

Since we defined that the default path is refs/rad/id, it would technically work without a path -- because the first thing we fetch is refs/rad/{id, signed_refs, ids/*}. I can't think of a situation in which we would want to distinguish between "it is specifically rad/id which got updated" and "there's some undisclosed update for this URN, go check for yourself". But maybe there is?

[FintanH]

I can't think of a situation in which we would want to distinguish between "it is specifically rad/id which got updated" and "there's some undisclosed update for this URN, go check for yourself". But maybe there is?

I believe the code that executes this in Upstream simply iterates over the Refs and if there's new updates it'll announce them. Which now that I say it, I'm wondering why the path isn't set to refs/rad/id, because I'm pretty sure the into_qualified function is used. But alas, I'm wrong https://github.com/radicle-dev/radicle-link/blob/master/daemon/src/peer/announcement.rs#L91.

I think the use of into_qualified would actually fix this because then the path should be refs/rad/id. I imagine this fix will be needed when cobs come down the line and we want to only send out updates for those paths.

[kim]

I'm wondering why the path isn't set to refs/rad/id, because I'm pretty sure the into_qualified function is used. But alas, I'm wrong

Strings are hard.

[FintanH]

Strings are hard.

There's a whole theory about them