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

Sanity check in PrivateKey constructor using bytes #438

Merged
merged 6 commits into from Aug 15, 2019

Conversation

@ipdae
Copy link
Member

commented Aug 15, 2019

resolved #358

@ipdae ipdae force-pushed the ipdae:issues-358 branch from ef9b0e4 to 993ad31 Aug 15, 2019

CHANGES.md Outdated Show resolved Hide resolved

@earlbread earlbread requested review from dahlia and longfin Aug 15, 2019

@codecov

This comment has been minimized.

Copy link

commented Aug 15, 2019

Codecov Report

Merging #438 into master will decrease coverage by 0.02%.
The diff coverage is 94.44%.

@@            Coverage Diff             @@
##           master     #438      +/-   ##
==========================================
- Coverage   87.94%   87.92%   -0.03%     
==========================================
  Files         198      198              
  Lines       14426    14439      +13     
==========================================
+ Hits        12687    12695       +8     
- Misses       1425     1429       +4     
- Partials      314      315       +1
Impacted Files Coverage Δ
Libplanet/Crypto/PrivateKey.cs 86.2% <100%> (+0.62%) ⬆️
Libplanet.Tests/Crypto/PrivateKeyTest.cs 98.94% <87.5%> (-0.51%) ⬇️
Libplanet/Net/Swarm.cs 77.74% <0%> (-0.27%) ⬇️
Update CHANGES.md
Co-Authored-By: Seunghun Lee <waydi1@gmail.com>
CHANGES.md Outdated
@@ -82,6 +82,7 @@ To be released.
the `Transaction<T>` more than once. [[#413]]
- `BlockChain<T>.Swap()` became to omit common block finding when `render` is
`false`. [[#423]]
- `PrivateKey(bytes)` became to valid check. [[#438]]

This comment has been minimized.

Copy link
@dahlia

dahlia Aug 15, 2019

Member
Suggested change
- `PrivateKey(bytes)` became to valid check. [[#438]]
- `PrivateKey(byte[])` constructor became to check validity. [[#438]]
}
catch (ArgumentException)
{
throw new InvalidOperationException(

This comment has been minimized.

Copy link
@dahlia

dahlia Aug 15, 2019

Member

I believe ArgumentException would be more suitable here.

Suggested change
throw new InvalidOperationException(
throw new ArgumentException(
);

var key = new PrivateKey(param);
var publicKey = key.PublicKey;

This comment has been minimized.

Copy link
@longfin

longfin Aug 15, 2019

Member

This statement also seems unnatural, so it would be better if there is some comment about it.

);

var key = new PrivateKey(param);
var publicKey = key.PublicKey;

This comment has been minimized.

Copy link
@longfin

longfin Aug 15, 2019

Member
Suggested change
var publicKey = key.PublicKey;
var _ = key.PublicKey;

@ipdae ipdae dismissed stale reviews from earlbread and dahlia via 9814a2c Aug 15, 2019

@ipdae ipdae force-pushed the ipdae:issues-358 branch from 9814a2c to a45dc18 Aug 15, 2019

@ipdae ipdae force-pushed the ipdae:issues-358 branch from a45dc18 to b9e28fc Aug 15, 2019

@dahlia
dahlia approved these changes Aug 15, 2019

@dahlia dahlia added the bug label Aug 15, 2019

@dahlia dahlia added this to In progress in PyCon Korea 2019 via automation Aug 15, 2019

@dahlia dahlia added this to the 0.5.0 milestone Aug 15, 2019

@dahlia dahlia merged commit dabcaf6 into planetarium:master Aug 15, 2019

17 checks passed

WIP Ready for review
Details
changelog This contains self-describing changelog.
Details
codecov/patch 94.44% of diff hit (target 87.94%)
Details
codecov/project Absolute coverage decreased by -0.02% but relative coverage increased by +6.49% compared to b41da29
Details
docs Libplanet docs generated by DocFX
Details
license/cla Contributor License Agreement is signed.
Details
planetarium.libplanet Build #20190815.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

@longfin longfin moved this from In progress to Done in PyCon Korea 2019 Aug 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4 participants
You can’t perform that action at this time.