-
Notifications
You must be signed in to change notification settings - Fork 534
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
Fix Register validator e2e test #1178
Fix Register validator e2e test #1178
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1178 +/- ##
========================================
Coverage 54.68% 54.68%
========================================
Files 176 176
Lines 23553 23547 -6
========================================
- Hits 12880 12877 -3
+ Misses 9645 9641 -4
- Partials 1028 1029 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
c022ea0
to
3a064f9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM good catch! 🎩
Good catch 🔥 P.s. not able to approve it, because I'm a creator of PR |
Description
This PR fixes register validator e2e test. It was failing due to 2 reasons, which are described below.
Misplaced
NextValidatorsHash
queryRegister validator e2e test is flaky, because we are querying for
ExtraData
andNextValidatorsHash
before we make sure that newly registered validator is joined to the consensus protocol.Proposed solution consists of querying block and check next validators hash only after we make sure that validator is present in new validator set.
Newly registered validator receives 0 for rewards
Another problem why this test was failing was because of new validator's rewards. The validator rewards were 0 even after the validator is registered, and jointed validator set and few epochs are passed. This happened because validator never signed any block, and it was not present in the uptime data. The cause why validator wasn't signing is because FSM calculation was never cancelled on receiving fresh blocks and FSM was working with the 'stale' data.
In other words, this occurred:
"get latest proposer not found - height: 16, round: 0, pc height: 15, pc round: 0"
syncerBlockCh
and cause sequence cancellation is following:The thing here is that syncer will call
WriteFullBlock
for the newly received block, and first it will write the block in the blockchain and then in the same fn callsb.dispatchEvent(evnt)
which results in generating an event about the inserted block. But since the block is already inserted,ev.NewChain[0].Number
andp.blockchain.CurrentHeader().Number
will be the same.Proposed solution:
The fix for this issue is also included in this PR and allows sequence cancellation also in the case when the syncer block height is equal to the current height from the blockchain.
The FSM at that moment might be calculating block for the lower or the same height, and it is ok to be interrupted since write from the consensus would not happen cause syncer already took the lock and wrote the block for that height. FSM calculation for the newer block then the saved one, can be interrupted only if e.g. node becomes the validator and the new sequence is called after the syncer wrote the block(with the current height+1), but before the event is handled. In that case the calculation for the new block will be interrupted but also new sequence will be started again.
Additional comments
E2E test is still going to fail occasionally (although more rarely than it is currently) with validators' hash mismatch until #1191 gets merged.
Changes include
Checklist
Testing