Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Displaying Deterministic Username for a given Token #1921

Merged
merged 23 commits into from
Jun 27, 2024

Conversation

Kaz-ookid
Copy link
Contributor

@Kaz-ookid Kaz-ookid commented Jun 12, 2024

PR Description

Context

Throughout the app, users were previously referred to by their PoP Token, which was not user-friendly. This PR implements a deterministic username system to improve the user experience. A detailed documentation of the project's idea and motivations is to be found in #1915. It is recommended to read it before diving into this PR.

Summary of Changes

This PR implements the deterministic username system across various parts of the app, ensuring a consistent and user-friendly display of usernames derived from PoP Tokens. The changes have been done in collaboration with FE2 (#1918) to ensure consistency throughout both systems.

General Changes

  • PublicKey class now includes the getUsername() function, returning the username version of the hash.
  • Added a mnemonic deterministic username generator to GeneralUtils. The usernames are a concatenation of two mnemonics words, and a 4 digits number. The number corresponds to the 4 first digits in a token's hash, starting from the left

Example:

token : d_xeXEsurEnyWOp04mrrMxC3m4cS-3jK_9_Aw-UYfww=
mnemonics words generated form the hash : spoon, issue
first 4 digits : 0434
final username : spoonissue0434

Token

  • TokenFragment.kt: When participating in a RollCall, you receive a PoP Token that you can see in the Token Tab. The username is now displayed above the hash of the token in the Token tab.

Social Media

  • ChirpListAdapter.kt: The chirps feed now displays the username of a user instead of the full PoP Token hash.

Roll Call

  • RollCallArrayAdapter.kt, RollCallFragment.kt: The list of attendees now displays the username above the Token hash. Structural changes were made in RollCallArrayAdapter to include two TextViews per item, and enhancing the class overall.

Digital Cash

  • Refactored the digital cash folder to work with PublicKey instead of String (token hashes).
  • Displayed the username in appropriate places, simplifying the sending workflow and transaction history readability.
  • For organizer transactions, the full public key hash is displayed with "(organizer)" appended, as the public key is longer than 32 bits and cannot be instantiated as PublicKey. Also, it juste made no sense to label this public key under a username anyway.
  • Fixed a bug in the transaction history where title and value were inverted ("From" label, and Transaction ID). see screenshots below.
  • Fixed a bug where the current user's token was not correctly removed from the selection Spinner when issuing as an organizer, causing a crash if it was selected.

Bugs and Documentation

  • Numerous bugs were discovered during implementation and have been well documented.

Screenshots

  • Token fragment displaying the username.
    username_token

  • Chirps feed displaying the username.
    image

  • Roll call displaying the username.
    username_rollcall

  • Digital cash sending activity displaying the username.
    image

  • Digital cash receipt activity displaying the username.
    username_receipt

  • Digital cash transaction history activity displaying the username and the organizer hash.
    username_transactions

  • Digital cash issuing activity.
    image

  • Bugged UI in transaction history showing the corrected title and value (now fixed).
    image

@Kaz-ookid Kaz-ookid self-assigned this Jun 12, 2024
# Conflicts:
#	fe2-android/app/src/main/java/com/github/dedis/popstellar/ui/lao/event/rollcall/RollCallArrayAdapter.kt
#	fe2-android/app/src/main/java/com/github/dedis/popstellar/ui/lao/event/rollcall/RollCallFragment.kt
#	fe2-android/app/src/main/res/values/strings.xml
…ername' into work-fe2-maxime-deterministic-username

# Conflicts:
#	fe2-android/app/src/main/java/com/github/dedis/popstellar/utility/Constants.kt
@Kaz-ookid Kaz-ookid marked this pull request as ready for review June 26, 2024 18:48
@Kaz-ookid Kaz-ookid requested a review from a team as a code owner June 26, 2024 18:48
Copy link
Contributor

@matteosz matteosz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! That's going to be a great feature :)

Just some comments

@matteosz
Copy link
Contributor

Also, now that I'm looking at the screenshots, perhaps it's better to have the username using underscores or capitalized letters so the 2 words are more easily recognizable
e.g. HomeSoldier0864, or home_soldier_0864, or any other fancy way of splitting the 2 words that you may prefer

@Kaz-ookid
Copy link
Contributor Author

Kaz-ookid commented Jun 26, 2024

Also, now that I'm looking at the screenshots, perhaps it's better to have the username using underscores or capitalized letters so the 2 words are more easily recognizable e.g. HomeSoldier0864, or home_soldier_0864, or any other fancy way of splitting the 2 words that you may prefer

I'll check with FE1 and modify before merging

edit : we decided to go for the capitalized version : HomeSoldier0864

- changed getUsername to getLabel (toUsername to label) in PublicKey
- refactored getPublicKeyFromUsername to avoid code duplication (now in PublicKey as static)
- removed unnecessary comments
- disabled serialization of label in PublicKey
- changed getUsername to getLabel (toUsername to label) in PublicKey
- refactored getPublicKeyFromUsername to avoid code duplication (now in PublicKey as static)
- removed unnecessary comments
- disabled serialization of label in PublicKey
…ername' into work-fe2-maxime-deterministic-username

# Conflicts:
#	fe2-android/app/src/main/java/com/github/dedis/popstellar/model/objects/security/PublicKey.kt
#	fe2-android/app/src/main/java/com/github/dedis/popstellar/ui/lao/digitalcash/DigitalCashIssueFragment.kt
#	fe2-android/app/src/main/java/com/github/dedis/popstellar/ui/lao/digitalcash/DigitalCashSendFragment.kt
Copy link

sonarcloud bot commented Jun 27, 2024

Copy link

sonarcloud bot commented Jun 27, 2024

Copy link

sonarcloud bot commented Jun 27, 2024

Copy link

sonarcloud bot commented Jun 27, 2024

@quadcopterman quadcopterman self-requested a review June 27, 2024 09:50
@Kaz-ookid Kaz-ookid added this pull request to the merge queue Jun 27, 2024
Merged via the queue into master with commit d2a394e Jun 27, 2024
17 checks passed
@Kaz-ookid Kaz-ookid deleted the work-fe2-maxime-deterministic-username branch June 27, 2024 11:07
Copy link
Contributor

@quadcopterman quadcopterman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, just one comment

Assert.assertEquals(digits, pk.getLabel().substring(pk.getLabel().length - USERNAME_DIGITS))
}

@Test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you could add one or two more tests then just the empty username for the generateUsername function

@quadcopterman
Copy link
Contributor

Oh too late, it was merged while I was reviewing it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants