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

187-4 Add a consistency check for the tezos-specific fork detector #899

Merged
merged 11 commits into from
Sep 29, 2020

Conversation

ivanopagano
Copy link
Contributor

@ivanopagano ivanopagano commented Aug 19, 2020

Related to #42 and #187

The currently available detector works under the assumption that the search for the fork level will be fast enough for the node to remain "stable".
That means that if the node "jumps" to a different fork while the detector is executing the search, we might get in serious trouble.
For Tezos is sounds reasonable that this might occur, even if only by a remote chance.
To handle the situation a more robust detector is defined to use with tezos specifically (since it involved data that is block-specific), which does a double-check before committing to the identified fork level.

NOTE This is part of a long-running streak of PRs to define the overall fork handling strategy.
Before merging it we need to merge the previous PRs #896 #898 and rebase on the new master branch, after which the draft tag could be removed.

Copy link
Contributor

@piotrkosecki piotrkosecki 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!

@ivanopagano ivanopagano force-pushed the feature/187-3-fork-handling-simulation branch from 8ea45fd to f33e0bb Compare September 4, 2020 14:54
@ivanopagano ivanopagano force-pushed the feature/187-4-tezos-consistent-fork-detector branch from 83e56d1 to 96f73fd Compare September 4, 2020 14:55
@ivanopagano ivanopagano force-pushed the feature/187-3-fork-handling-simulation branch from f33e0bb to 6fc5578 Compare September 17, 2020 16:44
@vishakh vishakh changed the base branch from feature/187-3-fork-handling-simulation to master September 25, 2020 01:00
@vishakh vishakh changed the base branch from master to feature/187-3-fork-handling-simulation September 25, 2020 01:00
@ivanopagano ivanopagano force-pushed the feature/187-4-tezos-consistent-fork-detector branch from 681938b to ff8d4ee Compare September 25, 2020 07:28
* consistent results.
* It might happen that network calls to both services were not fast enough, to the
* point that the fork on which the reference node was at the time of detection changed
* during the algorthm execution. This might be a reasonable expectation in a highly
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: Algorithm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, and I additionally rephrased the "locally consistent" definition, trying to make it easier to understand.


/** Here we verify the properties of the [[ConsistentForkDetector]] implementation.
* This spec works essentially in the same way as
* the [[tech.cryptonomic.conseil.indexer.ForkDetectorSpec]]
Copy link
Contributor

Choose a reason for hiding this comment

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

The package name is incorrect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@ivanopagano ivanopagano force-pushed the feature/187-3-fork-handling-simulation branch from abe65cc to ce17921 Compare September 29, 2020 15:27
@ivanopagano ivanopagano force-pushed the feature/187-4-tezos-consistent-fork-detector branch from f1aaf4c to 72affdd Compare September 29, 2020 15:48
@ivanopagano ivanopagano changed the base branch from feature/187-3-fork-handling-simulation to master September 29, 2020 16:02
 * correct all database types for level as bigint
 * correct all code modeling levels to use the new alias
 * give an explicit class to BlockReference for better matching
 * simplify the node operator tests and add explanatory comments
 * add test case for loading blocks not indexed in db
 * add some log about random generation results during tests
 * fix an incorrect test
 * correct all database types for level as bigint
 * correct all code modeling levels to use the new alias
 * give an explicit class to BlockReference for better matching
 * simplify the node operator tests and add explanatory comments
 * add test case for loading blocks not indexed in db
 * add a checker to match the fork level block predecessor with the
   one stored by the indexer
 * define a property spec similar to the generic detector
@ivanopagano ivanopagano force-pushed the feature/187-4-tezos-consistent-fork-detector branch from 72affdd to ae1917b Compare September 29, 2020 16:14
@sonarcloud
Copy link

sonarcloud bot commented Sep 29, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ivanopagano ivanopagano marked this pull request as ready for review September 29, 2020 16:19
@vishakh vishakh merged commit 2dfa6e1 into master Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants