Skip to content

Conversation

@AdoAdoAdo
Copy link
Contributor

@AdoAdoAdo AdoAdoAdo commented Jan 27, 2025

Reasoning behind the pull request

  • During testing the shard nodes falsely detects from time to time that metachain is falling behind with notarization

Proposed changes

  • Fork detector should be updated with the final blocks based on the new rules
  • Consensus subround EndRound needs to be updated to commit the block only once the proof is stored, otherwise the fork detector would wrongly compute the last notarized blocks

Testing procedure

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

  • was the PR targeted to the correct branch?
  • if this is a larger feature that probably needs more than one PR, is there a feat branch created?
  • if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

return sr.finalizeConfirmedBlock()
}

func (sr *subroundEndRound) waitForProof(shardID uint32, headerHash []byte) bool {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parameters are not necessarily needed

}

func (sr *subroundEndRound) finalizeConfirmedBlock() bool {
sr.waitForProof(sr.ShardCoordinator().SelfId(), sr.GetData())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check result of waitForProof and do not proceed in case of false?

if !sr.waitForSignalSync() {
return false
}
_, _ = sr.sendProof()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can remove the return values of sendProof now

@AdoAdoAdo AdoAdoAdo marked this pull request as ready for review January 28, 2025 10:33
// broadcast header proof
proof, err := sr.createAndBroadcastProof(sig, bitmap)
return proof, err == nil
_, err = sr.createAndBroadcastProof(sig, bitmap)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

returned proof variable here can also be removed

func (sr *subroundEndRound) waitForProof() bool {
shardID := sr.ShardCoordinator().SelfId()
headerHash := sr.GetData()
if sr.EquivalentProofsPool().HasProof(sr.ShardCoordinator().SelfId(), sr.GetData()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if sr.EquivalentProofsPool().HasProof(sr.ShardCoordinator().SelfId(), sr.GetData()) {
if sr.EquivalentProofsPool().HasProof(shardID, headerHash) {

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, forgot it.

@AdoAdoAdo AdoAdoAdo merged commit 494e921 into equivalent-proofs-stabilization-2 Jan 28, 2025
3 checks passed
@AdoAdoAdo AdoAdoAdo deleted the fix-shard-stuck branch January 28, 2025 12:50
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.

4 participants