Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Align getRandomSeed() implementation to protocol changes from LIP-46 #8570

Merged
merged 4 commits into from
Jun 14, 2023

Conversation

bobanm
Copy link
Contributor

@bobanm bobanm commented Jun 9, 2023

What was the problem?

This PR resolves #8154. It is related only to the second point about changes in PR related to getRandomBytes() and getRandomSeed(). The remaining points from this original issue have been tracked by the newly created, isolated issues, because they need to be merged into different branches.

How was it solved?

  • updated getRandomSeed() function, as per the LIP change
  • removed redundant import of lisk-cryptography package
  • updated unit tests for getRandomSeed()
  • removed redundant import of protocol spec fixture from the test suite
  • removed duplicated symlink to protocol spec fixture
  • updated protocol spec fixture generator with the new logic
  • re-generated protocol spec fixtures using the updated generator

How was it tested?

Run the updated test suite, and all tests are passing 👌🏻

@bobanm bobanm requested review from shuse2 and gkoumout June 9, 2023 08:19
@bobanm bobanm self-assigned this Jun 9, 2023
@codecov
Copy link

codecov bot commented Jun 9, 2023

Codecov Report

Merging #8570 (546106b) into release/6.0.0 (fe67ea1) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                @@
##           release/6.0.0    #8570      +/-   ##
=================================================
- Coverage          83.31%   83.31%   -0.01%     
=================================================
  Files                593      593              
  Lines              22300    22297       -3     
  Branches            3279     3278       -1     
=================================================
- Hits               18579    18576       -3     
  Misses              3721     3721              
Impacted Files Coverage Δ
framework/src/modules/random/utils.ts 96.42% <100.00%> (-0.19%) ⬇️

Copy link

@gkoumout gkoumout 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 left some small comments.

I am not able to judge the changes related to the lisk-cryptography package and files of folder generator_outputs/dpos_random_seed_generation, so I assume this will be reviewed within SDK.

framework/src/modules/random/utils.ts Outdated Show resolved Hide resolved
@bobanm bobanm requested a review from gkoumout June 13, 2023 02:28
@shuse2 shuse2 enabled auto-merge (squash) June 14, 2023 04:03
@shuse2 shuse2 merged commit c225b00 into release/6.0.0 Jun 14, 2023
@shuse2 shuse2 deleted the 8154-random-module-method branch June 14, 2023 06:56
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.

3 participants