added checks on proofs parameters like epoch, nonce and shard #6698
Conversation
sstanculeanu
added
the
ignore-for-release-notes
process/block/baseProcess.go
Outdated
| return fmt.Errorf("%w while getting header for proof hash %s", err, hex.EncodeToString(prevProof.GetHeaderHash())) | ||
| } | ||
|
|
||
| if prevProof.GetHeaderNonce() != prevHeader.GetNonce() { |
There was a problem hiding this comment.
we have now also round in header proof, so we can also add a check for round
| return fmt.Errorf("%w while getting header for proof hash %s", err, hex.EncodeToString(iep.proof.GetHeaderHash())) | ||
| } | ||
|
|
||
| if iep.proof.GetHeaderNonce() != header.GetNonce() { |
process/block/baseProcess.go
Outdated
| return bp.checkPrevProofValidity(headerHandler) | ||
| } | ||
|
|
||
| func (bp *baseProcessor) checkPrevProofValidity(headerHandler data.HeaderHandler) error { |
There was a problem hiding this comment.
I think the checks can be done against the currentBlockHeader fields which is already loaded, like in the checkBlockValidity method.
currentBlockHeader can be passed as prevHeader
There was a problem hiding this comment.
also, do we have this kind of check as well for the proofs in the shard data included in a metablock?
e.g the prevProof from a shardData needs to be verified that it has the correct data (epoch, round, nonce) compared to the prevBlock of that shardData
There was a problem hiding this comment.
extended the checks over the prevProof as suggested
process/block/baseProcess.go
Outdated
| } | ||
|
|
||
| func (bp *baseProcessor) checkPrevProofValidity(headerHandler data.HeaderHandler) error { | ||
| if !bp.enableEpochsHandler.IsFlagEnabledInEpoch(common.EquivalentMessagesFlag, headerHandler.GetEpoch()) { |
There was a problem hiding this comment.
can you check with this instead:
if !common.ShouldBlockHavePrevProof(headerHandler, enableEpochsHandler, common.EquivalentMessagesFlag) {
return nil
}the above check takes into consideration also the first block which should not have a previous proof.
| if !mp.proofsPool.HasProof(shardData.ShardID, shardData.HeaderHash) { | ||
| return fmt.Errorf("%w for header hash %s", process.ErrMissingHeaderProof, hex.EncodeToString(shardData.HeaderHash)) | ||
| } | ||
|
|
||
| prevProof := shardData.GetPreviousProof() | ||
| headersPool := mp.dataPool.Headers() | ||
| prevHeader, err := headersPool.GetHeaderByHash(prevProof.GetHeaderHash()) |
There was a problem hiding this comment.
here we check if there is shardData.HeaderHash proof in pool, and then we check proof fields on previousProof, is this ok?
There was a problem hiding this comment.
yes, the proof from pool should have already been validated
Reasoning behind the pull request
Proposed changes
Testing procedure
Pre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
featbranch created?featbranch merging, do all satellite projects have a proper tag insidego.mod?