From 5b3a246b79ca45a2ce10c885fc9f55f31162d9cd Mon Sep 17 00:00:00 2001 From: Victor Elias Date: Tue, 2 Apr 2024 16:41:10 +0200 Subject: [PATCH 1/5] server: Remove maxPrice check mid-session --- server/segment_rpc.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/server/segment_rpc.go b/server/segment_rpc.go index 7f81999f7b..3472222e27 100644 --- a/server/segment_rpc.go +++ b/server/segment_rpc.go @@ -826,10 +826,6 @@ func validatePrice(sess *BroadcastSession) error { return errors.New("missing orchestrator price") } - maxPrice := BroadcastCfg.MaxPrice() - if maxPrice != nil && oPrice.Cmp(maxPrice) == 1 { - return fmt.Errorf("Orchestrator price higher than the set maximum price of %v wei per %v pixels", maxPrice.Num().Int64(), maxPrice.Denom().Int64()) - } iPrice, err := common.RatPriceInfo(sess.InitialPrice) if err == nil && iPrice != nil && oPrice.Cmp(iPrice) == 1 { return fmt.Errorf("Orchestrator price has changed, Orchestrator price: %v, Orchestrator initial price: %v", oPrice, iPrice) From 90bcffb293b576e02663f4ccebecd6c08c16ed0d Mon Sep 17 00:00:00 2001 From: Victor Elias Date: Tue, 2 Apr 2024 16:52:15 +0200 Subject: [PATCH 2/5] server: Fix tests --- server/rpc_test.go | 26 ++++---------------------- server/segment_rpc_test.go | 7 ++++--- 2 files changed, 8 insertions(+), 25 deletions(-) diff --git a/server/rpc_test.go b/server/rpc_test.go index ab35bb0875..f90b70b339 100644 --- a/server/rpc_test.go +++ b/server/rpc_test.go @@ -549,13 +549,13 @@ func TestGenPayment(t *testing.T) { sender := &pm.MockSender{} s.Sender = sender - // Test invalid price - BroadcastCfg.SetMaxPrice(core.NewFixedPrice(big.NewRat(1, 5))) + // Test changing O price + s.InitialPrice = &net.PriceInfo{PricePerUnit: 1, PixelsPerUnit: 5} payment, err = genPayment(context.TODO(), s, 1) assert.Equal("", payment) assert.Errorf(err, err.Error(), "Orchestrator price higher than the set maximum price of %v wei per %v pixels", int64(1), int64(5)) - BroadcastCfg.SetMaxPrice(nil) + s.InitialPrice = nil // Test CreateTicketBatch error sender.On("CreateTicketBatch", mock.Anything, mock.Anything).Return(nil, errors.New("CreateTicketBatch error")).Once() @@ -680,22 +680,10 @@ func TestValidatePrice(t *testing.T) { PMSessionID: "foo", } - // B's MaxPrice is nil + // O's Initial Price is nil err := validatePrice(s) assert.Nil(err) - defer BroadcastCfg.SetMaxPrice(nil) - - // B MaxPrice > O Price - BroadcastCfg.SetMaxPrice(core.NewFixedPrice(big.NewRat(5, 1))) - err = validatePrice(s) - assert.Nil(err) - - // B MaxPrice == O Price - BroadcastCfg.SetMaxPrice(core.NewFixedPrice(big.NewRat(1, 3))) - err = validatePrice(s) - assert.Nil(err) - // O Initial Price == O Price s.InitialPrice = oinfo.PriceInfo err = validatePrice(s) @@ -711,12 +699,6 @@ func TestValidatePrice(t *testing.T) { err = validatePrice(s) assert.ErrorContains(err, "price has changed") - // B MaxPrice < O Price - s.InitialPrice = nil - BroadcastCfg.SetMaxPrice(core.NewFixedPrice(big.NewRat(1, 5))) - err = validatePrice(s) - assert.EqualError(err, fmt.Sprintf("Orchestrator price higher than the set maximum price of %v wei per %v pixels", int64(1), int64(5))) - // O.PriceInfo is nil s.OrchestratorInfo.PriceInfo = nil err = validatePrice(s) diff --git a/server/segment_rpc_test.go b/server/segment_rpc_test.go index f8786202cd..9c6d5391b4 100644 --- a/server/segment_rpc_test.go +++ b/server/segment_rpc_test.go @@ -1677,11 +1677,12 @@ func TestSubmitSegment_GenPaymentError_ValidatePriceError(t *testing.T) { Sender: sender, Balance: balance, OrchestratorInfo: oinfo, + InitialPrice: &net.PriceInfo{ + PricePerUnit: 1, + PixelsPerUnit: 5, + }, } - BroadcastCfg.SetMaxPrice(core.NewFixedPrice(big.NewRat(1, 5))) - defer BroadcastCfg.SetMaxPrice(nil) - _, err := SubmitSegment(context.TODO(), s, &stream.HLSSegment{}, nil, 0, false, true) assert.EqualErrorf(t, err, err.Error(), "Orchestrator price higher than the set maximum price of %v wei per %v pixels", int64(1), int64(5)) From 630272acfc58dd470781f392cfb3d022e1aef69e Mon Sep 17 00:00:00 2001 From: Victor Elias Date: Wed, 27 Mar 2024 20:34:52 -0300 Subject: [PATCH 3/5] server: Fix erroneous usage of assert.EqualErrorf When I was writing the tests for validatePrice I found out we were using that function wrong in a couple places and never checking any error. We were sending err.Error() to check the error from err. --- server/handlers_test.go | 6 +++--- server/segment_rpc_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/handlers_test.go b/server/handlers_test.go index f7852e3d46..d191d0a75e 100644 --- a/server/handlers_test.go +++ b/server/handlers_test.go @@ -520,13 +520,13 @@ func TestSetOrchestratorPriceInfo(t *testing.T) { assert.Zero(t, s.LivepeerNode.GetBasePrice("default").Cmp(big.NewRat(1, 1))) err = s.setOrchestratorPriceInfo("default", "-5", "1", "") - assert.EqualErrorf(t, err, err.Error(), "price unit must be greater than or equal to 0, provided %d\n", -5) + assert.EqualError(t, err, fmt.Sprintf("price unit must be greater than or equal to 0, provided %d", -5)) // pixels per unit <= 0 err = s.setOrchestratorPriceInfo("default", "1", "0", "") - assert.EqualErrorf(t, err, err.Error(), "pixels per unit must be greater than 0, provided %d\n", 0) + assert.EqualError(t, err, fmt.Sprintf("pixels per unit must be greater than 0, provided %d", 0)) err = s.setOrchestratorPriceInfo("default", "1", "-5", "") - assert.EqualErrorf(t, err, err.Error(), "pixels per unit must be greater than 0, provided %d\n", -5) + assert.EqualError(t, err, fmt.Sprintf("pixels per unit must be greater than 0, provided %d", -5)) } func TestSetPriceForBroadcasterHandler(t *testing.T) { diff --git a/server/segment_rpc_test.go b/server/segment_rpc_test.go index 9c6d5391b4..a400fee4ca 100644 --- a/server/segment_rpc_test.go +++ b/server/segment_rpc_test.go @@ -1685,7 +1685,7 @@ func TestSubmitSegment_GenPaymentError_ValidatePriceError(t *testing.T) { _, err := SubmitSegment(context.TODO(), s, &stream.HLSSegment{}, nil, 0, false, true) - assert.EqualErrorf(t, err, err.Error(), "Orchestrator price higher than the set maximum price of %v wei per %v pixels", int64(1), int64(5)) + assert.EqualError(t, err, fmt.Sprintf("Orchestrator price higher than the set maximum price of %v wei per %v pixels", int64(1), int64(5))) balance.AssertCalled(t, "Credit", existingCredit) } From 19b3669df4015d42165d081cf3e7cdec1da3aec6 Mon Sep 17 00:00:00 2001 From: Victor Elias Date: Tue, 2 Apr 2024 17:11:15 +0200 Subject: [PATCH 4/5] server: Fix error checks after fixing assertion --- server/rpc_test.go | 2 +- server/segment_rpc_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/rpc_test.go b/server/rpc_test.go index f90b70b339..93c776f456 100644 --- a/server/rpc_test.go +++ b/server/rpc_test.go @@ -553,7 +553,7 @@ func TestGenPayment(t *testing.T) { s.InitialPrice = &net.PriceInfo{PricePerUnit: 1, PixelsPerUnit: 5} payment, err = genPayment(context.TODO(), s, 1) assert.Equal("", payment) - assert.Errorf(err, err.Error(), "Orchestrator price higher than the set maximum price of %v wei per %v pixels", int64(1), int64(5)) + assert.Errorf(err, "Orchestrator price has changed, Orchestrator price: %v, Orchestrator initial price: %v", "1/3", "1/5") s.InitialPrice = nil diff --git a/server/segment_rpc_test.go b/server/segment_rpc_test.go index a400fee4ca..01c37f903c 100644 --- a/server/segment_rpc_test.go +++ b/server/segment_rpc_test.go @@ -1685,7 +1685,7 @@ func TestSubmitSegment_GenPaymentError_ValidatePriceError(t *testing.T) { _, err := SubmitSegment(context.TODO(), s, &stream.HLSSegment{}, nil, 0, false, true) - assert.EqualError(t, err, fmt.Sprintf("Orchestrator price higher than the set maximum price of %v wei per %v pixels", int64(1), int64(5))) + assert.EqualError(t, err, fmt.Sprintf("Orchestrator price has changed, Orchestrator price: %v, Orchestrator initial price: %v", "1/3", "1/5")) balance.AssertCalled(t, "Credit", existingCredit) } From 73fd824cfe7b44f6189d84dc7f9008396d14edc9 Mon Sep 17 00:00:00 2001 From: Victor Elias Date: Tue, 2 Apr 2024 17:20:33 +0200 Subject: [PATCH 5/5] CHANGELOG --- CHANGELOG_PENDING.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 3ca5044811..cb662d0568 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -22,6 +22,8 @@ #### Broadcaster +- [#2994](https://github.com/livepeer/go-livepeer/pull/2994) server: Skip redundant maxPrice check in ongoing session (@victorges) + #### Orchestrator #### Transcoder