diff --git a/discovery/db_discovery.go b/discovery/db_discovery.go index 0dbc0aad0a..330290cfd0 100644 --- a/discovery/db_discovery.go +++ b/discovery/db_discovery.go @@ -2,7 +2,6 @@ package discovery import ( "context" - "errors" "fmt" "math/big" "net/url" @@ -256,11 +255,18 @@ func (dbo *DBOrchestratorPoolCache) cacheDBOrchs() error { errc <- err return } - if info.PriceInfo == nil || info.PriceInfo.GetPixelsPerUnit() <= 0 { - errc <- errors.New("invalid price from orchestrator") + price, err := common.RatPriceInfo(info.PriceInfo) + if err != nil { + errc <- fmt.Errorf("invalid price info orch=%v err=%v", info.GetTranscoder(), err) + return + } + // PriceToFixed also checks if the input is nil, but this check tells us + // which orch was missing price info + if price == nil { + errc <- fmt.Errorf("missing price info orch=%v", info.GetTranscoder()) return } - dbOrch.PricePerPixel, err = common.PriceToFixed(big.NewRat(info.PriceInfo.GetPricePerUnit(), info.PriceInfo.GetPixelsPerUnit())) + dbOrch.PricePerPixel, err = common.PriceToFixed(price) if err != nil { errc <- err return diff --git a/discovery/discovery_test.go b/discovery/discovery_test.go index 6011850172..0c9c1e883e 100644 --- a/discovery/discovery_test.go +++ b/discovery/discovery_test.go @@ -179,6 +179,59 @@ func TestDBOrchestratorPoolCacheSize(t *testing.T) { assert.Equal(len(addresses), nonEmptyPool.Size()) } +func TestNewDBOrchestratorPoolCache_InvalidPrices(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + + priceInfo := &net.PriceInfo{ + PricePerUnit: 0, + PixelsPerUnit: 0, + } + + oldServerGetOrchInfo := serverGetOrchInfo + defer func() { serverGetOrchInfo = oldServerGetOrchInfo }() + var mu sync.Mutex + serverGetOrchInfo = func(ctx context.Context, bcast common.Broadcaster, orchestratorServer *url.URL) (*net.OrchestratorInfo, error) { + mu.Lock() + defer mu.Unlock() + + return &net.OrchestratorInfo{ + Transcoder: "transcoder", + PriceInfo: priceInfo, + }, nil + } + + dbh, dbraw, err := common.TempDB(t) + defer dbh.Close() + defer dbraw.Close() + require.Nil(err) + + // Populate test DB with a stub orch so that serverGetOrchInfo will be called in cacheDBOrchs + orch := common.NewDBOrch("foo", "foo", 0, 0, 1000000000000, 100) + require.Nil(dbh.UpdateOrch(orch)) + + node := &core.LivepeerNode{ + Database: dbh, + Eth: ð.StubClient{}, + } + // LastInitializedRound() will ensure the stub orch is always returned from the DB + rm := &stubRoundsManager{round: big.NewInt(100)} + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + pool, err := NewDBOrchestratorPoolCache(ctx, node, rm) + require.Nil(err) + + // priceInfo.PixelsPerUnit = 0 + // Check that this does not trigger a division by zero + assert.Nil(pool.cacheDBOrchs()) + + // priceInfo = nil + // Check that this does not trigger a nil pointer error + priceInfo = nil + assert.Nil(pool.cacheDBOrchs()) +} + func TestNewDBOrchestratorPoolCache_GivenListOfOrchs_CreatesPoolCacheCorrectly(t *testing.T) { expPriceInfo := &net.PriceInfo{ PricePerUnit: 999,