remove Debug from SessionRecord

[cosmicexplorer]

1. SessionRecord impls Debug, but it's not used anywhere. This removes that.
2. PrivateKey is given a Debug impl which avoids leaking any useful data or making elliptic curve computations.

[jrose-signal]

Hm. I feel weird about making these Debug because I feel like logging them can leak more information than you might intend. But I guess if that were really true then SessionRecord shouldn't be Debug in the first place, and it is, so okay.

[cosmicexplorer]

I feel like logging them can leak more information than you might intend

I think this is a very reasonable concern. The best argument in favor I can think of is that if we define a canonical Debug here (and we take precautions like not printing private key contents), then it's perhaps less likely anyone will later feel the need to define an ad-hoc debug printing method later which might not take those precautions?

But to take a step back, this change in particular was motivated by me wanting to be able to more easily debug tests for my own code, a CLI tool which depends on libsignal-protocol and e.g. serializes those in-memory stores to files on disk. That use case is definitely not what Signal is concerned with, so if your gut is feeling weird about adding this, I would recommend we close this, since it's not very difficult for me to add a debug formatting method for my own purposes.

But I guess if that were really true then SessionRecord shouldn't be Debug in the first place, and it is, so okay.

It actually looks like removing Debug from SessionRecord causes no compile issues, so after convincing myself with the above argument I've actually modified this change to simply remove Debug from it! I've kept the PrivateKey debug impl though, with a comment that calculating the public key equivalent isn't free.

[jrose-signal]

Hm, if we're trending that way then I'd say PrivateKey doesn't need it either, since you can always ask for the key_type. (I'm thinking about a user looking at generated API docs and seeing that PrivateKey implements Debug, but then trying it and realizing it doesn't tell them anything useful.) What do you think?

[cosmicexplorer]

(I'm thinking about a user looking at generated API docs and seeing that PrivateKey implements Debug, but then trying it and realizing it doesn't tell them anything useful.)

This is a very useful framing to evaluate these docs changes in, and I agree that it would be more confusing than helpful. I have removed the Debug impl for PrivateKey.

[jrose-signal]

Handling the cherry-pick now, but note that it's showing up as "unverified" in GitHub because I can't re-sign the commit with your key. I could remove you as a commit author, or you could turn off "vigilant mode", or we could leave it "unverified".

[cosmicexplorer]

Hm, that's a strange little quirk. I have turned off "vigilant mode" now, as it is rarely needed. Please let me know if that lets you cherry-pick it without any smudges.

[cosmicexplorer]

Ping! I think this should be good to merge? Please feel free to edit the commit (including removing my authorship) if that is needed to safely merge.

[jrose-signal]

Sorry, it's in the private branch and will be in the next release! But we haven't had anything to drive a release just yet. I'll close this when the release happens and I can put the cherry-picked commit hash here.

[cosmicexplorer]

Great, thanks!

[jrose-signal]

Released in v0.22.0!