add docs to identity_key.rs

[cosmicexplorer]

Broken out of #287.

[jrose-signal]

Thank you!

[jrose-signal]

I'm not sure what you mean by "for the given signature". The signature is used to perform the verification, but if the verification passes, self and other represent the same user in all cases, not just regarding this signature.

[cosmicexplorer]

I was trying to clarify why the signature is needed for this method, and wasn't quite understanding what "alternate" identity was referring to. I have modified this to:

/// Given a trusted identity `self`, verify that `other` represents an alternate identity for
/// this user.
///
/// `signature` must be calculated from [`IdentityKeyPair::sign_alternate_identity`].

It looks like the alternate identity calculation mechanism is consumed in the app somewhere; I'll take a look at documenting that more fully in another PR perhaps.

[jrose-signal]

It's part of the Phone Number Privacy project but I don't know if anyone's hooked it up end-to-end on the app side yet. But as usual that's just the current use case; the API's designed for future sorts of "alternate identities" as well.

[jrose-signal]

Since the last round of PRs we've changed libsignal to match the other Signal repositories in developing mostly in private and doing public releases, with external contributions being cherry-picked into the upcoming release's private branch. That's not so great for an ongoing patch series like this one, though. Perhaps we can make a side branch your PRs are targeted to, docs-effort or something, so that you can build on your own work between releases? While meanwhile they are also cherry-picked to our private main.

[clauz9]

Since the last round of PRs we've changed libsignal to match the other Signal repositories in developing mostly in private and doing public releases,

I was just admiring the fact that the development for libsignal is a lot more transparent, and publicly accessible, compared to the clients 🤔 I love the fact that I am able to see developers making PR's for features and reviewing each other's code, it's incredibly educational.

so, can I ask, why the change ?

[jrose-signal]

There wasn't one big reason, but a bunch of small things adding up:

• Being inconsistent with the other Signal projects means that it's harder to write automation generic across all Signal-managed projects.

• Being inconsistent with other Signal projects means that Signal employees have to remember to be more circumspect on libsignal reviews (i.e. not mentioning possible future features or security concerns that could get taken out of context, which has historically been quite frustrating). Our releases are scrutinized quite closely throughout the internet already; having our discussion be public too was 90% okay and 10% walking on eggshells. (I think I got congratulated on joining Signal within a few days because of this GitHub account as well as my https://community.signalusers.org account, even though I hadn't said anything elsewhere.)

  This still comes up sometimes with public forks (signalapp/boring being the most active example for libsignal at the moment; will be used soon), but that's usually a lot rarer.

• Even in the original development style, we sometimes wanted specific reviews to be discussed privately, for similar reasons. Making the most sensitive reviews the ones with the more awkward process was optimizing for the wrong thing.

I'm a little sad too, but ultimately while it was nice to have a "fully open" development, that's not really the main goal of the project. The thing we optimize for is "supporting the Signal client apps" rather than "making a model open-source library", and I do think that's the right choice.

[This is not something that can be argued with, but if you want to keep discussing it please move that discussion to https://community.signalusers.org, so we can go back to talking about the PR content and how to integrate it!]

[clauz9]

Signal employees have to remember to be more circumspect on libsignal reviews (i.e. not mentioning possible future features or security concerns that could get taken out of context, which has historically been quite frustrating)

yes, this makes perfect sense..

This is not something that can be argued with, but if you want to keep discussing it please move that discussion to https://community.signalusers.org, 

No need, your answer was quite detailed, thanks for taking the time to write it!

[cosmicexplorer]

I have updated the description in the tracking issue #467 to note that I am maintaining a branch named docs-effort at https://github.com/cosmicexplorer/libsignal-client/tree/docs-effort with all these changes together! @jrose-signal I appreciate your careful description of the new review process and appreciate all the effort Signal puts into ensuring the app remains secure!

[jrose-signal]

Cherry-picked as 9ad2362, will be in the next release!