Small fix for ConsensusContext #1413
Conversation
| @@ -88,8 +88,8 @@ public Block CreateBlock() | |||
| for (int i = 0, j = 0; i < Validators.Length && j < M; i++) | |||
| { | |||
| if (CommitPayloads[i]?.ConsensusMessage.ViewNumber != ViewNumber) continue; | |||
| sc.AddSignature(contract, Validators[i], CommitPayloads[i].GetDeserializedMessage<Commit>().Signature); | |||
| j++; | |||
| if (sc.AddSignature(contract, Validators[i], CommitPayloads[i].GetDeserializedMessage<Commit>().Signature)) | |||
There was a problem hiding this comment.
Under what kind of attack will the addition fail?
There was a problem hiding this comment.
No attack, just an error.
There was a problem hiding this comment.
I notice this error with a wrong blockchain imported, and a valid CN open (Related to #1414 )
There was a problem hiding this comment.
Imported?
Strange, Shargon, did it really solve the problem?
Because the import does not pass through here, it goes to OnPersist and it is another flow, right?
There was a problem hiding this comment.
This doesn't solve the problem of wrong import, only I notice that this variable is increased wrong if signature was not added
There was a problem hiding this comment.
I checked this before and I found it strange as well, because some commit could be sent with wrong signature.
But I think this will not be enough in such case, I believe we would need to clean Header and exit the Return Block, right?
| @@ -88,8 +88,8 @@ public Block CreateBlock() | |||
| for (int i = 0, j = 0; i < Validators.Length && j < M; i++) | |||
| { | |||
| if (CommitPayloads[i]?.ConsensusMessage.ViewNumber != ViewNumber) continue; | |||
| sc.AddSignature(contract, Validators[i], CommitPayloads[i].GetDeserializedMessage<Commit>().Signature); | |||
| j++; | |||
| if (sc.AddSignature(contract, Validators[i], CommitPayloads[i].GetDeserializedMessage<Commit>().Signature)) | |||
There was a problem hiding this comment.
It is impossible to return false, I think.
|
With #1415 I think that it's impossible to return false |
jmust be only increased if theAddSignaturewas true