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

Hash signature before feeding as seed #519

Merged
merged 1 commit into from Sep 19, 2019

Conversation

@longfin
Copy link
Member

commented Sep 18, 2019

This PR fixes a bug that IActionContext.Random had generated the same result in different IAction.Execute() against the same Block<T>.

@longfin longfin force-pushed the longfin:bugfix/random-seed branch 3 times, most recently from e3b4194 to 6fa5b63 Sep 18, 2019
@longfin longfin marked this pull request as ready for review Sep 18, 2019
@longfin longfin requested a review from libplanet Sep 18, 2019
@codecov

This comment has been minimized.

Copy link

commented Sep 18, 2019

Codecov Report

Merging #519 into master will decrease coverage by 0.08%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #519      +/-   ##
==========================================
- Coverage    90.2%   90.12%   -0.09%     
==========================================
  Files         201      201              
  Lines       14908    14845      -63     
==========================================
- Hits        13448    13379      -69     
+ Misses       1183     1180       -3     
- Partials      277      286       +9
Impacted Files Coverage Δ
Libplanet/Action/ActionEvaluation.cs 100% <100%> (ø) ⬆️
Libplanet/Blocks/Block.cs 86.59% <0%> (-4.06%) ⬇️
Libplanet.Tests/Action/ActionContextTest.cs 89.7% <0%> (-0.78%) ⬇️
Libplanet/Net/Swarm.cs 81.66% <0%> (-0.14%) ⬇️
Libplanet.Tests/Blocks/BlockTest.cs 98.06% <0%> (-0.12%) ⬇️
Libplanet/Action/RandomExtension.cs 100% <0%> (ø) ⬆️
Libplanet/Net/Protocols/KademliaProtocol.cs 60.86% <0%> (+0.28%) ⬆️
Libplanet/Tx/Transaction.cs 95.75% <0%> (+1.15%) ⬆️
Copy link
Member

left a comment

It would be good to have a test case for this case.

CHANGES.md Outdated Show resolved Hide resolved
@longfin longfin self-assigned this Sep 19, 2019
@longfin longfin added the bug label Sep 19, 2019
Copy link
Member

left a comment

This needs to be rebased on the current master.

CHANGES.md Outdated Show resolved Hide resolved
@longfin longfin force-pushed the longfin:bugfix/random-seed branch from 616c7d1 to 0d12def Sep 19, 2019
@longfin longfin requested review from dahlia and earlbread Sep 19, 2019
@dahlia
dahlia approved these changes Sep 19, 2019
@moreal
moreal approved these changes Sep 19, 2019
@longfin longfin merged commit 2e02a7a into planetarium:master Sep 19, 2019
15 checks passed
15 checks passed
WIP Ready for review
Details
changelog This contains self-describing changelog.
Details
docs Libplanet docs generated by DocFX
Details
license/cla Contributor License Agreement is signed.
Details
planetarium.libplanet Build #20190919.8 succeeded
Details
planetarium.libplanet (Linux_Mono) Linux_Mono succeeded
Details
planetarium.libplanet (Linux_NETCore) Linux_NETCore succeeded
Details
planetarium.libplanet (Windows_Mono) Windows_Mono succeeded
Details
planetarium.libplanet (Windows_NETCore) Windows_NETCore succeeded
Details
planetarium.libplanet (Windows_NETCore_Benchmark) Windows_NETCore_Benchmark succeeded
Details
planetarium.libplanet (Windows_NETCore_coverage) Windows_NETCore_coverage succeeded
Details
planetarium.libplanet (Windows_NETFramework) Windows_NETFramework succeeded
Details
planetarium.libplanet (macOS_Mono) macOS_Mono succeeded
Details
planetarium.libplanet (macOS_NETCore) macOS_NETCore succeeded
Details
planetarium.libplanet (macOS_Unity) macOS_Unity succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.