core,discovery: add checks for PriceInfo.pixelsPerUnit==0

CHANGELOG_PENDING.md

@@ -16,10 +16,12 @@
 
 #### General
 
+- \#1813 Check that priceInfo.pixelsPerUnit is not 0 (@kyriediculous)
+
 #### Broadcaster
 
 - \#1782 Fix SegsInFlight data-loss on refreshing O sessions (@darkdragon)
 
 #### Transcoder
 
-[Full list of changes](https://github.com/livepeer/go-livepeer/compare/v0.5.15...v0.5.16)
+[Full list of changes](https://github.com/livepeer/go-livepeer/compare/v0.5.15...v0.5.16)

core/orch_test.go

@@ -1280,6 +1280,15 @@ func TestTicketParams_GivenNilNode_ReturnsNil(t *testing.T) {
 	assert.Nil(t, params)
 }
 
+func TestTicketParams_GivenZeroPriceInfoDenom_ReturnsErr(t *testing.T) {
+	n, _ := NewLivepeerNode(nil, "", nil)
+	orch := NewOrchestrator(n, nil)
+	n.Recipient = new(pm.MockRecipient)
+	params, err := orch.TicketParams(ethcommon.Address{}, &net.PriceInfo{PricePerUnit: 0, PixelsPerUnit: 0})
+	assert.Nil(t, params)
+	assert.EqualError(t, err, "pixels per unit is 0")
+}
+
 func TestTicketParams_GivenNilRecipient_ReturnsNil(t *testing.T) {
 	n, _ := NewLivepeerNode(nil, "", nil)
 	orch := NewOrchestrator(n, nil)

core/orchestrator.go

@@ -233,7 +233,12 @@ func (orch *orchestrator) TicketParams(sender ethcommon.Address, priceInfo *net.
 		return nil, nil
 	}
 
-	params, err := orch.node.Recipient.TicketParams(sender, big.NewRat(priceInfo.PricePerUnit, priceInfo.PixelsPerUnit))
+	ratPriceInfo, err := common.RatPriceInfo(priceInfo)
+	if err != nil {
+		return nil, err
+	}
+
+	params, err := orch.node.Recipient.TicketParams(sender, ratPriceInfo)
 	if err != nil {
 		return nil, err
 	}

discovery/db_discovery.go

@@ -99,7 +99,7 @@ func (dbo *DBOrchestratorPoolCache) GetOrchestrators(numOrchestrators int, suspe
 	pred := func(info *net.OrchestratorInfo) bool {
 
 		if err := dbo.ticketParamsValidator.ValidateTicketParams(pmTicketParams(info.TicketParams)); err != nil {
-			glog.V(common.DEBUG).Infof("invalid ticket params - orch=%v err=%v",
+			glog.V(common.DEBUG).Infof("invalid ticket params orch=%v err=%v",
 				info.GetTranscoder(),
 				err,
 			)
@@ -108,9 +108,17 @@ func (dbo *DBOrchestratorPoolCache) GetOrchestrators(numOrchestrators int, suspe
 
 		// check if O's price is below B's max price
 		maxPrice := server.BroadcastCfg.MaxPrice()
-		price := big.NewRat(info.PriceInfo.PricePerUnit, info.PriceInfo.PixelsPerUnit)
+		price, err := common.RatPriceInfo(info.PriceInfo)
+		if err != nil {
+			glog.V(common.DEBUG).Infof("invalid price info orch=%v err=%v", info.GetTranscoder(), err)
+			return false
+		}
+		if price == nil {
+			glog.V(common.DEBUG).Infof("no price info received for orch=%v", info.GetTranscoder())
+			return false
+		}
 		if maxPrice != nil && price.Cmp(maxPrice) > 0 {
-			glog.V(common.DEBUG).Infof("orchestrator's price is too high - orch=%v price=%v wei/pixel maxPrice=%v wei/pixel",
+			glog.V(common.DEBUG).Infof("orchestrator's price is too high orch=%v price=%v wei/pixel maxPrice=%v wei/pixel",
 				info.GetTranscoder(),
 				price.FloatString(3),
 				maxPrice.FloatString(3),

discovery/discovery_test.go

@@ -469,10 +469,14 @@ func TestNewOrchestratorPoolCache_GivenListOfOrchs_CreatesPoolCacheCorrectly(t *
 func TestNewOrchestratorPoolWithPred_TestPredicate(t *testing.T) {
 	pred := func(info *net.OrchestratorInfo) bool {
 		price := server.BroadcastCfg.MaxPrice()
-		if price != nil {
-			return big.NewRat(info.PriceInfo.PricePerUnit, info.PriceInfo.PixelsPerUnit).Cmp(price) <= 0
+		if price == nil {
+			return true
 		}
-		return true
+		ratPriceInfo, err := common.RatPriceInfo(info.PriceInfo)
+		if err != nil {
+			return false
+		}
+		return ratPriceInfo.Cmp(price) <= 0
 	}
 
 	addresses := []string{}
@@ -500,6 +504,10 @@ func TestNewOrchestratorPoolWithPred_TestPredicate(t *testing.T) {
 	// Set MaxBroadcastPrice lower than PriceInfo, should return false
 	server.BroadcastCfg.SetMaxPrice(big.NewRat(1, 1))
 	assert.False(t, pool.pred(oInfo))
+
+	// PixelsPerUnit is 0 , return false
+	oInfo.PriceInfo.PixelsPerUnit = 0
+	assert.False(t, pool.pred(oInfo))
 }
 
 func TestCachedPool_AllOrchestratorsTooExpensive_ReturnsEmptyList(t *testing.T) {