-
Notifications
You must be signed in to change notification settings - Fork 205
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 writing on channel when a block is requested in order to attest the current one #6794
Conversation
process/block/metablock.go
Outdated
@@ -2073,6 +2073,12 @@ func (mp *metaProcessor) receivedShardHeader(headerHandler data.HeaderHandler, s | |||
"hash", shardHeaderHash, | |||
) | |||
|
|||
if mp.isWaitingForNextHeader.IsSet() { |
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.
there is no guarantee that the received header is the one we requested or it is in fact another one, so in the second case we would falsely exit the wait.
process/block/shardblock.go
Outdated
@@ -1751,6 +1751,12 @@ func (sp *shardProcessor) receivedMetaBlock(headerHandler data.HeaderHandler, me | |||
"hash", metaBlockHash, | |||
) | |||
|
|||
if sp.isWaitingForNextHeader.IsSet() { |
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.
there is no guarantee that the received header is the one that was requested.
process/block/baseProcess.go
Outdated
@@ -11,6 +11,7 @@ import ( | |||
"time" | |||
|
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.
please optimize as well func (mp *metaProcessor) checkProofsForShardData(header *block.MetaBlock)
to request needed blocks and verify the shard data in parallel
process/block/baseProcess.go
Outdated
|
||
bp.mutRequestedAttestingNoncesMap.RLock() | ||
isWaitingForProofs := len(bp.requestedAttestingNoncesMap) > 0 | ||
defer bp.mutRequestedAttestingNoncesMap.RUnlock() |
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.
defer bp.mutRequestedAttestingNoncesMap.RUnlock() | |
bp.mutRequestedAttestingNoncesMap.RUnlock() |
?
…n-go into fix_time_out_error_on_sync
process/block/baseProcess.go
Outdated
select { | ||
case <-bp.chanNextHeader: | ||
bp.isWaitingForNextHeader.Reset() | ||
case <-bp.allProofsReceived: | ||
return nil | ||
case <-time.After(bp.extraDelayRequestBlockInfo): |
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.
check what happens in case of synced node processing in consensus
if this timeout is set properly for that
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.
maybe use the passed haveTime callback instead
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.
updated
return | ||
} | ||
|
||
bp.mutRequestedAttestingNoncesMap.Lock() |
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.
maybe extract the locked section into a method, then you can do a defer and cover future exit cases
process/block/metablock.go
Outdated
@@ -423,6 +423,11 @@ func (mp *metaProcessor) checkProofsForShardData(header *block.MetaBlock) error | |||
return nil | |||
} | |||
|
|||
mp.mutRequestedAttestingNoncesMap.Lock() | |||
mp.requestedAttestingNoncesMap = make(map[string]uint64) | |||
_ = core.EmptyChannel(mp.allProofsReceived) |
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.
outside of the locked section
…n-go into fix_time_out_error_on_sync
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:
feat
branch created?feat
branch merging, do all satellite projects have a proper tag insidego.mod
?