Fix getBlocks to not add the anticone when some blocks were filtered by GetHashesBetween (#1611)

* Fix getBlocks to not add the anticone when some blocks were filtered by GetHashesBetween

* Fix TestSyncManager_GetHashesBetween
This commit is contained in:
Ori Newman 2021-03-16 14:43:02 +02:00 committed by GitHub
parent cbd0bb6d14
commit b84080f3d9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 29 additions and 28 deletions

View File

@ -50,7 +50,7 @@ func (flow *handleRequestHeadersFlow) start() error {
// GetHashesBetween is a relatively heavy operation so we limit it // GetHashesBetween is a relatively heavy operation so we limit it
// in order to avoid locking the consensus for too long // in order to avoid locking the consensus for too long
const maxBlueScoreDifference = 1 << 10 const maxBlueScoreDifference = 1 << 10
blockHashes, err := flow.Domain().Consensus().GetHashesBetween(lowHash, highHash, maxBlueScoreDifference) blockHashes, _, err := flow.Domain().Consensus().GetHashesBetween(lowHash, highHash, maxBlueScoreDifference)
if err != nil { if err != nil {
return err return err
} }

View File

@ -181,7 +181,7 @@ func (f *fakeRelayInvsContext) GetBlockAcceptanceData(blockHash *externalapi.Dom
panic(errors.Errorf("called unimplemented function from test '%s'", f.testName)) panic(errors.Errorf("called unimplemented function from test '%s'", f.testName))
} }
func (f *fakeRelayInvsContext) GetHashesBetween(lowHash, highHash *externalapi.DomainHash, maxBlueScoreDifference uint64) ([]*externalapi.DomainHash, error) { func (f *fakeRelayInvsContext) GetHashesBetween(lowHash, highHash *externalapi.DomainHash, maxBlueScoreDifference uint64) (hashes []*externalapi.DomainHash, actualHighHash *externalapi.DomainHash, err error) {
panic(errors.Errorf("called unimplemented function from test '%s'", f.testName)) panic(errors.Errorf("called unimplemented function from test '%s'", f.testName))
} }

View File

@ -55,7 +55,7 @@ func HandleGetBlocks(context *rpccontext.Context, _ *router.Router, request appm
if err != nil { if err != nil {
return nil, err return nil, err
} }
blockHashes, err := context.Domain.Consensus().GetHashesBetween( blockHashes, highHash, err := context.Domain.Consensus().GetHashesBetween(
lowHash, virtualSelectedParent, maxBlocksInGetBlocksResponse) lowHash, virtualSelectedParent, maxBlocksInGetBlocksResponse)
if err != nil { if err != nil {
return nil, err return nil, err
@ -64,9 +64,10 @@ func HandleGetBlocks(context *rpccontext.Context, _ *router.Router, request appm
// prepend low hash to make it inclusive // prepend low hash to make it inclusive
blockHashes = append([]*externalapi.DomainHash{lowHash}, blockHashes...) blockHashes = append([]*externalapi.DomainHash{lowHash}, blockHashes...)
// If there are no maxBlocksInGetBlocksResponse between lowHash and virtualSelectedParent - // If the high hash is equal to virtualSelectedParent it means GetHashesBetween didn't skip any hashes, and
// add virtualSelectedParent's anticone // there's space to add the virtualSelectedParent's anticone, otherwise you can't add the anticone because
if len(blockHashes) < maxBlocksInGetBlocksResponse { // there's no guarantee that all of the anticone root ancestors will be present.
if highHash.Equal(virtualSelectedParent) {
virtualSelectedParentAnticone, err := context.Domain.Consensus().Anticone(virtualSelectedParent) virtualSelectedParentAnticone, err := context.Domain.Consensus().Anticone(virtualSelectedParent)
if err != nil { if err != nil {
return nil, err return nil, err

View File

@ -182,18 +182,18 @@ func (s *consensus) GetBlockAcceptanceData(blockHash *externalapi.DomainHash) (e
} }
func (s *consensus) GetHashesBetween(lowHash, highHash *externalapi.DomainHash, func (s *consensus) GetHashesBetween(lowHash, highHash *externalapi.DomainHash,
maxBlueScoreDifference uint64) ([]*externalapi.DomainHash, error) { maxBlueScoreDifference uint64) (hashes []*externalapi.DomainHash, actualHighHash *externalapi.DomainHash, err error) {
s.lock.Lock() s.lock.Lock()
defer s.lock.Unlock() defer s.lock.Unlock()
err := s.validateBlockHashExists(lowHash) err = s.validateBlockHashExists(lowHash)
if err != nil { if err != nil {
return nil, err return nil, nil, err
} }
err = s.validateBlockHashExists(highHash) err = s.validateBlockHashExists(highHash)
if err != nil { if err != nil {
return nil, err return nil, nil, err
} }
return s.syncManager.GetHashesBetween(lowHash, highHash, maxBlueScoreDifference) return s.syncManager.GetHashesBetween(lowHash, highHash, maxBlueScoreDifference)

View File

@ -12,7 +12,7 @@ type Consensus interface {
GetBlockChildren(blockHash *DomainHash) ([]*DomainHash, error) GetBlockChildren(blockHash *DomainHash) ([]*DomainHash, error)
GetBlockAcceptanceData(blockHash *DomainHash) (AcceptanceData, error) GetBlockAcceptanceData(blockHash *DomainHash) (AcceptanceData, error)
GetHashesBetween(lowHash, highHash *DomainHash, maxBlueScoreDifference uint64) ([]*DomainHash, error) GetHashesBetween(lowHash, highHash *DomainHash, maxBlueScoreDifference uint64) (hashes []*DomainHash, actualHighHash *DomainHash, err error)
GetMissingBlockBodyHashes(highHash *DomainHash) ([]*DomainHash, error) GetMissingBlockBodyHashes(highHash *DomainHash) ([]*DomainHash, error)
GetPruningPointUTXOs(expectedPruningPointHash *DomainHash, fromOutpoint *DomainOutpoint, limit int) ([]*OutpointAndUTXOEntryPair, error) GetPruningPointUTXOs(expectedPruningPointHash *DomainHash, fromOutpoint *DomainOutpoint, limit int) ([]*OutpointAndUTXOEntryPair, error)
GetVirtualUTXOs(expectedVirtualParents []*DomainHash, fromOutpoint *DomainOutpoint, limit int) ([]*OutpointAndUTXOEntryPair, error) GetVirtualUTXOs(expectedVirtualParents []*DomainHash, fromOutpoint *DomainOutpoint, limit int) ([]*OutpointAndUTXOEntryPair, error)

View File

@ -4,7 +4,8 @@ import "github.com/kaspanet/kaspad/domain/consensus/model/externalapi"
// SyncManager exposes functions to support sync between kaspad nodes // SyncManager exposes functions to support sync between kaspad nodes
type SyncManager interface { type SyncManager interface {
GetHashesBetween(lowHash, highHash *externalapi.DomainHash, maxBlueScoreDifference uint64) ([]*externalapi.DomainHash, error) GetHashesBetween(lowHash, highHash *externalapi.DomainHash, maxBlueScoreDifference uint64) (
hashes []*externalapi.DomainHash, actualHighHash *externalapi.DomainHash, err error)
GetMissingBlockBodyHashes(highHash *externalapi.DomainHash) ([]*externalapi.DomainHash, error) GetMissingBlockBodyHashes(highHash *externalapi.DomainHash) ([]*externalapi.DomainHash, error)
CreateBlockLocator(lowHash, highHash *externalapi.DomainHash, limit uint32) (externalapi.BlockLocator, error) CreateBlockLocator(lowHash, highHash *externalapi.DomainHash, limit uint32) (externalapi.BlockLocator, error)
CreateHeadersSelectedChainBlockLocator(lowHash, highHash *externalapi.DomainHash) (externalapi.BlockLocator, error) CreateHeadersSelectedChainBlockLocator(lowHash, highHash *externalapi.DomainHash) (externalapi.BlockLocator, error)

View File

@ -11,29 +11,28 @@ import (
// `maxBlueScoreDifference`, if non-zero. // `maxBlueScoreDifference`, if non-zero.
// The result excludes lowHash and includes highHash. If lowHash == highHash, returns nothing. // The result excludes lowHash and includes highHash. If lowHash == highHash, returns nothing.
func (sm *syncManager) antiPastHashesBetween(lowHash, highHash *externalapi.DomainHash, func (sm *syncManager) antiPastHashesBetween(lowHash, highHash *externalapi.DomainHash,
maxBlueScoreDifference uint64) ([]*externalapi.DomainHash, error) { maxBlueScoreDifference uint64) (hashes []*externalapi.DomainHash, actualHighHash *externalapi.DomainHash, err error) {
// If lowHash is not in the selectedParentChain of highHash - SelectedChildIterator will fail. // If lowHash is not in the selectedParentChain of highHash - SelectedChildIterator will fail.
// Therefore, we traverse down lowHash's selectedParentChain until we reach a block that is in // Therefore, we traverse down lowHash's selectedParentChain until we reach a block that is in
// highHash's selectedParentChain. // highHash's selectedParentChain.
// We keep originalLowHash to filter out blocks in it's past later down the road // We keep originalLowHash to filter out blocks in it's past later down the road
originalLowHash := lowHash originalLowHash := lowHash
var err error
lowHash, err = sm.findLowHashInHighHashSelectedParentChain(lowHash, highHash) lowHash, err = sm.findLowHashInHighHashSelectedParentChain(lowHash, highHash)
if err != nil { if err != nil {
return nil, err return nil, nil, err
} }
lowBlockGHOSTDAGData, err := sm.ghostdagDataStore.Get(sm.databaseContext, lowHash) lowBlockGHOSTDAGData, err := sm.ghostdagDataStore.Get(sm.databaseContext, lowHash)
if err != nil { if err != nil {
return nil, err return nil, nil, err
} }
highBlockGHOSTDAGData, err := sm.ghostdagDataStore.Get(sm.databaseContext, highHash) highBlockGHOSTDAGData, err := sm.ghostdagDataStore.Get(sm.databaseContext, highHash)
if err != nil { if err != nil {
return nil, err return nil, nil, err
} }
if lowBlockGHOSTDAGData.BlueScore() > highBlockGHOSTDAGData.BlueScore() { if lowBlockGHOSTDAGData.BlueScore() > highBlockGHOSTDAGData.BlueScore() {
return nil, errors.Errorf("low hash blueScore > high hash blueScore (%d > %d)", return nil, nil, errors.Errorf("low hash blueScore > high hash blueScore (%d > %d)",
lowBlockGHOSTDAGData.BlueScore(), highBlockGHOSTDAGData.BlueScore()) lowBlockGHOSTDAGData.BlueScore(), highBlockGHOSTDAGData.BlueScore())
} }
@ -49,7 +48,7 @@ func (sm *syncManager) antiPastHashesBetween(lowHash, highHash *externalapi.Doma
// blue. // blue.
highHash, err = sm.findHighHashAccordingToMaxBlueScoreDifference(lowHash, highHash, maxBlueScoreDifference, highBlockGHOSTDAGData, lowBlockGHOSTDAGData) highHash, err = sm.findHighHashAccordingToMaxBlueScoreDifference(lowHash, highHash, maxBlueScoreDifference, highBlockGHOSTDAGData, lowBlockGHOSTDAGData)
if err != nil { if err != nil {
return nil, err return nil, nil, err
} }
} }
@ -57,13 +56,13 @@ func (sm *syncManager) antiPastHashesBetween(lowHash, highHash *externalapi.Doma
blockHashes := []*externalapi.DomainHash{} blockHashes := []*externalapi.DomainHash{}
iterator, err := sm.dagTraversalManager.SelectedChildIterator(highHash, lowHash) iterator, err := sm.dagTraversalManager.SelectedChildIterator(highHash, lowHash)
if err != nil { if err != nil {
return nil, err return nil, nil, err
} }
defer iterator.Close() defer iterator.Close()
for ok := iterator.First(); ok; ok = iterator.Next() { for ok := iterator.First(); ok; ok = iterator.Next() {
current, err := iterator.Get() current, err := iterator.Get()
if err != nil { if err != nil {
return nil, err return nil, nil, err
} }
// Both blue and red merge sets are topologically sorted, but not the concatenation of the two. // Both blue and red merge sets are topologically sorted, but not the concatenation of the two.
// We require the blocks to be topologically sorted. In addition, for optimal performance, // We require the blocks to be topologically sorted. In addition, for optimal performance,
@ -73,14 +72,14 @@ func (sm *syncManager) antiPastHashesBetween(lowHash, highHash *externalapi.Doma
// Therefore we first append the selectedParent, then the rest of blocks in ghostdag order. // Therefore we first append the selectedParent, then the rest of blocks in ghostdag order.
sortedMergeSet, err := sm.getSortedMergeSet(current) sortedMergeSet, err := sm.getSortedMergeSet(current)
if err != nil { if err != nil {
return nil, err return nil, nil, err
} }
// append to blockHashes all blocks in sortedMergeSet which are not in the past of originalLowHash // append to blockHashes all blocks in sortedMergeSet which are not in the past of originalLowHash
for _, blockHash := range sortedMergeSet { for _, blockHash := range sortedMergeSet {
isInPastOfOriginalLowHash, err := sm.dagTopologyManager.IsAncestorOf(blockHash, originalLowHash) isInPastOfOriginalLowHash, err := sm.dagTopologyManager.IsAncestorOf(blockHash, originalLowHash)
if err != nil { if err != nil {
return nil, err return nil, nil, err
} }
if isInPastOfOriginalLowHash { if isInPastOfOriginalLowHash {
continue continue
@ -94,7 +93,7 @@ func (sm *syncManager) antiPastHashesBetween(lowHash, highHash *externalapi.Doma
blockHashes = append(blockHashes, highHash) blockHashes = append(blockHashes, highHash)
} }
return blockHashes, nil return blockHashes, highHash, nil
} }
func (sm *syncManager) getSortedMergeSet(current *externalapi.DomainHash) ([]*externalapi.DomainHash, error) { func (sm *syncManager) getSortedMergeSet(current *externalapi.DomainHash) ([]*externalapi.DomainHash, error) {
@ -226,7 +225,7 @@ func (sm *syncManager) missingBlockBodyHashes(highHash *externalapi.DomainHash)
lowHash, highHash) lowHash, highHash)
} }
hashesBetween, err := sm.antiPastHashesBetween(lowHash, highHash, 0) hashesBetween, _, err := sm.antiPastHashesBetween(lowHash, highHash, 0)
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@ -58,7 +58,7 @@ func New(
} }
func (sm *syncManager) GetHashesBetween(lowHash, highHash *externalapi.DomainHash, func (sm *syncManager) GetHashesBetween(lowHash, highHash *externalapi.DomainHash,
maxBlueScoreDifference uint64) ([]*externalapi.DomainHash, error) { maxBlueScoreDifference uint64) (hashes []*externalapi.DomainHash, actualHighHash *externalapi.DomainHash, err error) {
onEnd := logger.LogAndMeasureExecutionTime(log, "GetHashesBetween") onEnd := logger.LogAndMeasureExecutionTime(log, "GetHashesBetween")
defer onEnd() defer onEnd()

View File

@ -55,7 +55,7 @@ func TestSyncManager_GetHashesBetween(t *testing.T) {
} }
for i, blockHash := range expectedOrder { for i, blockHash := range expectedOrder {
empty, err := tc.SyncManager().GetHashesBetween(blockHash, blockHash, math.MaxUint64) empty, _, err := tc.SyncManager().GetHashesBetween(blockHash, blockHash, math.MaxUint64)
if err != nil { if err != nil {
t.Fatalf("TestSyncManager_GetHashesBetween failed returning 0 hashes on the %d'th block: %v", i, err) t.Fatalf("TestSyncManager_GetHashesBetween failed returning 0 hashes on the %d'th block: %v", i, err)
} }
@ -64,7 +64,7 @@ func TestSyncManager_GetHashesBetween(t *testing.T) {
} }
} }
actualOrder, err := tc.SyncManager().GetHashesBetween(params.GenesisHash, expectedOrder[len(expectedOrder)-1], math.MaxUint64) actualOrder, _, err := tc.SyncManager().GetHashesBetween(params.GenesisHash, expectedOrder[len(expectedOrder)-1], math.MaxUint64)
if err != nil { if err != nil {
t.Fatalf("TestSyncManager_GetHashesBetween failed returning actualOrder: %v", err) t.Fatalf("TestSyncManager_GetHashesBetween failed returning actualOrder: %v", err)
} }