Skip to content

Random values for generateLocalAlias()#2337

Merged
pm47 merged 4 commits into
masterfrom
local-alias-gen-nodupcheck
Jun 29, 2022
Merged

Random values for generateLocalAlias()#2337
pm47 merged 4 commits into
masterfrom
local-alias-gen-nodupcheck

Conversation

@pm47
Copy link
Copy Markdown
Member

@pm47 pm47 commented Jun 28, 2022

This is an alternative to #2316.

Turns out the value space is large enough to not have to worry about duplicates.

Turns out the value space is large enough to not have to worry about
duplicates.
@pm47 pm47 requested a review from t-bast June 28, 2022 16:18
t-bast
t-bast previously approved these changes Jun 28, 2022
Copy link
Copy Markdown
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

It would be nice to also detect when we have a collision and log an error (which we can do for example in the register). This way if we completely messed up our maths or have an issue with our random generator, we will detect it?

Comment thread eclair-core/src/main/scala/fr/acinq/eclair/ShortChannelId.scala Outdated
@pm47
Copy link
Copy Markdown
Member Author

pm47 commented Jun 29, 2022

It would be nice to also detect when we have a collision and log an error (which we can do for example in the register). This way if we completely messed up our maths or have an issue with our random generator, we will detect it?

Done in 3ff4831.

Copy link
Copy Markdown
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

LGTM 💯

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 29, 2022

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 84.72%. Comparing base (af79f44) to head (88cbc77).
⚠️ Report is 634 commits behind head on master.

Files with missing lines Patch % Lines
.../main/scala/fr/acinq/eclair/channel/Register.scala 75.00% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2337      +/-   ##
==========================================
+ Coverage   84.71%   84.72%   +0.01%     
==========================================
  Files         194      194              
  Lines       14632    14638       +6     
  Branches      621      627       +6     
==========================================
+ Hits        12395    12402       +7     
+ Misses       2237     2236       -1     
Files with missing lines Coverage Δ
...rc/main/scala/fr/acinq/eclair/ShortChannelId.scala 92.59% <100.00%> (+0.59%) ⬆️
.../main/scala/fr/acinq/eclair/channel/Register.scala 87.87% <75.00%> (-1.78%) ⬇️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pm47 pm47 merged commit 08d2ad4 into master Jun 29, 2022
@pm47 pm47 deleted the local-alias-gen-nodupcheck branch June 29, 2022 11:22
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.

3 participants