Skip to content
Draft
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
2 changes: 2 additions & 0 deletions CHANGELOG_PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

#### Orchestrator

- The "insufficient sender reserve" error now reports which check failed and the values involved (faceValue vs ticketEV vs sender max float, or faceValue vs redemption tx cost) so operators can tell whether the sender's reserve, `-ticketEV`, or `-maxFaceValue` needs adjusting

#### Transcoder

### Bug Fixes 🐞
Expand Down
5 changes: 3 additions & 2 deletions pm/recipient.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"crypto/hmac"
"crypto/rand"
"crypto/sha256"
"fmt"
"math/big"
"sync"

Expand Down Expand Up @@ -293,7 +294,7 @@ func (r *recipient) faceValue(sender ethcommon.Address) (*big.Int, error) {
monitor.MaxFloat(sender.Hex(), maxFloat)
}
if faceValue.Cmp(r.cfg.EV) < 0 {
return nil, errInsufficientSenderReserve
return nil, fmt.Errorf("%w: faceValue %v is less than ticketEV %v (sender max float: %v); the sender's reserve is too low for this orchestrator's -ticketEV", errInsufficientSenderReserve, faceValue, r.cfg.EV, maxFloat)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== repo files =="
git ls-files | rg '(^|/)(recipient\.go|job_orchestrator\.go)$|\.go$' | sed -n '1,120p'

echo "== recipient.go relevant block =="
if [ -f pm/recipient.go ]; then
  wc -l pm/recipient.go
  sed -n '260,330p' pm/recipient.go
fi

echo "== job_orchestrator.go occurrences =="
if [ -f pm/byoc/job_orchestrator.go ]; then
  wc -l pm/byoc/job_orchestrator.go
  rg -n 'insufficient|Insufficient|503|400|errors\.Is|errInsufficientSenderReserve|SenderReserve' pm/byoc/job_orchestrator.go
else
  fd -i 'job_orchestrator.go' . | sed -n '1,20p'
fi

Repository: livepeer/go-livepeer

Length of output: 6313


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== all occurrences of errInsufficientSenderReserve and insufficient sender reserve =="
rg -n 'errInsufficientSenderReserve|insufficient sender reserve|InsufficientSenderReserve|SenderReserve|job_orchestrator' .

echo "== go module / package summary =="
sed -n '1,80p' pm/byoc/job_orchestrator.go 2>/dev/null || true
sed -n '1,80p' pm/recipient.go 2>/dev/null || true

Repository: livepeer/go-livepeer

Length of output: 4579


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== job_orchestrator.go relevant blocks =="
sed -n '130,180p' byoc/job_orchestrator.go
echo "== job_orchestrator_test.go relevant error case =="
sed -n '580,630p' byoc/job_orchestrator_test.go
rg -n 'if err\.Error\(\) == "insufficient sender reserve"|ErrorIs|insufficient sender reserve' byoc/job_orchestrator.go byoc/job_orchestrator_test.go pm/recipient.go pm/recipient_test.go | sed -n '1,120p'

Repository: livepeer/go-livepeer

Length of output: 4623


Preserve the downstream insufficient-reserve status contract.

pm/recipient.go now wraps errInsufficientSenderReserve, so err.Error() == "insufficient sender reserve" no longer matches these production errors. byoc/job_orchestrator.go:159 therefore classifies 503 reserve failures as 400. Update this consumer to use errors.Is with an exported/shared sentinel or helper, and update byoc/job_orchestrator_test.go to exercise the wrapped error.

Also applies to lines 297 and 313 in pm/recipient.go.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pm/recipient.go` at line 297, Update the insufficient-reserve classification
in the byoc job orchestrator consumer to use errors.Is against a shared exported
sentinel or helper for errInsufficientSenderReserve, rather than comparing
err.Error() text. Apply the change to the relevant classification logic and
extend byoc job orchestrator tests to verify wrapped insufficient-reserve errors
still produce the 503 response; preserve existing behavior for unrelated errors.

}
Comment on lines 296 to 298

// faceValue must be >= txCostWithGasPrice(current gasPrice) OR >= txCostWithGasPrice(avg gasPrice)
Expand All @@ -309,7 +310,7 @@ func (r *recipient) faceValue(sender ethcommon.Address) (*big.Int, error) {
// and needs to be redeemed.
// For now, avgGasPrice is hardcoded. See the comment for avgGasPrice for TODO information.
if faceValue.Cmp(txCost) < 0 && faceValue.Cmp(r.txCostWithGasPrice(avgGasPrice)) < 0 {
return nil, errInsufficientSenderReserve
return nil, fmt.Errorf("%w: faceValue %v cannot cover the ticket redemption tx cost (at current gas price: %v, at avg gas price: %v); increase -maxFaceValue or the sender's reserve", errInsufficientSenderReserve, faceValue, txCost, r.txCostWithGasPrice(avgGasPrice))
}
Comment on lines 312 to 314

return faceValue, nil
Expand Down
10 changes: 6 additions & 4 deletions pm/recipient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,8 @@ func TestTicketParams(t *testing.T) {
// Test insufficient sender reserve error due to maxFloat < EV
sm.maxFloat = new(big.Int).Sub(cfg.EV, big.NewInt(1))
_, err = r.TicketParams(sender, big.NewRat(1, 1))
assert.EqualError(err, errInsufficientSenderReserve.Error())
assert.ErrorIs(err, errInsufficientSenderReserve)
assert.ErrorContains(err, "less than ticketEV")

// Test faceValue < txCostWithGasPrice(current gasPrice) and faceValue > txCostWithGasPrice(avg gasPrice)
// Set current gasPrice higher than avg gasPrice
Expand All @@ -553,7 +554,8 @@ func TestTicketParams(t *testing.T) {
require.True(sm.maxFloat.Cmp(txCost) < 0)
require.True(sm.maxFloat.Cmp(txCostAvgGasPrice) < 0)
_, err = r.TicketParams(sender, big.NewRat(1, 1))
assert.EqualError(err, errInsufficientSenderReserve.Error())
assert.ErrorIs(err, errInsufficientSenderReserve)
assert.ErrorContains(err, "cannot cover the ticket redemption tx cost")

// Test lazy evaluation when faceValue > txCostWithGasPrice(current gasPrice)
// Set current gasPrice lower than avg gasPrice
Expand Down Expand Up @@ -587,7 +589,7 @@ func TestTicketParams(t *testing.T) {
sm.maxFloat = big.NewInt(0) // Set maxFloat to some value less than EV

_, err = r.TicketParams(sender, big.NewRat(1, 1))
assert.EqualError(err, errInsufficientSenderReserve.Error())
assert.ErrorIs(err, errInsufficientSenderReserve)
}

func TestTxCostMultiplier_UsingFaceValue_ReturnsDefaultMultiplier(t *testing.T) {
Expand Down Expand Up @@ -638,7 +640,7 @@ func TestTxCostMultiplier_InsufficientReserve_ReturnsError(t *testing.T) {

mul, err := r.TxCostMultiplier(sender)
assert.Nil(t, mul)
assert.EqualError(t, err, errInsufficientSenderReserve.Error())
assert.ErrorIs(t, err, errInsufficientSenderReserve)
}

func TestTxCostMultiplier_ZeroTxCost_Returns_Zero(t *testing.T) {
Expand Down
Loading