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.

Sign up for GitHub

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

Jump to bottom

erigon-lib: tidy up slices,generics,cmp commons #11216

Merged
merged 12 commits into from
Jul 18, 2024
Merged

erigon-lib: tidy up slices,generics,cmp commons #11216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
bd0306a
eth/stagedsync: polygon sync stage to use new heimdall service
taratorio Jul 16, 2024
76acf7a
introduce txActionStream to fix issues with RoTx not seeing updates
taratorio Jul 17, 2024
f505248
Merge branch 'main' of github.com:ledgerwatch/erigon into poly-sync-s…
taratorio Jul 17, 2024
4d23831
polygon/heimdall: remove old duplicate heimdall component
taratorio Jul 17, 2024
c0165cc
tidy rename
taratorio Jul 17, 2024
9007305
erigon-lib: tidy up slices,generics,cmp commons
taratorio Jul 17, 2024
fee1e2a
tidy comment
taratorio Jul 17, 2024
5a2acd4
remove old heimdall store
taratorio Jul 17, 2024
f51f5ff
Merge branch 'poly-remove-duplicate-old-heimdall' of github.com:ledge…
taratorio Jul 17, 2024
09094da
Merge branch 'main' of github.com:ledgerwatch/erigon into tidy-commons
taratorio Jul 18, 2024
9c1ed2a
add licensing comment
taratorio Jul 18, 2024
d02f989
Merge branch 'main' of github.com:ledgerwatch/erigon into tidy-commons
taratorio Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions core/blockchain.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
package core

import (
"cmp"
"encoding/json"
"fmt"
"reflect"
Expand All @@ -29,17 +30,14 @@ import (

"golang.org/x/crypto/sha3"

"github.com/erigontech/erigon-lib/log/v3"

"github.com/erigontech/erigon-lib/chain"
libcommon "github.com/erigontech/erigon-lib/common"
"github.com/erigontech/erigon-lib/common/cmp"
"github.com/erigontech/erigon-lib/common/dbg"
"github.com/erigontech/erigon-lib/log/v3"
"github.com/erigontech/erigon-lib/metrics"
"github.com/erigontech/erigon/common/math"
"github.com/erigontech/erigon/common/u256"
"github.com/erigontech/erigon/consensus"

"github.com/erigontech/erigon/core/state"
"github.com/erigontech/erigon/core/tracing"
"github.com/erigontech/erigon/core/types"
Expand Down
15 changes: 2 additions & 13 deletions erigon-lib/common/cmp/cmp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
package cmp

import (
"golang.org/x/exp/constraints"
"cmp"
Copy link
Member

Choose a reason for hiding this comment

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

Should we still call our package cmp given the name clash?

Copy link
Member Author

@taratorio taratorio Jul 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@antonis19
what is your suggestion for naming?
I can think of cmpext for "cmp extensions"

Copy link
Member Author

Choose a reason for hiding this comment

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

or cmpx

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with cmpx

Copy link
Member Author

@taratorio taratorio Jul 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

we have the same clash with std math vs erigon-lib/common/math, probably others too
seems like other people also struggle with how to name packages that extend std/other packages - https://stackoverflow.com/questions/58476795/how-to-build-extensions-to-existing-packages-idiomatically

one suggestion there is to use cmputil

Copy link
Member

@antonis19 antonis19 Jul 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think I am actually fine with cmp also. We could always rename when we do the import in case there is an import with the standard cmp. So I will approve and leave it up to you how you want to handle the name.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, I think I'd prefer to leave it as cmp too for consistency
it seems that right now the way we handle clashes is by using aliases
so if std cmp clashes with erigon-lib/common/cmp the dev decides how to name the alias
we should probably have a standard for this but this is outside of the scope of this PR

Copy link
Member

@antonis19 antonis19 Jul 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I agree. Keeping consistent naming makes searching for these kinds of packages simpler. And aliasing helps with the disambiguation.

)

// InRange - ensure val is in [min,max] range
func InRange[T constraints.Ordered](min, max, val T) T {
func InRange[T cmp.Ordered](min, max, val T) T {
antonis19 marked this conversation as resolved.
Show resolved Hide resolved
if min >= val {
return min
}
Expand All @@ -30,14 +30,3 @@ func InRange[T constraints.Ordered](min, max, val T) T {
}
return val
}

func Compare[T constraints.Ordered](a, b T) int {
antonis19 marked this conversation as resolved.
Show resolved Hide resolved
switch {
case a < b:
return -1
case a == b:
return 0
default:
return 1
}
}
6 changes: 0 additions & 6 deletions erigon-lib/common/collections.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@ import (
"math/rand"
)

func SliceReverse[T any](s []T) {
antonis19 marked this conversation as resolved.
Show resolved Hide resolved
for i, j := 0, len(s)-1; i < j; i, j = i+1, j-1 {
s[i], s[j] = s[j], s[i]
}
}

func SliceMap[T any, U any](s []T, mapFunc func(T) U) []U {
out := make([]U, 0, len(s))
for _, x := range s {
Expand Down
15 changes: 5 additions & 10 deletions polygon/bor/finality/generics/generics.go → erigon-lib/common/generics/generics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,11 @@

package generics

import (
libcommon "github.com/erigontech/erigon-lib/common"
"github.com/erigontech/erigon/core/types"
)

func Empty[T any]() (t T) {
return
func Zero[T any]() T {
var value T
return value
}

type Response struct {
Headers []*types.Header
Hashes []libcommon.Hash
func New[T any]() *T {
return new(T)
}
2 changes: 1 addition & 1 deletion erigon-lib/downloader/snaptype/files.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package snaptype

import (
"cmp"
"encoding/hex"
"errors"
"fmt"
Expand All @@ -29,7 +30,6 @@ import (

"github.com/anacrolix/torrent/metainfo"

"github.com/erigontech/erigon-lib/common/cmp"
"github.com/erigontech/erigon-lib/common/dir"
)

Expand Down
2 changes: 1 addition & 1 deletion erigon-lib/seg/compress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package seg
import (
"bufio"
"bytes"
"cmp"
"container/heap"
"context"
"encoding/binary"
Expand All @@ -35,7 +36,6 @@ import (
"github.com/c2h5oh/datasize"

"github.com/erigontech/erigon-lib/common"
"github.com/erigontech/erigon-lib/common/cmp"
dir2 "github.com/erigontech/erigon-lib/common/dir"
"github.com/erigontech/erigon-lib/etl"
"github.com/erigontech/erigon-lib/log/v3"
Expand Down
2 changes: 1 addition & 1 deletion erigon-lib/seg/patricia/patricia.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@
package patricia

import (
"cmp"
"fmt"
"math/bits"
"slices"
"strings"

"github.com/erigontech/erigon-lib/common/cmp"
"github.com/erigontech/erigon-lib/seg/sais"
)

Expand Down
7 changes: 3 additions & 4 deletions eth/stagedsync/stage_polygon_sync.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (

"github.com/erigontech/erigon-lib/chain"
"github.com/erigontech/erigon-lib/common"
"github.com/erigontech/erigon-lib/common/generics"
"github.com/erigontech/erigon-lib/common/metrics"
"github.com/erigontech/erigon-lib/direct"
"github.com/erigontech/erigon-lib/kv"
Expand Down Expand Up @@ -951,16 +952,14 @@ func awaitTxAction[T any](

select {
case <-ctx.Done():
var nilValue T
return nilValue, ctx.Err()
return generics.Zero[T](), ctx.Err()
case txActionStream <- txAction:
// no-op
}

select {
case <-ctx.Done():
var nilValue T
return nilValue, ctx.Err()
return generics.Zero[T](), ctx.Err()
case resp := <-responseStream:
return resp, nil
}
Expand Down
4 changes: 2 additions & 2 deletions polygon/bor/finality/rawdb/milestone.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ import (
"fmt"

libcommon "github.com/erigontech/erigon-lib/common"
"github.com/erigontech/erigon-lib/common/generics"
"github.com/erigontech/erigon-lib/kv"
"github.com/erigontech/erigon-lib/log/v3"
"github.com/erigontech/erigon/polygon/bor/finality/generics"
)

var (
Expand Down Expand Up @@ -130,7 +130,7 @@ type BlockFinality[T any] interface {
}

func getKey[T BlockFinality[T]]() (T, []byte) {
lastT := generics.Empty[T]().clone()
lastT := generics.Zero[T]().clone()

var key []byte

Expand Down
21 changes: 7 additions & 14 deletions polygon/heimdall/entity_store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"encoding/json"
"sync"

"github.com/erigontech/erigon-lib/common/generics"
"github.com/erigontech/erigon-lib/kv"
"github.com/erigontech/erigon-lib/kv/stream"
"github.com/erigontech/erigon/polygon/polygoncommon"
Expand Down Expand Up @@ -121,22 +122,14 @@ func (s *mdbxEntityStore[TEntity]) GetLastEntityId(ctx context.Context) (uint64,
return entityStoreKeyParse(lastKey), true, nil
}

// Zero value of any type T
// https://stackoverflow.com/questions/70585852/return-default-value-for-generic-type)
// https://go.dev/ref/spec#The_zero_value
func Zero[T any]() T {
var value T
return value
}

func (s *mdbxEntityStore[TEntity]) GetLastEntity(ctx context.Context) (TEntity, error) {
id, ok, err := s.GetLastEntityId(ctx)
if err != nil {
return Zero[TEntity](), err
return generics.Zero[TEntity](), err
}
// not found
if !ok {
return Zero[TEntity](), nil
return generics.Zero[TEntity](), nil
}
return s.GetEntity(ctx, id)
}
Expand All @@ -154,26 +147,26 @@ func entityStoreKeyParse(key []byte) uint64 {
func (s *mdbxEntityStore[TEntity]) entityUnmarshalJSON(jsonBytes []byte) (TEntity, error) {
entity := s.makeEntity()
if err := json.Unmarshal(jsonBytes, entity); err != nil {
return Zero[TEntity](), err
return generics.Zero[TEntity](), err
}
return entity, nil
}

func (s *mdbxEntityStore[TEntity]) GetEntity(ctx context.Context, id uint64) (TEntity, error) {
tx, err := s.db.BeginRo(ctx)
if err != nil {
return Zero[TEntity](), err
return generics.Zero[TEntity](), err
}
defer tx.Rollback()

key := entityStoreKey(id)
jsonBytes, err := tx.GetOne(s.table, key[:])
if err != nil {
return Zero[TEntity](), err
return generics.Zero[TEntity](), err
}
// not found
if jsonBytes == nil {
return Zero[TEntity](), nil
return generics.Zero[TEntity](), nil
}

return s.entityUnmarshalJSON(jsonBytes)
Expand Down
7 changes: 2 additions & 5 deletions polygon/heimdall/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package heimdall
import (
"context"
"errors"
"slices"
"time"

"golang.org/x/sync/errgroup"
Expand All @@ -44,10 +45,6 @@ type service struct {
spanScraper *scraper[*Span]
}

func makeType[T any]() *T {
return new(T)
}

func AssembleService(heimdallUrl string, dataDir string, tmpDir string, logger log.Logger) Service {
store := NewMdbxServiceStore(logger, dataDir, tmpDir)
client := NewHeimdallClient(heimdallUrl, logger)
Expand Down Expand Up @@ -170,7 +167,7 @@ func (s *service) FetchLatestSpans(ctx context.Context, count uint) ([]*Span, er
count--
}

libcommon.SliceReverse(latestSpans)
slices.Reverse(latestSpans)
return latestSpans, nil
}

Expand Down
7 changes: 4 additions & 3 deletions polygon/heimdall/service_store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

"golang.org/x/sync/errgroup"

"github.com/erigontech/erigon-lib/common/generics"
"github.com/erigontech/erigon-lib/kv"
"github.com/erigontech/erigon-lib/log/v3"
"github.com/erigontech/erigon/polygon/polygoncommon"
Expand All @@ -42,9 +43,9 @@ func NewMdbxServiceStore(logger log.Logger, dataDir string, tmpDir string) *Mdbx

return &MdbxServiceStore{
db: db,
checkpoints: newMdbxEntityStore(db, kv.HeimdallDB, kv.BorCheckpoints, makeType[Checkpoint], blockNumToIdIndexFactory),
milestones: newMdbxEntityStore(db, kv.HeimdallDB, kv.BorMilestones, makeType[Milestone], blockNumToIdIndexFactory),
spans: newMdbxEntityStore(db, kv.HeimdallDB, kv.BorSpans, makeType[Span], blockNumToIdIndexFactory),
checkpoints: newMdbxEntityStore(db, kv.HeimdallDB, kv.BorCheckpoints, generics.New[Checkpoint], blockNumToIdIndexFactory),
milestones: newMdbxEntityStore(db, kv.HeimdallDB, kv.BorMilestones, generics.New[Milestone], blockNumToIdIndexFactory),
spans: newMdbxEntityStore(db, kv.HeimdallDB, kv.BorSpans, generics.New[Span], blockNumToIdIndexFactory),
}
}

Expand Down
13 changes: 6 additions & 7 deletions polygon/p2p/fetcher_base.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/cenkalti/backoff/v4"

"github.com/erigontech/erigon-lib/common"
"github.com/erigontech/erigon-lib/common/generics"
"github.com/erigontech/erigon/core/types"
"github.com/erigontech/erigon/eth/protocols/eth"
)
Expand Down Expand Up @@ -332,21 +333,19 @@ func fetchWithRetry[TData any](config FetcherConfig, fetch func() (TData, error)
data, err := backoff.RetryWithData(func() (TData, error) {
data, err := fetch()
if err != nil {
var nilData TData
// retry timeouts
if errors.Is(err, context.DeadlineExceeded) {
return nilData, err
return generics.Zero[TData](), err
}

// permanent errors are not retried
return nilData, backoff.Permanent(err)
return generics.Zero[TData](), backoff.Permanent(err)
}

return data, nil
}, backoff.WithMaxRetries(backoff.NewConstantBackOff(config.retryBackOff), config.maxRetries))
if err != nil {
var nilData TData
return nilData, err
return generics.Zero[TData](), err
}

return data, nil
Expand All @@ -364,8 +363,8 @@ func awaitResponse[TPacket any](
for {
select {
case <-ctx.Done():
var nilPacket TPacket
return nilPacket, 0, fmt.Errorf("await %v response interrupted: %w", reflect.TypeOf(nilPacket), ctx.Err())
packet := generics.Zero[TPacket]()
return packet, 0, fmt.Errorf("await %v response interrupted: %w", reflect.TypeOf(packet), ctx.Err())
case message := <-messages:
if filter(message) {
continue
Expand Down
7 changes: 3 additions & 4 deletions polygon/p2p/message_listener_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ import (
"google.golang.org/grpc/metadata"
"google.golang.org/protobuf/types/known/emptypb"

"github.com/erigontech/erigon-lib/log/v3"

"github.com/erigontech/erigon-lib/common"
"github.com/erigontech/erigon-lib/common/generics"
"github.com/erigontech/erigon-lib/direct"
sentry "github.com/erigontech/erigon-lib/gointerfaces/sentryproto"
"github.com/erigontech/erigon-lib/log/v3"
"github.com/erigontech/erigon/core/types"
"github.com/erigontech/erigon/eth/protocols/eth"
sentrymulticlient "github.com/erigontech/erigon/p2p/sentry/sentry_multi_client"
Expand Down Expand Up @@ -335,8 +335,7 @@ type mockSentryMessagesStream[M any] struct {
}

func (s *mockSentryMessagesStream[M]) Recv() (M, error) {
var nilValue M
return nilValue, nil
return generics.Zero[M](), nil
}

func (s *mockSentryMessagesStream[M]) Header() (metadata.MD, error) {
Expand Down
3 changes: 2 additions & 1 deletion polygon/sync/canonical_chain_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"bytes"
"errors"
"fmt"
"slices"
"time"

libcommon "github.com/erigontech/erigon-lib/common"
Expand Down Expand Up @@ -133,7 +134,7 @@ func (ccb *canonicalChainBuilder) Headers() []*types.Header {
headers = append(headers, node.header)
node = node.parent
}
libcommon.SliceReverse(headers)
slices.Reverse(headers)
return headers
}

Expand Down
Loading
Loading