From de447490446663a93650607a363f2e0a78eb4d2b Mon Sep 17 00:00:00 2001 From: Sebastian Marian Date: Sat, 26 Oct 2019 14:37:35 +0300 Subject: [PATCH] * Improved mechanism to set forced fork * Changed default values of some constants --- consensus/mock/forkDetectorMock.go | 5 ++ integrationTests/mock/forkDetectorMock.go | 5 ++ node/mock/forkDetectorMock.go | 5 ++ process/block/metablock.go | 2 +- process/block/shardblock.go | 2 +- process/constants.go | 12 +++-- process/interface.go | 1 + process/mock/forkDetectorMock.go | 5 ++ process/sync/baseForkDetector.go | 64 +++++++++++++++++++---- process/sync/baseForkDetector_test.go | 4 +- process/sync/metaForkDetector.go | 2 +- process/sync/metablock.go | 15 ++++-- process/sync/metablock_test.go | 2 +- process/sync/shardForkDetector.go | 2 +- process/sync/shardblock.go | 15 ++++-- process/sync/shardblock_test.go | 2 +- 16 files changed, 112 insertions(+), 31 deletions(-) diff --git a/consensus/mock/forkDetectorMock.go b/consensus/mock/forkDetectorMock.go index a28915cd178..ad14cf7b8e1 100644 --- a/consensus/mock/forkDetectorMock.go +++ b/consensus/mock/forkDetectorMock.go @@ -13,6 +13,7 @@ type ForkDetectorMock struct { ProbableHighestNonceCalled func() uint64 ResetProbableHighestNonceIfNeededCalled func() ResetProbableHighestNonceCalled func() + ResetForcedForkCalled func() } func (fdm *ForkDetectorMock) AddHeader(header data.HeaderHandler, hash []byte, state process.BlockHeaderState, finalHeaders []data.HeaderHandler, finalHeadersHashes [][]byte) error { @@ -43,6 +44,10 @@ func (fdm *ForkDetectorMock) ResetProbableHighestNonce() { fdm.ResetProbableHighestNonceCalled() } +func (fdm *ForkDetectorMock) ResetForcedFork() { + fdm.ResetForcedForkCalled() +} + // IsInterfaceNil returns true if there is no value under the interface func (fdm *ForkDetectorMock) IsInterfaceNil() bool { if fdm == nil { diff --git a/integrationTests/mock/forkDetectorMock.go b/integrationTests/mock/forkDetectorMock.go index 81e0df24f09..dd8919ca367 100644 --- a/integrationTests/mock/forkDetectorMock.go +++ b/integrationTests/mock/forkDetectorMock.go @@ -14,6 +14,7 @@ type ForkDetectorMock struct { ProbableHighestNonceCalled func() uint64 ResetProbableHighestNonceIfNeededCalled func() ResetProbableHighestNonceCalled func() + ResetForcedForkCalled func() } // AddHeader is a mock implementation for AddHeader @@ -49,6 +50,10 @@ func (fdm *ForkDetectorMock) ResetProbableHighestNonce() { fdm.ResetProbableHighestNonceCalled() } +func (fdm *ForkDetectorMock) ResetForcedFork() { + fdm.ResetForcedForkCalled() +} + // IsInterfaceNil returns true if there is no value under the interface func (fdm *ForkDetectorMock) IsInterfaceNil() bool { if fdm == nil { diff --git a/node/mock/forkDetectorMock.go b/node/mock/forkDetectorMock.go index ed42b5206ce..0e3b06c06cd 100644 --- a/node/mock/forkDetectorMock.go +++ b/node/mock/forkDetectorMock.go @@ -14,6 +14,7 @@ type ForkDetectorMock struct { ProbableHighestNonceCalled func() uint64 ResetProbableHighestNonceIfNeededCalled func() ResetProbableHighestNonceCalled func() + ResetForcedForkCalled func() } // AddHeader is a mock implementation for AddHeader @@ -49,6 +50,10 @@ func (fdm *ForkDetectorMock) ResetProbableHighestNonce() { fdm.ResetProbableHighestNonceCalled() } +func (fdm *ForkDetectorMock) ResetForcedFork() { + fdm.ResetForcedForkCalled() +} + // IsInterfaceNil returns true if there is no value under the interface func (fdm *ForkDetectorMock) IsInterfaceNil() bool { if fdm == nil { diff --git a/process/block/metablock.go b/process/block/metablock.go index 23800a1e739..ed35b064260 100644 --- a/process/block/metablock.go +++ b/process/block/metablock.go @@ -459,7 +459,7 @@ func (mp *metaProcessor) CommitBlock( } headerNoncePool.Remove(header.GetNonce(), header.GetShardID()) - //metaBlockPool.Remove(headerHash) + //TODO: metaBlockPool.Remove(headerHash) should not be called, just to have a restore point if it is needed later body, ok := bodyHandler.(*block.MetaBlockBody) if !ok { diff --git a/process/block/shardblock.go b/process/block/shardblock.go index 47ee82a0bd9..d3933ca6ff5 100644 --- a/process/block/shardblock.go +++ b/process/block/shardblock.go @@ -627,7 +627,7 @@ func (sp *shardProcessor) CommitBlock( } headerNoncePool.Remove(header.GetNonce(), header.GetShardID()) - //headersPool.Remove(headerHash) + //TODO: headersPool.Remove(headerHash) should not be called, just to have a restore point if it is needed later body, ok := bodyHandler.(block.Body) if !ok { diff --git a/process/constants.go b/process/constants.go index 5190384a5e0..c179d7fdec4 100644 --- a/process/constants.go +++ b/process/constants.go @@ -35,7 +35,7 @@ const MetaBlockFinality = 1 const MaxHeaderRequestsAllowed = 10 const MaxItemsInBlock = 15000 const MinItemsInBlock = 15000 -const MaxNoncesDifference = 5 +const MaxNoncesDifference = 0 // TODO - calculate exactly in case of the VM, for every VM to have a similar constant, operations / seconds const MaxGasLimitPerMiniBlock = uint64(100000) @@ -51,6 +51,10 @@ const RoundModulusTrigger = 10 // from the full pool capacity, for the received data which are not needed in the near future const MaxOccupancyPercentageAllowed = float64(0.9) -// MaxRoundsToWait defines the maximum rounds to wait, when bootstrapping, after which the node will add an empty -// block through recovery mechanism, if its block request is not resolved and no new block header is received meantime -const MaxRoundsToWait = 5 +// MaxRoundsWithoutReceivedBlock defines the maximum rounds to wait for a new block to be received, before a special +// action to be applied +const MaxRoundsWithoutReceivedBlock = 5 + +// MaxRoundsWithoutCommittedBlock defines the maximum rounds to wait for a new block to be committed, before a special +// action to be applied +const MaxRoundsWithoutCommittedBlock = 20 diff --git a/process/interface.go b/process/interface.go index d8737c2feff..8d402d2fc07 100644 --- a/process/interface.go +++ b/process/interface.go @@ -277,6 +277,7 @@ type ForkDetector interface { ProbableHighestNonce() uint64 ResetProbableHighestNonceIfNeeded() ResetProbableHighestNonce() + ResetForcedFork() IsInterfaceNil() bool } diff --git a/process/mock/forkDetectorMock.go b/process/mock/forkDetectorMock.go index a28915cd178..ad14cf7b8e1 100644 --- a/process/mock/forkDetectorMock.go +++ b/process/mock/forkDetectorMock.go @@ -13,6 +13,7 @@ type ForkDetectorMock struct { ProbableHighestNonceCalled func() uint64 ResetProbableHighestNonceIfNeededCalled func() ResetProbableHighestNonceCalled func() + ResetForcedForkCalled func() } func (fdm *ForkDetectorMock) AddHeader(header data.HeaderHandler, hash []byte, state process.BlockHeaderState, finalHeaders []data.HeaderHandler, finalHeadersHashes [][]byte) error { @@ -43,6 +44,10 @@ func (fdm *ForkDetectorMock) ResetProbableHighestNonce() { fdm.ResetProbableHighestNonceCalled() } +func (fdm *ForkDetectorMock) ResetForcedFork() { + fdm.ResetForcedForkCalled() +} + // IsInterfaceNil returns true if there is no value under the interface func (fdm *ForkDetectorMock) IsInterfaceNil() bool { if fdm == nil { diff --git a/process/sync/baseForkDetector.go b/process/sync/baseForkDetector.go index f407afa0a27..91fccdccd36 100644 --- a/process/sync/baseForkDetector.go +++ b/process/sync/baseForkDetector.go @@ -12,6 +12,7 @@ import ( "github.com/ElrondNetwork/elrond-go/process" ) +// MinForkRound represents the minimum fork round set by a notarized header received const MinForkRound = uint64(0) type headerInfo struct { @@ -32,6 +33,7 @@ type forkInfo struct { probableHighestNonce uint64 lastBlockRound uint64 lastProposedBlockNonce uint64 + forcedFork bool } // baseForkDetector defines a struct with necessary data needed for fork detection @@ -42,7 +44,6 @@ type baseForkDetector struct { mutHeaders sync.RWMutex fork forkInfo mutFork sync.RWMutex - forceFork bool } func (bfd *baseForkDetector) removePastOrInvalidRecords() { @@ -259,7 +260,7 @@ func (bfd *baseForkDetector) ResetProbableHighestNonceIfNeeded() { // committed block + 1, which could not be verified by hash -> prev hash and only by rand seed -> prev random seed roundsWithoutReceivedBlock := bfd.rounder.Index() - int64(bfd.lastBlockRound()) isInProperRound := process.IsInProperRound(bfd.rounder.Index()) - if roundsWithoutReceivedBlock > process.MaxRoundsToWait && isInProperRound { + if roundsWithoutReceivedBlock > process.MaxRoundsWithoutReceivedBlock && isInProperRound { bfd.ResetProbableHighestNonce() } } @@ -273,6 +274,11 @@ func (bfd *baseForkDetector) ResetProbableHighestNonce() { } } +// ResetForcedFork resets the forced fork +func (bfd *baseForkDetector) ResetForcedFork() { + bfd.setForcedFork(false) +} + func (bfd *baseForkDetector) addCheckpoint(checkpoint *checkpointInfo) { bfd.mutFork.Lock() bfd.fork.checkpoint = append(bfd.fork.checkpoint, checkpoint) @@ -348,6 +354,20 @@ func (bfd *baseForkDetector) lastProposedBlockNonce() uint64 { return lastProposedBlockNonce } +func (bfd *baseForkDetector) setForcedFork(forcedFork bool) { + bfd.mutFork.Lock() + bfd.fork.forcedFork = forcedFork + bfd.mutFork.Unlock() +} + +func (bfd *baseForkDetector) forcedFork() bool { + bfd.mutFork.RLock() + forcedFork := bfd.fork.forcedFork + bfd.mutFork.RUnlock() + + return forcedFork +} + // IsInterfaceNil returns true if there is no value under the interface func (bfd *baseForkDetector) IsInterfaceNil() bool { if bfd == nil { @@ -358,10 +378,6 @@ func (bfd *baseForkDetector) IsInterfaceNil() bool { // CheckFork method checks if the node could be on the fork func (bfd *baseForkDetector) CheckFork() (bool, uint64, []byte) { - if bfd.forceFork { - return true, math.MaxUint64, nil - } - var ( lowestForkNonce uint64 hashOfLowestForkNonce []byte @@ -374,6 +390,10 @@ func (bfd *baseForkDetector) CheckFork() (bool, uint64, []byte) { hashOfLowestForkNonce = nil forkDetected := false + if bfd.forcedFork() { + return true, lowestForkNonce, hashOfLowestForkNonce + } + bfd.mutHeaders.Lock() for nonce, hdrInfos := range bfd.headers { if len(hdrInfos) == 1 { @@ -463,7 +483,7 @@ func (bfd *baseForkDetector) shouldAddBlockInForkDetector( ) error { noncesDifference := int64(bfd.ProbableHighestNonce()) - int64(header.GetNonce()) - isSyncing := state == process.BHReceived && noncesDifference > process.MaxNoncesDifference + isSyncing := noncesDifference > process.MaxNoncesDifference if state == process.BHProcessed || isSyncing { return nil } @@ -476,9 +496,31 @@ func (bfd *baseForkDetector) shouldAddBlockInForkDetector( return nil } -func (bfd *baseForkDetector) shouldForceFork() bool { - roundsWithoutReceivedBlock := bfd.rounder.Index() - int64(bfd.lastBlockRound()) +func (bfd *baseForkDetector) computeForcedFork( + header data.HeaderHandler, + state process.BlockHeaderState) { + if state != process.BHProposed { + bfd.setForcedFork(false) + return + } + + noncesDifference := int64(bfd.ProbableHighestNonce()) - int64(header.GetNonce()) + isSyncing := noncesDifference > process.MaxNoncesDifference + if isSyncing { + bfd.setForcedFork(false) + return + } + + lastCheckpointRound := bfd.lastCheckpoint().round + lastCheckpointNonce := bfd.lastCheckpoint().nonce + + roundsDifference := int64(header.GetRound()) - int64(lastCheckpointRound) + noncesDifference = int64(header.GetNonce()) - int64(lastCheckpointNonce) isInProperRound := process.IsInProperRound(bfd.rounder.Index()) - shouldForceFork := roundsWithoutReceivedBlock > process.MaxRoundsToWait && isInProperRound - return shouldForceFork + + shouldForceFork := roundsDifference > process.MaxRoundsWithoutCommittedBlock && + noncesDifference <= 1 && + isInProperRound + + bfd.setForcedFork(shouldForceFork) } diff --git a/process/sync/baseForkDetector_test.go b/process/sync/baseForkDetector_test.go index 33a7acba358..9c2b7a54c18 100644 --- a/process/sync/baseForkDetector_test.go +++ b/process/sync/baseForkDetector_test.go @@ -738,7 +738,7 @@ func TestShardForkDetector_ShouldAddBlockInForkDetectorShouldErrLowerRoundInBloc err := sfd.ShouldAddBlockInForkDetector(hdr, process.BHReceived, process.ShardBlockFinality) assert.Equal(t, sync.ErrLowerRoundInBlock, err) - sfd.SetProbableHighestNonce(hdr.GetNonce() + process.MaxNoncesDifference + 1) + sfd.SetProbableHighestNonce(hdr.GetNonce() + process.MaxNoncesDifference) err = sfd.ShouldAddBlockInForkDetector(hdr, process.BHProposed, process.ShardBlockFinality) assert.Equal(t, sync.ErrLowerRoundInBlock, err) } @@ -776,7 +776,7 @@ func TestMetaForkDetector_ShouldAddBlockInForkDetectorShouldErrLowerRoundInBlock err := mfd.ShouldAddBlockInForkDetector(hdr, process.BHReceived, process.MetaBlockFinality) assert.Equal(t, sync.ErrLowerRoundInBlock, err) - mfd.SetProbableHighestNonce(hdr.GetNonce() + process.MaxNoncesDifference + 1) + mfd.SetProbableHighestNonce(hdr.GetNonce() + process.MaxNoncesDifference) err = mfd.ShouldAddBlockInForkDetector(hdr, process.BHProposed, process.MetaBlockFinality) assert.Equal(t, sync.ErrLowerRoundInBlock, err) } diff --git a/process/sync/metaForkDetector.go b/process/sync/metaForkDetector.go index ac3006c338c..a7a181c86a6 100644 --- a/process/sync/metaForkDetector.go +++ b/process/sync/metaForkDetector.go @@ -54,7 +54,7 @@ func (mfd *metaForkDetector) AddHeader( return err } - mfd.forceFork = mfd.shouldForceFork() + mfd.computeForcedFork(header, state) err = mfd.shouldAddBlockInForkDetector(header, state, process.MetaBlockFinality) if err != nil { diff --git a/process/sync/metablock.go b/process/sync/metablock.go index 26aa715eeac..0c02deb2371 100644 --- a/process/sync/metablock.go +++ b/process/sync/metablock.go @@ -422,9 +422,10 @@ func (boot *MetaBootstrap) SyncBlock() error { return nil } - forceFork := boot.isForkDetected && boot.forkNonce == math.MaxUint64 && boot.forkHash == nil if boot.isForkDetected { - if forceFork { + isForcedFork := boot.forkNonce == math.MaxUint64 && boot.forkHash == nil + + if isForcedFork { log.Info(fmt.Sprintf("fork has been forced\n")) } else { log.Info(fmt.Sprintf("fork detected at nonce %d with hash %s\n", @@ -434,10 +435,16 @@ func (boot *MetaBootstrap) SyncBlock() error { boot.statusHandler.Increment(core.MetricNumTimesInForkChoice) - err := boot.forkChoice(!forceFork) + err := boot.forkChoice(!isForcedFork) if err != nil { log.Info(err.Error()) } + + if isForcedFork { + boot.forkDetector.ResetProbableHighestNonce() + boot.forkDetector.ResetForcedFork() + return nil + } } boot.setRequestedHeaderNonce(nil) @@ -454,7 +461,7 @@ func (boot *MetaBootstrap) SyncBlock() error { } }() - if boot.isForkDetected && !forceFork { + if boot.isForkDetected { hdr, err = boot.getHeaderWithHashRequestingIfMissing(boot.forkHash) } else { hdr, err = boot.getHeaderWithNonceRequestingIfMissing(nonce) diff --git a/process/sync/metablock_test.go b/process/sync/metablock_test.go index 975948ae495..04fc52a1bdb 100644 --- a/process/sync/metablock_test.go +++ b/process/sync/metablock_test.go @@ -657,7 +657,7 @@ func TestMetaBootstrap_SyncBlockShouldCallForkChoice(t *testing.T) { marshalizer := &mock.MarshalizerMock{} forkDetector := &mock.ForkDetectorMock{} forkDetector.CheckForkCalled = func() (bool, uint64, []byte) { - return true, math.MaxUint64, nil + return true, 90, []byte("hash") } forkDetector.RemoveHeadersCalled = func(nonce uint64, hash []byte) { } diff --git a/process/sync/shardForkDetector.go b/process/sync/shardForkDetector.go index 90364990f61..e3d28a00acd 100644 --- a/process/sync/shardForkDetector.go +++ b/process/sync/shardForkDetector.go @@ -54,7 +54,7 @@ func (sfd *shardForkDetector) AddHeader( return err } - sfd.forceFork = sfd.shouldForceFork() + sfd.computeForcedFork(header, state) err = sfd.shouldAddBlockInForkDetector(header, state, process.ShardBlockFinality) if err != nil { diff --git a/process/sync/shardblock.go b/process/sync/shardblock.go index b51019eea38..d4ad2c81ab6 100644 --- a/process/sync/shardblock.go +++ b/process/sync/shardblock.go @@ -652,9 +652,10 @@ func (boot *ShardBootstrap) SyncBlock() error { return nil } - forceFork := boot.isForkDetected && boot.forkNonce == math.MaxUint64 && boot.forkHash == nil if boot.isForkDetected { - if forceFork { + isForcedFork := boot.forkNonce == math.MaxUint64 && boot.forkHash == nil + + if isForcedFork { log.Info(fmt.Sprintf("fork has been forced\n")) } else { log.Info(fmt.Sprintf("fork detected at nonce %d with hash %s\n", @@ -664,10 +665,16 @@ func (boot *ShardBootstrap) SyncBlock() error { boot.statusHandler.Increment(core.MetricNumTimesInForkChoice) - err := boot.forkChoice(!forceFork) + err := boot.forkChoice(!isForcedFork) if err != nil { log.Info(err.Error()) } + + if isForcedFork { + boot.forkDetector.ResetProbableHighestNonce() + boot.forkDetector.ResetForcedFork() + return nil + } } boot.setRequestedHeaderNonce(nil) @@ -685,7 +692,7 @@ func (boot *ShardBootstrap) SyncBlock() error { } }() - if boot.isForkDetected && !forceFork { + if boot.isForkDetected { hdr, err = boot.getHeaderWithHashRequestingIfMissing(boot.forkHash) } else { hdr, err = boot.getHeaderWithNonceRequestingIfMissing(nonce) diff --git a/process/sync/shardblock_test.go b/process/sync/shardblock_test.go index 2ba85102216..fc2160a14a2 100644 --- a/process/sync/shardblock_test.go +++ b/process/sync/shardblock_test.go @@ -935,7 +935,7 @@ func TestBootstrap_SyncBlockShouldCallForkChoice(t *testing.T) { marshalizer := &mock.MarshalizerMock{} forkDetector := &mock.ForkDetectorMock{} forkDetector.CheckForkCalled = func() (bool, uint64, []byte) { - return true, math.MaxUint64, nil + return true, 90, []byte("hash") } forkDetector.RemoveHeadersCalled = func(nonce uint64, hash []byte) { }