From dbcf54e4ba833288e0182913f017f290d2790a0e Mon Sep 17 00:00:00 2001 From: Nico Vergauwen Date: Thu, 25 Mar 2021 21:56:25 +0100 Subject: [PATCH 1/2] core,discovery: add checks for PriceInfo.pixelsPerUnit==0 --- core/orch_test.go | 9 +++++++++ core/orchestrator.go | 7 ++++++- discovery/db_discovery.go | 14 +++++++++++--- discovery/discovery_test.go | 14 +++++++++++--- 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/core/orch_test.go b/core/orch_test.go index a69bb35e88..b9b60a3715 100644 --- a/core/orch_test.go +++ b/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) diff --git a/core/orchestrator.go b/core/orchestrator.go index ab2e4202c8..93ca821669 100644 --- a/core/orchestrator.go +++ b/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 } diff --git a/discovery/db_discovery.go b/discovery/db_discovery.go index dc7ddeb3c9..70ddc5f188 100644 --- a/discovery/db_discovery.go +++ b/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 price == nil { + glog.V(common.DEBUG).Infof("no price info received for orch=%v", info.GetTranscoder()) + return false + } + if err != nil { + glog.V(common.DEBUG).Infof("invalid price info orch=%v err=%v", info.GetTranscoder(), err) + 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), diff --git a/discovery/discovery_test.go b/discovery/discovery_test.go index 6011850172..1a6e3fdd25 100644 --- a/discovery/discovery_test.go +++ b/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) { From 84effcd813dfdb6b13f2106a42d39b7f3d763ac3 Mon Sep 17 00:00:00 2001 From: Nico Vergauwen Date: Thu, 25 Mar 2021 22:02:50 +0100 Subject: [PATCH 2/2] pending changelog --- CHANGELOG_PENDING.md | 4 +++- discovery/db_discovery.go | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 42d2315e54..0c322860dc 100644 --- a/CHANGELOG_PENDING.md +++ b/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) \ No newline at end of file diff --git a/discovery/db_discovery.go b/discovery/db_discovery.go index 70ddc5f188..13e3343945 100644 --- a/discovery/db_discovery.go +++ b/discovery/db_discovery.go @@ -109,14 +109,14 @@ func (dbo *DBOrchestratorPoolCache) GetOrchestrators(numOrchestrators int, suspe // check if O's price is below B's max price maxPrice := server.BroadcastCfg.MaxPrice() price, err := common.RatPriceInfo(info.PriceInfo) - if price == nil { - glog.V(common.DEBUG).Infof("no price info received for orch=%v", info.GetTranscoder()) - return false - } 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", info.GetTranscoder(),