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

go/scheduler: remove debug static validators option #4222

Merged
merged 2 commits into from
Aug 30, 2021

Conversation

ptrus
Copy link
Member

@ptrus ptrus commented Aug 27, 2021

Fixes: #4215

@@ -1210,24 +1202,4 @@ WaitLoop:
case <-time.After(recvTimeout):
t.Fatalf("failed to receive roothash block")
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Cannot really test freezing with a single validator. This should either be tested in an e2e test with multiple validators or if we added an option for freezing to: "freeze the node, but allow manually unfreezing in the first epoch already".

Will open a follow up issue for this.

@ptrus ptrus force-pushed the ptrus/feature/remove-debug-static-validators branch 2 times, most recently from 20360ad to dc1dfd6 Compare August 27, 2021 10:15
@codecov
Copy link

codecov bot commented Aug 27, 2021

Codecov Report

Merging #4222 (f95ac4d) into master (ff20d6b) will decrease coverage by 0.49%.
The diff coverage is 90.74%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4222      +/-   ##
==========================================
- Coverage   69.16%   68.67%   -0.50%     
==========================================
  Files         411      411              
  Lines       47514    47549      +35     
==========================================
- Hits        32861    32652     -209     
- Misses      10596    10848     +252     
+ Partials     4057     4049       -8     
Impacted Files Coverage Δ
...consensus/tendermint/apps/governance/governance.go 61.51% <ø> (ø)
go/consensus/tendermint/apps/scheduler/genesis.go 35.08% <ø> (+1.26%) ⬆️
...o/consensus/tendermint/apps/scheduler/scheduler.go 74.40% <0.00%> (+0.64%) ⬆️
go/oasis-node/cmd/genesis/genesis.go 55.14% <ø> (-0.28%) ⬇️
go/storage/tests/tester.go 98.76% <ø> (-0.01%) ⬇️
go/consensus/tendermint/apps/scheduler/query.go 75.00% <50.00%> (ø)
go/oasis-node/cmd/node/node.go 55.79% <66.66%> (-0.71%) ⬇️
go/beacon/tests/tester.go 86.36% <85.18%> (-0.94%) ⬇️
go/consensus/tendermint/tests/genesis/genesis.go 92.48% <88.88%> (-5.14%) ⬇️
go/consensus/tendermint/tests/evidence.go 93.10% <100.00%> (+0.12%) ⬆️
... and 61 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff20d6b...f95ac4d. Read the comment docs.

@ptrus ptrus marked this pull request as ready for review August 27, 2021 11:01
@ptrus ptrus force-pushed the ptrus/feature/remove-debug-static-validators branch from dc1dfd6 to 07537ee Compare August 27, 2021 11:02
Copy link
Member

@kostko kostko left a comment

Choose a reason for hiding this comment

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

I assume that since this option was omitempty, this shouldn't affect the chain context? Would be great to test that this works on testnet just to be sure.

go/oasis-node/cmd/node/node.go Outdated Show resolved Hide resolved
go/registry/tests/tester.go Outdated Show resolved Hide resolved
go/registry/tests/tester.go Outdated Show resolved Hide resolved
go/scheduler/tests/tester.go Outdated Show resolved Hide resolved
@ptrus ptrus force-pushed the ptrus/feature/remove-debug-static-validators branch from 07537ee to f95ac4d Compare August 27, 2021 12:42
@ptrus
Copy link
Member Author

ptrus commented Aug 27, 2021

I assume that since this option was omitempty, this shouldn't affect the chain context? Would be great to test that this works on testnet just to be sure.

Yeah i think this should work, but i'll backport this and test on a testnet node.

@ptrus
Copy link
Member Author

ptrus commented Aug 27, 2021

Started a fresh node built on this branch on testnet and is syncing 👍 - so the chain context stayed the same.

./oasis-node --version
Software version: 21.2-gitf95ac4de8
...

@ptrus ptrus requested a review from kostko August 27, 2021 14:25
@ptrus ptrus merged commit a23b778 into master Aug 30, 2021
@ptrus ptrus deleted the ptrus/feature/remove-debug-static-validators branch August 30, 2021 05:56
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.

Flaky go unit tests (consensus failure due to no registered validators)
2 participants