Skip to content

Commit

Permalink
chore: address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
EclesioMeloJunior committed Sep 26, 2024
1 parent 87af0f8 commit 29b6a0a
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 60 deletions.
4 changes: 0 additions & 4 deletions dot/network/notifications.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,6 @@ func (s *Service) handleHandshake(info *notificationsProtocol, stream network.St

logger.Tracef("receiver: sent handshake to peer %s using protocol %s", peer, info.protocolID)

// if err := stream.CloseWrite(); err != nil {
// return fmt.Errorf("failed to close stream for writing: %s", err)
// }

return nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func newBlockImporter(cfg *FullSyncConfig) *blockImporter {
}
}

func (b *blockImporter) handle(bd *types.BlockData, origin BlockOrigin) (imported bool, err error) {
func (b *blockImporter) importBlock(bd *types.BlockData, origin BlockOrigin) (imported bool, err error) {
blockAlreadyExists, err := b.blockState.HasHeader(bd.Hash)
if err != nil && !errors.Is(err, database.ErrNotFound) {
return false, err
Expand Down
20 changes: 9 additions & 11 deletions dot/sync/fullsync.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ type FullSyncConfig struct {
RequestMaker network.RequestMaker
}

type Importer interface {
handle(*types.BlockData, BlockOrigin) (imported bool, err error)
type importer interface {
importBlock(*types.BlockData, BlockOrigin) (imported bool, err error)
}

// FullSyncStrategy protocol is the "default" protocol.
Expand All @@ -61,7 +61,7 @@ type FullSyncStrategy struct {
numOfTasks int
startedAt time.Time
syncedBlocks int
importer Importer
blockImporter importer
}

func NewFullSyncStrategy(cfg *FullSyncConfig) *FullSyncStrategy {
Expand All @@ -74,7 +74,7 @@ func NewFullSyncStrategy(cfg *FullSyncConfig) *FullSyncStrategy {
reqMaker: cfg.RequestMaker,
blockState: cfg.BlockState,
numOfTasks: cfg.NumOfTasks,
importer: newBlockImporter(cfg),
blockImporter: newBlockImporter(cfg),
unreadyBlocks: newUnreadyBlocks(),
requestQueue: &requestsQueue[*messages.BlockRequestMessage]{
queue: list.New(),
Expand Down Expand Up @@ -183,8 +183,8 @@ func (f *FullSyncStrategy) Process(results []*SyncTaskResult) (

// disjoint fragments are pieces of the chain that could not be imported right now
// because is blocks too far ahead or blocks that belongs to forks
orderedFragments := sortFragmentsOfChain(readyBlocks)
orderedFragments = mergeFragmentsOfChain(orderedFragments)
sortFragmentsOfChain(readyBlocks)
orderedFragments := mergeFragmentsOfChain(readyBlocks)

nextBlocksToImport := make([]*types.BlockData, 0)
disjointFragments := make([][]*types.BlockData, 0)
Expand All @@ -206,7 +206,7 @@ func (f *FullSyncStrategy) Process(results []*SyncTaskResult) (
// this loop goal is to import ready blocks as well as update the highestFinalized header
for len(nextBlocksToImport) > 0 || len(disjointFragments) > 0 {
for _, blockToImport := range nextBlocksToImport {
imported, err := f.importer.handle(blockToImport, networkInitialSync)
imported, err := f.blockImporter.importBlock(blockToImport, networkInitialSync)
if err != nil {
return false, nil, nil, fmt.Errorf("while handling ready block: %w", err)
}
Expand Down Expand Up @@ -486,9 +486,9 @@ resultLoop:
// note that we have fragments with single blocks, fragments with fork (in case of 8)
// after sorting these fragments we end up with:
// [ {1, 2, 3, 4, 5} {6, 7, 8, 9, 10} {8} {11, 12, 13, 14, 15, 16} {17} ]
func sortFragmentsOfChain(fragments [][]*types.BlockData) [][]*types.BlockData {
func sortFragmentsOfChain(fragments [][]*types.BlockData) {
if len(fragments) == 0 {
return nil
return
}

slices.SortFunc(fragments, func(a, b []*types.BlockData) int {
Expand All @@ -500,8 +500,6 @@ func sortFragmentsOfChain(fragments [][]*types.BlockData) [][]*types.BlockData {
}
return 1
})

return fragments
}

// mergeFragmentsOfChain expects a sorted slice of fragments and merges those
Expand Down
8 changes: 5 additions & 3 deletions dot/sync/fullsync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
_ "embed"
)

type mockBlockImporter struct{}

Check failure on line 23 in dot/sync/fullsync_test.go

View workflow job for this annotation

GitHub Actions / linting

type `mockBlockImporter` is unused (unused)

//go:embed testdata/westend_blocks.yaml
var rawWestendBlocks []byte

Expand Down Expand Up @@ -234,9 +236,9 @@ func TestFullSyncProcess(t *testing.T) {
Return(false, nil).
Times(2)

mockImporter := NewMockImporter(ctrl)
mockImporter := NewMockimporter(ctrl)
mockImporter.EXPECT().
handle(gomock.AssignableToTypeOf(&types.BlockData{}), networkInitialSync).
importBlock(gomock.AssignableToTypeOf(&types.BlockData{}), networkInitialSync).
Return(true, nil).
Times(10 + 128 + 128)

Expand All @@ -245,7 +247,7 @@ func TestFullSyncProcess(t *testing.T) {
}

fs := NewFullSyncStrategy(cfg)
fs.importer = mockImporter
fs.blockImporter = mockImporter

done, _, _, err := fs.Process(syncTaskResults)
require.NoError(t, err)
Expand Down
55 changes: 55 additions & 0 deletions dot/sync/mock_importer.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion dot/sync/mocks_generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@

package sync

//go:generate mockgen -destination=mocks_test.go -package=$GOPACKAGE . Telemetry,BlockState,StorageState,TransactionState,BabeVerifier,FinalityGadget,BlockImportHandler,Network,Importer
//go:generate mockgen -destination=mocks_test.go -package=$GOPACKAGE . Telemetry,BlockState,StorageState,TransactionState,BabeVerifier,FinalityGadget,BlockImportHandler,Network
//go:generate mockgen -destination=mock_request_maker.go -package $GOPACKAGE github.com/ChainSafe/gossamer/dot/network RequestMaker
//go:generate mockgen -destination=mock_importer.go -source=fullsync.go -package=sync
42 changes: 2 additions & 40 deletions dot/sync/mocks_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 29b6a0a

Please sign in to comment.