Skip to content
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

stabilization-equivalent-proofs #6686

Draft
wants to merge 23 commits into
base: feat/equivalent-messages
Choose a base branch
from

Conversation

AdoAdoAdo
Copy link
Contributor

@AdoAdoAdo AdoAdoAdo commented Dec 19, 2024

Reasoning behind the pull request

  • Bugfixes and changes while stabilizing the new consensus (v2) based on equivalent proofs

Proposed changes

  • change from epoch start trigger to generic epoch notifier to trigger the transition to new consensus - simpler and covers more cases
  • in case of generic epoch notifier the validators need to also need to trigger the transition correctly
  • early exit from consensus v1 operation if a transition to v2 happens.
  • adapt and fix tests

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?

@AdoAdoAdo AdoAdoAdo changed the title use EpochNotifier instead of EpochStartNotifier to transition to the … stabilization-equivalent-proofs Dec 20, 2024
@AdoAdoAdo AdoAdoAdo self-assigned this Dec 20, 2024
@@ -57,6 +56,13 @@ type SubroundsHandler struct {
currentConsensusType consensusStateMachineType
}

func (s *SubroundsHandler) EpochConfirmed(epoch uint32, _ uint64) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing comment

@@ -17,6 +17,7 @@ func TestEpochChangeWithNodesShufflingAndRater(t *testing.T) {
t.Skip("this is not a short test")
}

_ = logger.SetLogLevel("*:DEBUG")
Copy link
Collaborator

@sstanculeanu sstanculeanu Jan 10, 2025

Choose a reason for hiding this comment

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

not to forget removing this

@@ -203,12 +203,16 @@ func TestSyncWorksInShard_EmptyBlocksNoForks_With_EquivalentProofs(t *testing.T)
t.Skip("this is not a short test")
}

_ = logger.SetLogLevel("*:DEBUG,process:TRACE,consensus:TRACE")
Copy link
Collaborator

@sstanculeanu sstanculeanu Jan 10, 2025

Choose a reason for hiding this comment

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

not to forget removing this

@@ -56,6 +56,7 @@ func (idv *interceptedDataVerifier) Verify(interceptedData process.InterceptedDa

err := interceptedData.CheckValidity()
if err != nil {
log.Debug("Intercepted data is invalid", "hash", interceptedData.Hash(), "err", err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we'll need a special handle here, as interceptedData.Hash() will be the header hash, thus receiving an invalid proof for the first time will lead to always returning ErrInvalidInterceptedData for further proofs(even valid ones)

@@ -384,6 +384,7 @@ type ForkDetector interface {
GetNotarizedHeaderHash(nonce uint64) []byte
ResetProbableHighestNonce()
SetFinalToLastCheckpoint()
// ReceivedProof()
Copy link
Collaborator

@sstanculeanu sstanculeanu Jan 10, 2025

Choose a reason for hiding this comment

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

not to forget removing this

@@ -161,6 +162,10 @@ func (boot *baseBootstrap) requestedHeaderHash() []byte {
return boot.headerhash
}

func (boot *baseBootstrap) processReceivedProof(headerProof data.HeaderProofHandler) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

not to forget the implementation

@@ -52,13 +52,13 @@ export GENESIS_STAKE_TYPE="direct" #'delegated' or 'direct' as in direct stake
export OBSERVERS_ANTIFLOOD_DISABLE=0

# Shard structure
export SHARDCOUNT=2
export SHARD_VALIDATORCOUNT=3
export SHARDCOUNT=1
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be reverted

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.

3 participants