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.

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

[Merged by Bors] - chore: refactor panics into error handling #6345

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion activation/e2e/activation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ func Test_BuilderWithMultipleClients(t *testing.T) {
)
certClient := activation.NewCertifierClient(db, localDB, logger.Named("certifier"))
certifier := activation.NewCertifier(localDB, logger, certClient)
poetDb := activation.NewPoetDb(db, logger.Named("poetDb"))
poetDb, err := activation.NewPoetDb(db, logger.Named("poetDb"))
require.NoError(t, err)
client, err := poetProver.Client(poetDb, poetCfg, logger, activation.WithCertifier(certifier))
require.NoError(t, err)

Expand Down
3 changes: 2 additions & 1 deletion activation/e2e/atx_merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,8 @@ func Test_MarryAndMerge(t *testing.T) {
verifier, err := activation.NewPostVerifier(cfg, logger, activation.WithVerifyingOpts(verifyingOpts))
require.NoError(t, err)
t.Cleanup(func() { assert.NoError(t, verifier.Close()) })
poetDb := activation.NewPoetDb(db, logger.Named("poetDb"))
poetDb, err := activation.NewPoetDb(db, logger.Named("poetDb"))
require.NoError(t, err)
validator := activation.NewValidator(db, poetDb, cfg, opts.Scrypt, verifier)

eg, ctx := errgroup.WithContext(context.Background())
Expand Down
3 changes: 2 additions & 1 deletion activation/e2e/builds_atx_v2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ func TestBuilder_SwitchesToBuildV2(t *testing.T) {

initPost(t, cfg, opts, sig, goldenATX, grpcCfg, svc)

poetDb := activation.NewPoetDb(db, logger.Named("poetDb"))
poetDb, err := activation.NewPoetDb(db, logger.Named("poetDb"))
require.NoError(t, err)
verifier, err := activation.NewPostVerifier(cfg, logger.Named("verifier"))
require.NoError(t, err)
t.Cleanup(func() { assert.NoError(t, verifier.Close()) })
Expand Down
6 changes: 4 additions & 2 deletions activation/e2e/checkpoint_merged_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ func Test_CheckpointAfterMerge(t *testing.T) {
verifier, err := activation.NewPostVerifier(cfg, logger, activation.WithVerifyingOpts(verifyingOpts))
require.NoError(t, err)
t.Cleanup(func() { assert.NoError(t, verifier.Close()) })
poetDb := activation.NewPoetDb(db, logger.Named("poetDb"))
poetDb, err := activation.NewPoetDb(db, logger.Named("poetDb"))
require.NoError(t, err)
validator := activation.NewValidator(db, poetDb, cfg, opts.Scrypt, verifier)

eg, ctx := errgroup.WithContext(context.Background())
Expand Down Expand Up @@ -280,7 +281,8 @@ func Test_CheckpointAfterMerge(t *testing.T) {
require.Equal(t, marriageATX.ID(), *checkpointedMerged.MarriageATX)

// 4. Spawn new ATX handler and builder using the new DB
poetDb = activation.NewPoetDb(newDB, logger.Named("poetDb"))
poetDb, err = activation.NewPoetDb(newDB, logger.Named("poetDb"))
require.NoError(t, err)
cdb = datastore.NewCachedDB(newDB, logger)

poetSvc = activation.NewPoetServiceWithClient(poetDb, client, poetCfg, logger)
Expand Down
6 changes: 4 additions & 2 deletions activation/e2e/checkpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ func TestCheckpoint_PublishingSoloATXs(t *testing.T) {
initPost(t, cfg, opts, sig, goldenATX, grpcCfg, svc)
syncer := syncedSyncer(t)

poetDb := activation.NewPoetDb(db, logger.Named("poetDb"))
poetDb, err := activation.NewPoetDb(db, logger.Named("poetDb"))
require.NoError(t, err)
verifier, err := activation.NewPostVerifier(cfg, logger.Named("verifier"))
require.NoError(t, err)
t.Cleanup(func() { assert.NoError(t, verifier.Close()) })
Expand Down Expand Up @@ -186,7 +187,8 @@ func TestCheckpoint_PublishingSoloATXs(t *testing.T) {
defer newDB.Close()

// 3. Spawn new ATX handler and builder using the new DB
poetDb = activation.NewPoetDb(newDB, logger.Named("poetDb"))
poetDb, err = activation.NewPoetDb(newDB, logger.Named("poetDb"))
require.NoError(t, err)
cdb = datastore.NewCachedDB(newDB, logger)
atxdata, err = atxsdata.Warm(newDB, 1, logger)
poetService = activation.NewPoetServiceWithClient(poetDb, client, poetCfg, logger)
Expand Down
6 changes: 4 additions & 2 deletions activation/e2e/nipost_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,8 @@ func TestNIPostBuilderWithClients(t *testing.T) {
require.NoError(t, err)
t.Cleanup(func() { assert.NoError(t, verifier.Close()) })

poetDb := activation.NewPoetDb(db, logger.Named("poetDb"))
poetDb, err := activation.NewPoetDb(db, logger.Named("poetDb"))
require.NoError(t, err)

postClient, err := svc.Client(sig.NodeID())
require.NoError(t, err)
Expand Down Expand Up @@ -271,7 +272,8 @@ func Test_NIPostBuilderWithMultipleClients(t *testing.T) {
GracePeriod: epoch / 4,
}

poetDb := activation.NewPoetDb(db, logger.Named("poetDb"))
poetDb, err := activation.NewPoetDb(db, logger.Named("poetDb"))
require.NoError(t, err)
client := ae2e.NewTestPoetClient(len(signers), poetCfg)
poetService := activation.NewPoetServiceWithClient(poetDb, client, poetCfg, logger)

Expand Down
3 changes: 2 additions & 1 deletion activation/e2e/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ func TestValidator_Validate(t *testing.T) {
GracePeriod: epoch / 4,
}

poetDb := activation.NewPoetDb(statesql.InMemory(), logger.Named("poetDb"))
poetDb, err := activation.NewPoetDb(statesql.InMemory(), logger.Named("poetDb"))
require.NoError(t, err)
client := ae2e.NewTestPoetClient(1, poetCfg)
poetService := activation.NewPoetServiceWithClient(poetDb, client, poetCfg, logger)

Expand Down
3 changes: 2 additions & 1 deletion activation/poet_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,8 @@ func TestPoetService_CachesCertifierInfo(t *testing.T) {

cfg := DefaultPoetConfig()
cfg.InfoCacheTTL = tc.ttl
db := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
db, err := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
require.NoError(t, err)

url := &url.URL{Host: "certifier.hello"}
pubkey := []byte("pubkey")
Expand Down
6 changes: 3 additions & 3 deletions activation/poetdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
}

// NewPoetDb returns a new PoET handler.
func NewPoetDb(db sql.StateDatabase, log *zap.Logger, opts ...PoetDbOption) *PoetDb {
func NewPoetDb(db sql.StateDatabase, log *zap.Logger, opts ...PoetDbOption) (*PoetDb, error) {
options := PoetDbOptions{
// in last epochs there are 45 proofs per epoch, with each of them nearly 140KB
// 200 is set not to keep multiple epochs, but to account for unexpected growth
Expand All @@ -60,13 +60,13 @@
}
poetProofsLru, err := lru.New[types.PoetProofRef, *types.PoetProofMessage](options.cacheSize)
if err != nil {
log.Panic("failed to create PoET proofs LRU cache", zap.Error(err))
return nil, fmt.Errorf("create lru: %w", err)

Check warning on line 63 in activation/poetdb.go

View check run for this annotation

Codecov / codecov/patch

activation/poetdb.go#L63

Added line #L63 was not covered by tests
}
return &PoetDb{
sqlDB: db,
poetProofsLru: poetProofsLru,
logger: log,
}
}, nil
}

// HasProof returns true if the database contains a proof with the given reference, or false otherwise.
Expand Down
18 changes: 11 additions & 7 deletions activation/poetdb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ func getPoetProof(t *testing.T) types.PoetProofMessage {
func TestPoetDbHappyFlow(t *testing.T) {
r := require.New(t)
msg := getPoetProof(t)
poetDb := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
poetDb, err := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
r.NoError(err)

r.NoError(poetDb.Validate(msg.Statement[:], msg.PoetProof, msg.PoetServiceID, msg.RoundID, types.EmptyEdSignature))
ref, err := msg.Ref()
Expand All @@ -83,10 +84,11 @@ func TestPoetDbHappyFlow(t *testing.T) {
func TestPoetDbInvalidPoetProof(t *testing.T) {
r := require.New(t)
msg := getPoetProof(t)
poetDb := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
poetDb, err := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
r.NoError(err)
msg.PoetProof.Root = []byte("some other root")

err := poetDb.Validate(msg.Statement[:], msg.PoetProof, msg.PoetServiceID, msg.RoundID, types.EmptyEdSignature)
err = poetDb.Validate(msg.Statement[:], msg.PoetProof, msg.PoetServiceID, msg.RoundID, types.EmptyEdSignature)
r.EqualError(
err,
fmt.Sprintf(
Expand All @@ -99,10 +101,11 @@ func TestPoetDbInvalidPoetProof(t *testing.T) {
func TestPoetDbInvalidPoetStatement(t *testing.T) {
r := require.New(t)
msg := getPoetProof(t)
poetDb := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
poetDb, err := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
r.NoError(err)
msg.Statement = types.CalcHash32([]byte("some other statement"))

err := poetDb.Validate(msg.Statement[:], msg.PoetProof, msg.PoetServiceID, msg.RoundID, types.EmptyEdSignature)
err = poetDb.Validate(msg.Statement[:], msg.PoetProof, msg.PoetServiceID, msg.RoundID, types.EmptyEdSignature)
r.EqualError(
err,
fmt.Sprintf(
Expand All @@ -115,9 +118,10 @@ func TestPoetDbInvalidPoetStatement(t *testing.T) {
func TestPoetDbNonExistingKeys(t *testing.T) {
r := require.New(t)
msg := getPoetProof(t)
poetDb := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
poetDb, err := NewPoetDb(statesql.InMemory(), zaptest.NewLogger(t))
r.NoError(err)

_, err := poetDb.GetProofRef(msg.PoetServiceID, "0")
_, err = poetDb.GetProofRef(msg.PoetServiceID, "0")
r.EqualError(
err,
fmt.Sprintf(
Expand Down
8 changes: 4 additions & 4 deletions fetch/cache.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package fetch

import (
"fmt"
"math/rand/v2"
"sync"

Expand All @@ -9,7 +10,6 @@

"github.com/spacemeshos/go-spacemesh/common/types"
"github.com/spacemeshos/go-spacemesh/datastore"
"github.com/spacemeshos/go-spacemesh/log"
"github.com/spacemeshos/go-spacemesh/p2p"
)

Expand All @@ -24,12 +24,12 @@
type HashPeers map[p2p.Peer]struct{}

// NewHashPeersCache creates a new hash-to-peers cache.
func NewHashPeersCache(size int) *HashPeersCache {
func NewHashPeersCache(size int) (*HashPeersCache, error) {
cache, err := lru.New[types.Hash32, HashPeers](size)
if err != nil {
log.Panic("could not initialize cache ", err)
return nil, fmt.Errorf("create lru: %w", err)

Check warning on line 30 in fetch/cache.go

View check run for this annotation

Codecov / codecov/patch

fetch/cache.go#L30

Added line #L30 was not covered by tests
}
return &HashPeersCache{Cache: cache}
return &HashPeersCache{Cache: cache}, nil
}

// get returns peers for a given hash (non-thread-safe).
Expand Down
21 changes: 14 additions & 7 deletions fetch/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ func getCachedEntry(cache *HashPeersCache, hash types.Hash32) (HashPeers, bool)
func TestAdd(t *testing.T) {
t.Parallel()
t.Run("1Hash3Peers", func(t *testing.T) {
cache := NewHashPeersCache(10)
cache, err := NewHashPeersCache(10)
require.NoError(t, err)
hash := types.RandomHash()
peer1 := p2p.Peer("test_peer_1")
peer2 := p2p.Peer("test_peer_2")
Expand All @@ -49,7 +50,8 @@ func TestAdd(t *testing.T) {
require.Len(t, hashPeers, 3)
})
t.Run("2Hashes1Peer", func(t *testing.T) {
cache := NewHashPeersCache(10)
cache, err := NewHashPeersCache(10)
require.NoError(t, err)
hash1 := types.RandomHash()
hash2 := types.RandomHash()
peer := p2p.Peer("test_peer")
Expand All @@ -74,7 +76,8 @@ func TestAdd(t *testing.T) {
func TestGetRandom(t *testing.T) {
t.Parallel()
t.Run("no hash peers", func(t *testing.T) {
cache := NewHashPeersCache(10)
cache, err := NewHashPeersCache(10)
require.NoError(t, err)
hash := types.RandomHash()
var seed [32]byte
binary.LittleEndian.PutUint64(seed[:], uint64(time.Now().UnixNano()))
Expand All @@ -83,7 +86,8 @@ func TestGetRandom(t *testing.T) {
require.Empty(t, peers)
})
t.Run("1Hash3Peers", func(t *testing.T) {
cache := NewHashPeersCache(10)
cache, err := NewHashPeersCache(10)
require.NoError(t, err)
hash := types.RandomHash()
peer1 := p2p.Peer("test_peer_1")
peer2 := p2p.Peer("test_peer_2")
Expand All @@ -110,7 +114,8 @@ func TestGetRandom(t *testing.T) {
require.ElementsMatch(t, []p2p.Peer{peer1, peer2, peer3}, peers)
})
t.Run("2Hashes1Peer", func(t *testing.T) {
cache := NewHashPeersCache(10)
cache, err := NewHashPeersCache(10)
require.NoError(t, err)
hash1 := types.RandomHash()
hash2 := types.RandomHash()
peer := p2p.Peer("test_peer")
Expand Down Expand Up @@ -138,7 +143,8 @@ func TestGetRandom(t *testing.T) {
func TestRegisterPeerHashes(t *testing.T) {
t.Parallel()
t.Run("1Hash2Peers", func(t *testing.T) {
cache := NewHashPeersCache(10)
cache, err := NewHashPeersCache(10)
require.NoError(t, err)
hash1 := types.RandomHash()
hash2 := types.RandomHash()
hash3 := types.RandomHash()
Expand All @@ -154,7 +160,8 @@ func TestRegisterPeerHashes(t *testing.T) {
}

func TestRace(t *testing.T) {
cache := NewHashPeersCache(10)
cache, err := NewHashPeersCache(10)
require.NoError(t, err)
hash := types.RandomHash()
peer1 := p2p.Peer("test_peer_1")
peer2 := p2p.Peer("test_peer_2")
Expand Down
12 changes: 9 additions & 3 deletions fetch/fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"context"
"encoding/binary"
"errors"
"fmt"
"io"
"math/rand/v2"
"sync"
Expand Down Expand Up @@ -268,9 +269,14 @@
proposals *store.Store,
host *p2p.Host,
opts ...Option,
) *Fetch {
) (*Fetch, error) {
bs := datastore.NewBlobStore(cdb, proposals)

hashPeerCache, err := NewHashPeersCache(cacheSize)
if err != nil {
// this should never happen, since cacheSize is a constant, but just in case
panic(fmt.Errorf("create hash peer cache: %w", err))

Check warning on line 278 in fetch/fetch.go

View check run for this annotation

Codecov / codecov/patch

fetch/fetch.go#L278

Added line #L278 was not covered by tests
}
f := &Fetch{
cfg: DefaultConfig(),
logger: zap.NewNop(),
Expand All @@ -279,7 +285,7 @@
servers: map[string]requester{},
unprocessed: make(map[types.Hash32]*request),
ongoing: make(map[types.Hash32]*request),
hashToPeers: NewHashPeersCache(cacheSize),
hashToPeers: hashPeerCache,
}
for _, opt := range opts {
opt(f)
Expand Down Expand Up @@ -341,7 +347,7 @@
f.registerServer(host, lyrDataProtocol, server.WrapHandler(h.handleLayerDataReq))
f.registerServer(host, OpnProtocol, server.WrapHandler(h.handleLayerOpinionsReq2))
}
return f
return f, nil
}

func (f *Fetch) registerServer(
Expand Down
Loading
Loading