Skip to content

Fix #1834#1838

Merged
shargon merged 4 commits intoneo-project:masterfrom
shargon:fix-1834
Aug 13, 2020
Merged

Fix #1834#1838
shargon merged 4 commits intoneo-project:masterfrom
shargon:fix-1834

Conversation

@shargon
Copy link
Member

@shargon shargon commented Aug 12, 2020

We need to ensure that all the indexes can fit in LastSeenMessage, if we use a integer type, we have the half available chain.

Also, if heigh it's 0, LastSeenMessage?.Count(p => Block.Index > 0 && p.Value < (Block.Index - 1)) ?? 0; it will produce an integer overflow, and the condition always will be true.

@shargon shargon requested a review from erikzhang August 12, 2020 08:59
@erikzhang
Copy link
Member

uint is better?

@shargon
Copy link
Member Author

shargon commented Aug 12, 2020

Yes, it's better, but the consensus UT use -1, we can fix the UT and change to uint

@shargon
Copy link
Member Author

shargon commented Aug 12, 2020

The problem is that ConsensusService use Blockchain.Singleton and it has 1 Height. So it can't meet this UT

mockContext.Object.ChangeViewPayloads[mockContext.Object.MyIndex] = null;
Console.WriteLine("Forcing Failed nodes for recovery request... ");
mockContext.Object.CountFailed.Should().Be(0);
mockContext.Object.LastSeenMessage.Clear();
foreach (var validator in mockContext.Object.Validators)
{
mockContext.Object.LastSeenMessage[validator] = -1;
}
mockContext.Object.CountFailed.Should().Be(7);
Console.WriteLine("\nWaiting for recovery due to failed nodes... ");
var backupOnRecoveryDueToFailedNodes = subscriber.ExpectMsg<LocalNode.SendDirectly>();
var recoveryPayload = (ConsensusPayload)backupOnRecoveryDueToFailedNodes.Inventory;

@shargon
Copy link
Member Author

shargon commented Aug 12, 2020

I think that now it's better, because if LastSeen doesn't contain the pubKey, it will be counted as failed.

erikzhang
erikzhang previously approved these changes Aug 13, 2020
@erikzhang
Copy link
Member

Test need to be fixed.

@shargon shargon merged commit e6d68dc into neo-project:master Aug 13, 2020
@shargon shargon deleted the fix-1834 branch August 13, 2020 11:20
cloud8little pushed a commit to cloud8little/neo that referenced this pull request Jan 24, 2021
* Fix 1834

* Fix UT

* Remove useless condition

Co-authored-by: Erik Zhang <erik@neo.org>
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.

2 participants