Skip to content

feat_: Update alias generated #6342

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

feat_: Update alias generated #6342

wants to merge 1 commit into from

Conversation

ulisesmac
Copy link
Contributor

This PR is required to acomplish

The context about this change is mainly in this Discord thread.

Summary

This PR changes the implementation of GenerateFromPublicKeyString to return a compressed key with ellipsis instead of the 3-words name.

Important changes:

  • Changed from the 3-words name to be the compressed key with an ellipsis
  • Update the shortened compressed key to be consistent with the alias

@status-im-auto
Copy link
Member

status-im-auto commented Feb 13, 2025

Jenkins Builds

Click to see older builds (74)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 91989c3 #1 2025-02-13 19:02:44 ~3 min macos 📦zip
✖️ 91989c3 #1 2025-02-13 19:03:56 ~4 min tests 📄log
✔️ 91989c3 #1 2025-02-13 19:04:37 ~5 min windows 📦zip
✔️ 91989c3 #1 2025-02-13 19:04:43 ~5 min ios 📦zip
✔️ 91989c3 #1 2025-02-13 19:05:01 ~5 min macos 📦zip
✔️ 91989c3 #1 2025-02-13 19:05:08 ~6 min linux 📦zip
✔️ 91989c3 #1 2025-02-13 19:05:36 ~6 min android 📦aar
✔️ 91989c3 #1 2025-02-13 19:11:14 ~12 min tests-rpc 📄log
✔️ efe7105 #2 2025-02-13 20:20:33 ~5 min macos 📦zip
✔️ efe7105 #2 2025-02-13 20:20:44 ~5 min ios 📦zip
✔️ efe7105 #2 2025-02-13 20:20:54 ~5 min macos 📦zip
✔️ efe7105 #2 2025-02-13 20:21:09 ~5 min windows 📦zip
✔️ efe7105 #2 2025-02-13 20:21:30 ~6 min android 📦aar
✔️ efe7105 #2 2025-02-13 20:21:31 ~6 min linux 📦zip
✖️ efe7105 #2 2025-02-13 20:28:25 ~12 min tests-rpc 📄log
✔️ efe7105 #2 2025-02-13 20:46:43 ~31 min tests 📄log
✖️ 3635971 #3 2025-02-17 15:18:48 ~1 min tests-rpc 📄log
✔️ 3635971 #3 2025-02-17 15:21:03 ~4 min windows 📦zip
✔️ 3635971 #3 2025-02-17 15:21:19 ~4 min macos 📦zip
✔️ 3635971 #3 2025-02-17 15:21:38 ~4 min ios 📦zip
✔️ 3635971 #3 2025-02-17 15:22:03 ~5 min macos 📦zip
✔️ 3635971 #3 2025-02-17 15:22:55 ~6 min linux 📦zip
✔️ 3635971 #3 2025-02-17 15:23:47 ~6 min android 📦aar
✔️ 3635971 #3 2025-02-17 15:47:29 ~30 min tests 📄log
✖️ 07005bb #4 2025-02-17 15:22:35 ~1 min tests-rpc 📄log
✔️ 07005bb #4 2025-02-17 15:24:58 ~3 min windows 📦zip
✔️ 07005bb #4 2025-02-17 15:25:36 ~4 min macos 📦zip
✔️ 07005bb #4 2025-02-17 15:26:40 ~4 min ios 📦zip
✔️ 07005bb #4 2025-02-17 15:27:16 ~5 min macos 📦zip
✔️ 07005bb #4 2025-02-17 15:28:08 ~5 min linux 📦zip
✔️ 07005bb #4 2025-02-17 15:29:53 ~6 min android 📦aar
✔️ 07005bb #4 2025-02-17 16:17:24 ~29 min tests 📄log
✖️ d912c7d #5 2025-02-18 16:46:04 ~1 min tests-rpc 📄log
✔️ d912c7d #5 2025-02-18 16:48:47 ~4 min macos 📦zip
✔️ d912c7d #5 2025-02-18 16:49:10 ~4 min ios 📦zip
✔️ d912c7d #5 2025-02-18 16:49:39 ~5 min macos 📦zip
✔️ d912c7d #5 2025-02-18 16:49:46 ~5 min windows 📦zip
✔️ d912c7d #5 2025-02-18 16:50:17 ~5 min linux 📦zip
✔️ d912c7d #5 2025-02-18 16:50:27 ~6 min android 📦aar
✔️ d912c7d #5 2025-02-18 17:15:36 ~31 min tests 📄log
✔️ 4a938d1 #6 2025-02-18 16:52:14 ~2 min ios 📦zip
✔️ 4a938d1 #6 2025-02-18 16:53:14 ~4 min macos 📦zip
✔️ 4a938d1 #6 2025-02-18 16:53:27 ~2 min android 📦aar
✔️ 4a938d1 #6 2025-02-18 16:53:37 ~3 min windows 📦zip
✔️ 4a938d1 #6 2025-02-18 16:54:57 ~5 min macos 📦zip
✔️ 4a938d1 #6 2025-02-18 16:55:50 ~5 min linux 📦zip
✔️ 4a938d1 #6 2025-02-18 16:59:21 ~12 min tests-rpc 📄log
✔️ 4a938d1 #6 2025-02-18 17:46:57 ~31 min tests 📄log
✔️ 1283819 #7 2025-02-21 14:24:30 ~3 min android 📦aar
✔️ 1283819 #7 2025-02-21 14:25:24 ~4 min ios 📦zip
✔️ 1283819 #7 2025-02-21 14:25:35 ~4 min windows 📦zip
✖️ 1283819 #7 2025-02-21 14:26:13 ~4 min tests 📄log
✔️ 1283819 #7 2025-02-21 14:26:28 ~5 min macos 📦zip
✔️ 1283819 #7 2025-02-21 14:26:41 ~5 min macos 📦zip
✔️ 1283819 #7 2025-02-21 14:27:23 ~6 min linux 📦zip
✔️ 1283819 #7 2025-02-21 14:33:53 ~12 min tests-rpc 📄log
✔️ c1812f9 #8 2025-02-21 14:54:08 ~2 min android 📦aar
✔️ c1812f9 #8 2025-02-21 14:54:34 ~3 min ios 📦zip
✖️ c1812f9 #8 2025-02-21 14:55:38 ~4 min tests 📄log
✔️ c1812f9 #8 2025-02-21 14:56:38 ~5 min linux 📦zip
✔️ c1812f9 #8 2025-02-21 14:56:43 ~5 min macos 📦zip
✔️ c1812f9 #8 2025-02-21 14:56:53 ~5 min windows 📦zip
✔️ c1812f9 #8 2025-02-21 14:57:43 ~6 min macos 📦zip
✔️ c1812f9 #8 2025-02-21 15:04:07 ~12 min tests-rpc 📄log
✔️ bb9ff68 #9 2025-02-24 15:12:23 ~3 min android 📦aar
✔️ bb9ff68 #9 2025-02-24 15:13:19 ~4 min windows 📦zip
✔️ bb9ff68 #9 2025-02-24 15:15:17 ~6 min linux 📦zip
✔️ bb9ff68 #9 2025-02-24 15:15:34 ~6 min macos 📦zip
✔️ bb9ff68 #9 2025-02-24 15:21:05 ~12 min macos 📦zip
✔️ bb9ff68 #9 2025-02-24 15:23:20 ~14 min tests-rpc 📄log
bb9ff68 #9 2025-02-24 15:24:50 ~15 min ios 📄log
✖️ bb9ff68 #9 2025-02-24 15:40:51 ~31 min tests 📄log
✖️ bb9ff68 #10 2025-02-24 16:38:03 ~27 min tests 📄log
✖️ bb9ff68 #11 2025-02-24 18:32:59 ~29 min tests 📄log
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ eac4403 #10 2025-02-26 17:59:55 ~3 min android 📦aar
✔️ eac4403 #10 2025-02-26 18:00:22 ~3 min ios 📦zip
✔️ eac4403 #10 2025-02-26 18:01:58 ~5 min macos 📦zip
✔️ eac4403 #10 2025-02-26 18:02:11 ~5 min windows 📦zip
✔️ eac4403 #10 2025-02-26 18:02:41 ~6 min linux 📦zip
✔️ eac4403 #10 2025-02-26 18:03:19 ~6 min macos 📦zip
✔️ eac4403 #10 2025-02-26 18:10:37 ~13 min tests-rpc 📄log
✖️ eac4403 #12 2025-02-26 18:27:31 ~30 min tests 📄log
✔️ 0f966a0 #11 2025-03-03 18:48:38 ~3 min android 📦aar
✔️ 0f966a0 #11 2025-03-03 18:49:49 ~4 min windows 📦zip
✔️ 0f966a0 #11 2025-03-03 18:50:58 ~5 min linux 📦zip
✔️ 0f966a0 #11 2025-03-03 18:51:33 ~6 min ios 📦zip
✔️ 0f966a0 #11 2025-03-03 18:51:49 ~6 min macos 📦zip
✔️ 0f966a0 #11 2025-03-03 18:52:25 ~6 min macos 📦zip
✔️ 0f966a0 #11 2025-03-03 19:02:26 ~16 min tests-rpc 📄log
✔️ 0f966a0 #13 2025-03-03 19:18:37 ~33 min tests 📄log

Copy link

codecov bot commented Feb 13, 2025

Codecov Report

Attention: Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.

Project coverage is 61.12%. Comparing base (6219faa) to head (0f966a0).

Files with missing lines Patch % Lines
protocol/identity/alias/generate.go 80.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6342      +/-   ##
===========================================
+ Coverage    61.09%   61.12%   +0.02%     
===========================================
  Files          867      867              
  Lines       112673   112668       -5     
===========================================
+ Hits         68843    68867      +24     
+ Misses       35851    35846       -5     
+ Partials      7979     7955      -24     
Flag Coverage Δ
functional 22.46% <63.63%> (-0.03%) ⬇️
unit 59.47% <81.81%> (+0.22%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
protocol/contact.go 80.85% <100.00%> (-0.30%) ⬇️
protocol/identity/alias/generate.go 83.33% <80.00%> (-6.67%) ⬇️

... and 39 files with indirect coverage changes

@ulisesmac ulisesmac requested review from ilmotta and qfrank February 14, 2025 14:14
@ulisesmac
Copy link
Contributor Author

@igor-sirotin @ilmotta The comments have been addressed, could you please review again?

Thank you for the feedback!

return abbreviatedKey
}
return ""
func getShortenedCompressedKey(compressedKey string) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that we can delete this function, which has only one usage and directly use alias.ShortenedCompressedKey(compressedKey)

@ulisesmac ulisesmac force-pushed the fix-display-name branch 2 times, most recently from 1283819 to c1812f9 Compare February 21, 2025 14:51
@ulisesmac
Copy link
Contributor Author

@igor-sirotin comments addressed, could you review again please?

Copy link
Collaborator

@igor-sirotin igor-sirotin left a comment

Choose a reason for hiding this comment

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

Thanks, looks good 👍

@ulisesmac ulisesmac force-pushed the fix-display-name branch 2 times, most recently from bb9ff68 to eac4403 Compare February 26, 2025 17:56
@igor-sirotin
Copy link
Collaborator

@ulisesmac seems that TestPairPendingContactRequest is breaking for real in your PR 🙁

@ulisesmac
Copy link
Contributor Author

@igor-sirotin Thank you, I'll check the tests and solve it.

- Changed from the 3-words name to be the compressed key with an ellipsis
- Update the shortened compressed key to be consistent with the alias
- Add more tests about the public key length received
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants