From bcf230246011eed66407f51f6c2478bd5410a66e Mon Sep 17 00:00:00 2001 From: Ori Newman Date: Tue, 12 Jan 2021 11:16:25 +0200 Subject: [PATCH] Add high hash to block locator, and add block locator tests (#1397) * Include high hash in the block locator * Add tests for block locator * Remove redundant function * Remove redundant assignments --- domain/consensus/consensus.go | 11 - .../consensus/model/externalapi/consensus.go | 1 - .../model/interface_processes_syncmanager.go | 1 - .../processes/syncmanager/blocklocator.go | 34 --- .../syncmanager/blocklocator_test.go | 231 ++++++++++++++++++ .../processes/syncmanager/syncmanager.go | 7 - 6 files changed, 231 insertions(+), 54 deletions(-) create mode 100644 domain/consensus/processes/syncmanager/blocklocator_test.go diff --git a/domain/consensus/consensus.go b/domain/consensus/consensus.go index 08a14e10c..9d7bf67d6 100644 --- a/domain/consensus/consensus.go +++ b/domain/consensus/consensus.go @@ -306,17 +306,6 @@ func (s *consensus) CreateHeadersSelectedChainBlockLocator(lowHash, return s.syncManager.CreateHeadersSelectedChainBlockLocator(lowHash, highHash) } -func (s *consensus) FindNextBlockLocatorBoundaries(blockLocator externalapi.BlockLocator) (lowHash, highHash *externalapi.DomainHash, err error) { - s.lock.Lock() - defer s.lock.Unlock() - - if len(blockLocator) == 0 { - return nil, nil, errors.Errorf("empty block locator") - } - - return s.syncManager.FindNextBlockLocatorBoundaries(blockLocator) -} - func (s *consensus) GetSyncInfo() (*externalapi.SyncInfo, error) { s.lock.Lock() defer s.lock.Unlock() diff --git a/domain/consensus/model/externalapi/consensus.go b/domain/consensus/model/externalapi/consensus.go index 535d99664..cce6c9b5e 100644 --- a/domain/consensus/model/externalapi/consensus.go +++ b/domain/consensus/model/externalapi/consensus.go @@ -19,7 +19,6 @@ type Consensus interface { GetVirtualSelectedParent() (*DomainHash, error) CreateBlockLocator(lowHash, highHash *DomainHash, limit uint32) (BlockLocator, error) CreateHeadersSelectedChainBlockLocator(lowHash, highHash *DomainHash) (BlockLocator, error) - FindNextBlockLocatorBoundaries(blockLocator BlockLocator) (lowHash, highHash *DomainHash, err error) GetSyncInfo() (*SyncInfo, error) Tips() ([]*DomainHash, error) GetVirtualInfo() (*VirtualInfo, error) diff --git a/domain/consensus/model/interface_processes_syncmanager.go b/domain/consensus/model/interface_processes_syncmanager.go index 5596f98e7..e67ccb884 100644 --- a/domain/consensus/model/interface_processes_syncmanager.go +++ b/domain/consensus/model/interface_processes_syncmanager.go @@ -8,6 +8,5 @@ type SyncManager interface { GetMissingBlockBodyHashes(highHash *externalapi.DomainHash) ([]*externalapi.DomainHash, error) CreateBlockLocator(lowHash, highHash *externalapi.DomainHash, limit uint32) (externalapi.BlockLocator, error) CreateHeadersSelectedChainBlockLocator(lowHash, highHash *externalapi.DomainHash) (externalapi.BlockLocator, error) - FindNextBlockLocatorBoundaries(blockLocator externalapi.BlockLocator) (lowHash, highHash *externalapi.DomainHash, err error) GetSyncInfo() (*externalapi.SyncInfo, error) } diff --git a/domain/consensus/processes/syncmanager/blocklocator.go b/domain/consensus/processes/syncmanager/blocklocator.go index 62ec0427a..56ddc3b56 100644 --- a/domain/consensus/processes/syncmanager/blocklocator.go +++ b/domain/consensus/processes/syncmanager/blocklocator.go @@ -8,14 +8,6 @@ import ( // createBlockLocator creates a block locator for the passed high and low hashes. // See the BlockLocator type comments for more details. func (sm *syncManager) createBlockLocator(lowHash, highHash *externalapi.DomainHash, limit uint32) (externalapi.BlockLocator, error) { - // We use the selected parent of the high block, so that the - // block locator won't contain it. - highBlockGHOSTDAGData, err := sm.ghostdagDataStore.Get(sm.databaseContext, highHash) - if err != nil { - return nil, err - } - highHash = highBlockGHOSTDAGData.SelectedParent() - lowBlockGHOSTDAGData, err := sm.ghostdagDataStore.Get(sm.databaseContext, lowHash) if err != nil { return nil, err @@ -73,32 +65,6 @@ func (sm *syncManager) createBlockLocator(lowHash, highHash *externalapi.DomainH return locator, nil } -// findNextBlockLocatorBoundaries finds the lowest unknown block locator -// hash and the highest known block locator hash. This is used to create the -// next block locator to find the highest shared known chain block with a -// remote kaspad. -func (sm *syncManager) findNextBlockLocatorBoundaries(blockLocator externalapi.BlockLocator) ( - lowHash, highHash *externalapi.DomainHash, err error) { - - // Find the most recent locator block hash in the DAG. In case none of - // the hashes in the locator are in the DAG, fall back to the genesis block. - highestKnownHash := sm.genesisBlockHash - lowestUnknownHash := blockLocator[len(blockLocator)-1] - for _, hash := range blockLocator { - exists, err := sm.blockStatusStore.Exists(sm.databaseContext, hash) - if err != nil { - return nil, nil, err - } - if !exists { - lowestUnknownHash = hash - } else { - highestKnownHash = hash - break - } - } - return highestKnownHash, lowestUnknownHash, nil -} - func (sm *syncManager) createHeadersSelectedChainBlockLocator(lowHash, highHash *externalapi.DomainHash) (externalapi.BlockLocator, error) { diff --git a/domain/consensus/processes/syncmanager/blocklocator_test.go b/domain/consensus/processes/syncmanager/blocklocator_test.go new file mode 100644 index 000000000..6a6276626 --- /dev/null +++ b/domain/consensus/processes/syncmanager/blocklocator_test.go @@ -0,0 +1,231 @@ +package syncmanager_test + +import ( + "github.com/kaspanet/kaspad/domain/consensus" + "github.com/kaspanet/kaspad/domain/consensus/model/externalapi" + "github.com/kaspanet/kaspad/domain/consensus/utils/testutils" + "github.com/kaspanet/kaspad/domain/dagconfig" + "github.com/kaspanet/kaspad/infrastructure/db/database" + "github.com/pkg/errors" + "strings" + "testing" +) + +func TestCreateBlockLocator(t *testing.T) { + testutils.ForAllNets(t, true, func(t *testing.T, params *dagconfig.Params) { + factory := consensus.NewFactory() + tc, tearDown, err := factory.NewTestConsensus(params, false, + "TestCreateBlockLocator") + if err != nil { + t.Fatalf("NewTestConsensus: %+v", err) + } + defer tearDown(false) + + chain := []*externalapi.DomainHash{params.GenesisHash} + tipHash := params.GenesisHash + for i := 0; i < 20; i++ { + var err error + tipHash, _, err = tc.AddBlock([]*externalapi.DomainHash{tipHash}, nil, nil) + if err != nil { + t.Fatalf("AddBlock: %+v", err) + } + + chain = append(chain, tipHash) + } + + sideChainTipHash, _, err := tc.AddBlock([]*externalapi.DomainHash{params.GenesisHash}, nil, nil) + if err != nil { + t.Fatalf("AddBlock: %+v", err) + } + + // Check a situation where low hash is not on the exact step blue score + locator, err := tc.CreateBlockLocator(params.GenesisHash, tipHash, 0) + if err != nil { + t.Fatalf("CreateBlockLocator: %+v", err) + } + + if !externalapi.HashesEqual(locator, []*externalapi.DomainHash{ + chain[20], + chain[19], + chain[17], + chain[13], + chain[5], + chain[0], + }) { + t.Fatalf("unexpected block locator %s", locator) + } + + // Check a situation where low hash is on the exact step blue score + locator, err = tc.CreateBlockLocator(chain[5], tipHash, 0) + if err != nil { + t.Fatalf("CreateBlockLocator: %+v", err) + } + + if !externalapi.HashesEqual(locator, []*externalapi.DomainHash{ + chain[20], + chain[19], + chain[17], + chain[13], + chain[5], + }) { + t.Fatalf("unexpected block locator %s", locator) + } + + // Check block locator with limit + locator, err = tc.CreateBlockLocator(params.GenesisHash, tipHash, 3) + if err != nil { + t.Fatalf("CreateBlockLocator: %+v", err) + } + + if !externalapi.HashesEqual(locator, []*externalapi.DomainHash{ + chain[20], + chain[19], + chain[17], + }) { + t.Fatalf("unexpected block locator %s", locator) + } + + // Check a block locator from genesis to genesis + locator, err = tc.CreateBlockLocator(params.GenesisHash, params.GenesisHash, 0) + if err != nil { + t.Fatalf("CreateBlockLocator: %+v", err) + } + + if !externalapi.HashesEqual(locator, []*externalapi.DomainHash{ + params.GenesisHash, + }) { + t.Fatalf("unexpected block locator %s", locator) + } + + // Check a block locator from one block to the same block + locator, err = tc.CreateBlockLocator(chain[7], chain[7], 0) + if err != nil { + t.Fatalf("CreateBlockLocator: %+v", err) + } + + if !externalapi.HashesEqual(locator, []*externalapi.DomainHash{ + chain[7], + }) { + t.Fatalf("unexpected block locator %s", locator) + } + + // Check block locator with incompatible blocks + _, err = tc.CreateBlockLocator(sideChainTipHash, tipHash, 0) + expectedErr := "highHash and lowHash are not in the same selected parent chain" + if err == nil || !strings.Contains(err.Error(), expectedErr) { + t.Fatalf("expected error '%s' but got '%s'", expectedErr, err) + } + + // Check block locator with non exist blocks + _, err = tc.CreateBlockLocator(&externalapi.DomainHash{}, tipHash, 0) + expectedErr = "does not exist" + if err == nil || !strings.Contains(err.Error(), expectedErr) { + t.Fatalf("expected error '%s' but got '%s'", expectedErr, err) + } + + _, err = tc.CreateBlockLocator(tipHash, &externalapi.DomainHash{}, 0) + expectedErr = "does not exist" + if err == nil || !strings.Contains(err.Error(), expectedErr) { + t.Fatalf("expected error '%s' but got '%s'", expectedErr, err) + } + }) +} + +func TestCreateHeadersSelectedChainBlockLocator(t *testing.T) { + testutils.ForAllNets(t, true, func(t *testing.T, params *dagconfig.Params) { + factory := consensus.NewFactory() + tc, tearDown, err := factory.NewTestConsensus(params, false, + "TestCreateHeadersSelectedChainBlockLocator") + if err != nil { + t.Fatalf("NewTestConsensus: %+v", err) + } + defer tearDown(false) + + chain := []*externalapi.DomainHash{params.GenesisHash} + tipHash := params.GenesisHash + for i := 0; i < 20; i++ { + var err error + tipHash, _, err = tc.AddBlock([]*externalapi.DomainHash{tipHash}, nil, nil) + if err != nil { + t.Fatalf("AddBlock: %+v", err) + } + + chain = append(chain, tipHash) + } + + sideChainTipHash, _, err := tc.AddBlock([]*externalapi.DomainHash{params.GenesisHash}, nil, nil) + if err != nil { + t.Fatalf("AddBlock: %+v", err) + } + + // Check a situation where low hash is not on the exact step + locator, err := tc.CreateHeadersSelectedChainBlockLocator(params.GenesisHash, tipHash) + if err != nil { + t.Fatalf("CreateBlockLocator: %+v", err) + } + + if !externalapi.HashesEqual(locator, []*externalapi.DomainHash{ + chain[20], + chain[19], + chain[17], + chain[13], + chain[5], + chain[0], + }) { + t.Fatalf("unexpected block locator %s", locator) + } + + // Check a situation where low hash is on the exact step + locator, err = tc.CreateHeadersSelectedChainBlockLocator(chain[5], tipHash) + if err != nil { + t.Fatalf("CreateBlockLocator: %+v", err) + } + + if !externalapi.HashesEqual(locator, []*externalapi.DomainHash{ + chain[20], + chain[19], + chain[17], + chain[13], + chain[5], + }) { + t.Fatalf("unexpected block locator %s", locator) + } + + // Check a block locator from genesis to genesis + locator, err = tc.CreateHeadersSelectedChainBlockLocator(params.GenesisHash, params.GenesisHash) + if err != nil { + t.Fatalf("CreateBlockLocator: %+v", err) + } + + if !externalapi.HashesEqual(locator, []*externalapi.DomainHash{ + params.GenesisHash, + }) { + t.Fatalf("unexpected block locator %s", locator) + } + + // Check a block locator from one block to the same block + locator, err = tc.CreateHeadersSelectedChainBlockLocator(chain[7], chain[7]) + if err != nil { + t.Fatalf("CreateBlockLocator: %+v", err) + } + + if !externalapi.HashesEqual(locator, []*externalapi.DomainHash{ + chain[7], + }) { + t.Fatalf("unexpected block locator %s", locator) + } + + // Check block locator with low hash higher than high hash + _, err = tc.CreateHeadersSelectedChainBlockLocator(chain[20], chain[19]) + expectedErr := "cannot build block locator while highHash is lower than lowHash" + if err == nil || !strings.Contains(err.Error(), expectedErr) { + t.Fatalf("expected error '%s' but got '%s'", expectedErr, err) + } + + // Check block locator with non chain blocks + _, err = tc.CreateHeadersSelectedChainBlockLocator(params.GenesisHash, sideChainTipHash) + if !errors.Is(err, database.ErrNotFound) { + t.Fatalf("expected error '%s' but got '%s'", database.ErrNotFound, err) + } + }) +} diff --git a/domain/consensus/processes/syncmanager/syncmanager.go b/domain/consensus/processes/syncmanager/syncmanager.go index 48ea10d65..c4f5d7384 100644 --- a/domain/consensus/processes/syncmanager/syncmanager.go +++ b/domain/consensus/processes/syncmanager/syncmanager.go @@ -89,13 +89,6 @@ func (sm *syncManager) CreateHeadersSelectedChainBlockLocator(lowHash, return sm.createHeadersSelectedChainBlockLocator(lowHash, highHash) } -func (sm *syncManager) FindNextBlockLocatorBoundaries(blockLocator externalapi.BlockLocator) (lowHash, highHash *externalapi.DomainHash, err error) { - onEnd := logger.LogAndMeasureExecutionTime(log, "FindNextBlockLocatorBoundaries") - defer onEnd() - - return sm.findNextBlockLocatorBoundaries(blockLocator) -} - func (sm *syncManager) GetSyncInfo() (*externalapi.SyncInfo, error) { onEnd := logger.LogAndMeasureExecutionTime(log, "GetSyncInfo") defer onEnd()