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.

Sign up for GitHub

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

Jump to bottom

Calculate state when evaluating an action if the block state doesn't exist #645

Merged
merged 1 commit into from
Nov 4, 2019
Merged

Calculate state when evaluating an action if the block state doesn't exist #645

merged 1 commit into from
Nov 4, 2019

Conversation

earlbread
Copy link
Contributor

@earlbread earlbread commented Oct 31, 2019 •
edited
Loading

When there is no block state when evaluating an action, it is changed to calculate the state from the beginning to avoid IncompleteBlockStatesException.

@earlbread earlbread self-assigned this Oct 31, 2019
@codecov
Copy link

codecov bot commented Oct 31, 2019 •
edited
Loading

Codecov Report

Merging #645 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #645      +/-   ##
==========================================
+ Coverage   88.13%   88.13%   +<.01%     
==========================================
  Files         217      217              
  Lines       17198    17181      -17     
==========================================
- Hits        15157    15143      -14     
+ Misses       1165     1162       -3     
  Partials      876      876
Impacted Files Coverage Δ
Libplanet/Blockchain/BlockChain.cs 94.52% <100%> (+0.29%) ⬆️

moreal
moreal previously approved these changes Nov 1, 2019
View reviewed changes
limebell
limebell previously approved these changes Nov 1, 2019
View reviewed changes
@earlbread earlbread dismissed stale reviews from limebell and moreal via 954ff21 November 1, 2019 05:38
dahlia
dahlia reviewed Nov 1, 2019
View reviewed changes
Copy link
Contributor

@dahlia dahlia left a comment •
edited
Loading

Choose a reason for hiding this comment

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

As I know there's the reason why we want to calculate incomplete states. So how about manually turning on the completeStates option for the problematic case while maintaining the default value turned off, rather than making the completeStates option turned on by default?

@earlbread
Copy link
Contributor Author

As I know there's the reason why we want to calculate incomplete states. So how about manually turning on the completeStates option for the problematic case while maintaining the default value turned off, rather than making the completeStates option turned on by default?

To do that, we need to add a flag to EvaluateActions, ExecuteActions, and Append. I thought it is too complicated for the case.

longfin
longfin previously approved these changes Nov 1, 2019
View reviewed changes
longfin
longfin previously approved these changes Nov 1, 2019
View reviewed changes
longfin
longfin previously approved these changes Nov 1, 2019
View reviewed changes
limebell
limebell previously approved these changes Nov 1, 2019
View reviewed changes
@earlbread earlbread dismissed stale reviews from limebell and longfin via e4b2c4b November 4, 2019 05:07
dahlia
dahlia previously approved these changes Nov 4, 2019
View reviewed changes
longfin
longfin previously approved these changes Nov 4, 2019
View reviewed changes
@earlbread earlbread changed the title Turn on completeStates parameter of GetState method Calculate state when evaluating an action if the block state doesn't exist Nov 4, 2019
@earlbread earlbread dismissed stale reviews from longfin and dahlia via 152990a November 4, 2019 06:01
@earlbread
Copy link
Contributor Author

Changed to calculate the state from the beginning when there is no block state when evaluating an action instead of changing the default value of completeStates parameter.

dahlia
dahlia reviewed Nov 4, 2019
View reviewed changes
CHANGES.md Outdated
@@ -74,6 +74,8 @@ To be released.
not related with [Kademlia protocol][Kademlia]. [[#594], [#627]]
- `Swarm<T>` became not to check least recently used peer every time when
new peer is fetched. [[#627]]
- When there is no block state when evaluating an action, it is changed to
calculate the state from the beginning. [[#645]]
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 this?

IAction became guaranteed that the given IActionContext.PreviousStates.GetState() never throws IncompleteBlockStatesException. Instead, now it may calculate the incomplete states from the beginning if necessary.

@earlbread
Calculate block state when evaluating action
31266a5 
When there is no block state when evaluating an action, it is changed to calculate the state from the beginning.
@earlbread earlbread requested a review from dahlia November 4, 2019 06:28
@earlbread earlbread merged commit 0040d74 into planetarium:master Nov 4, 2019
@earlbread earlbread deleted the on-complete-state branch November 4, 2019 06:57
@earlbread earlbread mentioned this pull request Jan 2, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dahlia dahlia dahlia approved these changes

@longfin longfin longfin approved these changes

@limebell limebell limebell approved these changes

@moreal moreal moreal approved these changes

Assignees

@earlbread earlbread

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@earlbread @dahlia @longfin @limebell @moreal