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

Update confirmation heights with new async timing #1899

Merged
merged 4 commits into from Apr 11, 2019

Conversation

3 participants
@wezrule
Copy link
Collaborator

commented Apr 11, 2019

This was solved in #1877 (although have realized now it is not quite enough), but recent changes have escalated its necessity earlier.

gap_live had a timing issue with unchecked blocks which can be voted on and hence set the confirmation height to higher than was previously expected in the test. We now wait for blocks to be confirmed as well.

To make sure frontiers aren't checked during the test, a new bool is passed through to active_transactions which delays the start of confirming frontiers. This is necessary because active starts in it's own thread after the node is created, so can introduce a data race which is only possible in tests so didn't want to complicate core logic with mutex etc just for it.

Also core_test/rpc.cpp this should have been removed during a previous refactor.

@wezrule wezrule added this to the V19.0 milestone Apr 11, 2019

@wezrule wezrule requested a review from cryptocode Apr 11, 2019

@wezrule wezrule self-assigned this Apr 11, 2019

@wezrule wezrule added this to CP3 (2019-04-10) in V19 Apr 11, 2019

@cryptocode
Copy link
Collaborator

left a comment

LGTM

@wezrule wezrule merged commit 644c84f into nanocurrency:master Apr 11, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@zhyatt zhyatt added the enhancement label Apr 11, 2019

guilhermelawless added a commit to guilhermelawless/nano-node that referenced this pull request Apr 15, 2019

Update confirmation heights with new async timing (nanocurrency#1899)
* Fix timing issue in tests

* Increase deadline time

* Remove RPC tests from core_tests

* Make sure quorum is valid in confirmation_height.multiple test.

@wezrule wezrule deleted the wezrule:fix_conf_height_tests branch Apr 25, 2019

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