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

Do not append genesis during fork #1063

Merged

Conversation

limebell
Copy link
Member

@limebell limebell commented Oct 30, 2020

Instead IStore.ForkBlockIndexes() became to iterate from 0 (was 1).

Instead just append its index only.

renderActions: !inFork,
evaluateActions: !inFork
);
if (!inFork)
Copy link
Contributor

Choose a reason for hiding this comment

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

How merge if statements?

Suggested change
if (!inFork)
if (count == 0 && !inFork)

Copy link
Member Author

@limebell limebell Oct 30, 2020

Choose a reason for hiding this comment

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

It will throw exception in next else if statement 😢

dahlia
dahlia previously approved these changes Oct 30, 2020
Comment on lines 175 to 177
if (Count == 0)
{
Append(
genesisBlock,
currentTime: genesisBlock.Timestamp,
renderBlocks: !inFork,
renderActions: !inFork,
evaluateActions: !inFork
);
if (!inFork)
Copy link
Contributor

Choose a reason for hiding this comment

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

How about merging two conditions into one using an && operator?

Copy link
Contributor

@dahlia dahlia Oct 30, 2020

Choose a reason for hiding this comment

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

Oh, fair enough. Never mind.

Copy link
Member Author

Choose a reason for hiding this comment

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

It will throw exception in next else if statement 😢

@limebell
Copy link
Member Author

limebell commented Nov 2, 2020

I made constructor of BlockChain<T> instance to not append genesis block every time during fork. Please review again! @libplanet

@limebell limebell requested review from dahlia and earlbread November 2, 2020 06:10
@limebell limebell force-pushed the refactor/skip-genesis-infork branch from e5ca06e to 534d8a5 Compare November 2, 2020 06:15
@limebell limebell requested a review from longfin November 2, 2020 06:16
@codecov
Copy link

codecov bot commented Nov 2, 2020

Codecov Report

Merging #1063 into 9c-main will decrease coverage by 0.06%.
The diff coverage is 40.00%.

@@             Coverage Diff             @@
##           9c-main    #1063      +/-   ##
===========================================
- Coverage    88.44%   88.37%   -0.07%     
===========================================
  Files          343      343              
  Lines        31490    31494       +4     
===========================================
- Hits         27850    27834      -16     
- Misses        1962     1980      +18     
- Partials      1678     1680       +2     
Impacted Files Coverage Δ
Libplanet/Blockchain/BlockChain.cs 90.04% <40.00%> (-0.24%) ⬇️
Libplanet/Crypto/PrivateKey.cs 83.80% <0.00%> (-0.96%) ⬇️
Libplanet/Store/DefaultStore.cs 85.75% <0.00%> (-0.95%) ⬇️
Libplanet/Net/BlockCompletion.cs 94.18% <0.00%> (-0.88%) ⬇️
Libplanet/Net/Protocols/KademliaProtocol.cs 79.36% <0.00%> (-0.43%) ⬇️
Libplanet/Net/Swarm.cs 84.33% <0.00%> (-0.38%) ⬇️

@limebell limebell merged commit 2f7863f into planetarium:9c-main Nov 2, 2020
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.

4 participants