Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Encrypted identities #4028

Merged
merged 6 commits into from
May 11, 2016
Merged

Encrypted identities #4028

merged 6 commits into from
May 11, 2016

Conversation

chadwhitacre
Copy link
Contributor

@chadwhitacre chadwhitacre commented May 11, 2016

#3994

#3976
#4001

Punchlist

@chadwhitacre
Copy link
Contributor Author

Thinking through take_over ...

@chadwhitacre
Copy link
Contributor Author

Hmmm ... build failed but it looks like an old commit; restarting at Travis.

@chadwhitacre
Copy link
Contributor Author

Given that ~alice is taking over ~bob, the options are:

  1. ~alice gets ~bob's identities.
  2. ~alice gets ~bob's verified identities but not ~bob's unverified ones.
  3. We drop ~bob's identities.
  4. We block the take_over until ~bob drop's their own identities.

For cases (1) and (2) we'd have to prompt ~alice to choose if both ~bob and ~alice have an identity for a given country. For (3) we'd have to warn the user (~alice).

@chadwhitacre
Copy link
Contributor Author

@kaguillera suggests (1) with reverification for verified identities.

@chadwhitacre
Copy link
Contributor Author

Travis is still failing. The commit is c6b1dfe, a merge commit of 7f2666c into 1308454.

@chadwhitacre
Copy link
Contributor Author

Huh. During the initial rebase for this PR, we fixed these tests that are now failing. Hmmm ...

@chadwhitacre
Copy link
Contributor Author

Now they're failing again locally, investigating ...

@chadwhitacre
Copy link
Contributor Author

Ah, okay. There was a merge conflict in the first commit during the rebase, so the rebase stopped and in the cleanup process we happened to fix the relevant tests on that commit. However, subsequent commits that did not introduce rebase conflicts did introduce new, now-bad tests. We didn't catch those because we didn't test at every step. Re-rebasing ...

@chadwhitacre chadwhitacre force-pushed the encrypted-identities branch from 7f2666c to c64b3ba Compare May 11, 2016 19:59
@chadwhitacre
Copy link
Contributor Author

C'mon, Travis! No whammies!

@chadwhitacre
Copy link
Contributor Author

Yesssssssssss.

@chadwhitacre
Copy link
Contributor Author

Alright, back to take over (#4028 (comment)) ...

We're thinking that for today we should simply error out if the secondary (other) participant has an identity. We'll have to be smarter when we expand takes beyond Gratipay (and perhaps before), but for now this should be okay.

@chadwhitacre
Copy link
Contributor Author

Alright, naive implementation of take_over protection in df1f985. We can improve later. If anyone triggers this in the near future, they're messing around.

@chadwhitacre
Copy link
Contributor Author

Alright, we're down to it here. This is the one PR in the chain that I wish we could get more review on, since it's larger and is where we actually use the crypto we added in #3998.

@rohitpaulk @aandis Perhaps you could take a look at this one after the fact? I want to get takes online for tomorrow or I'd hold this up further. 😶

@chadwhitacre
Copy link
Contributor Author

Aaaaaannnnddd ..........

@chadwhitacre
Copy link
Contributor Author

"Ready?"
"Merge."

@chadwhitacre chadwhitacre merged commit 5033e20 into master May 11, 2016
@chadwhitacre chadwhitacre deleted the encrypted-identities branch December 2, 2016 21:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant