livepeer / livepeer/go-livepeer

pm: Ticket redemption follow-ups after the TicketBroker patch (protocol#657)

Open
#4,013 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

status: triage
Dominant language
Go
Stars
586
Forks
226
Avg merge
1d 17h
Merged PRs (30d)
19

Description

Follow-up to [livepeer/protocol#657](https://github.com/livepeer/protocol/pull/657), which makes `redeemWinningTicket` revert (rather than underpay) when a sender's `deposit + reserve` cannot cover the full ticket face value. Tracking the go-livepeer side; we intend to fix these.

## What already works — no action needed

An underfunded sender is **already retried** today. `EstimateGas` catches the new revert pre-flight, so no transaction is submitted and no gas is spent, and the error text doesn't match `isNonRetryableTicketErr` (`pm/queue.go:166`). The ticket stays queued and `handleBlockEvent` re-tries it on every new L1 block (~12s) for its full 2-round validity window, so it redeems as soon as the sender tops up.

For a fully drained sender the client doesn't even reach the chain: `availableFunds (0) <= txCost` trips the guard at `pm/sendermonitor.go:389`, which is also retryable.

**nothing in go-livepeer matches the old `sender deposit and reserve are zero` revert string.**

## 1. A reverted redemption drops the ticket (correctness)

If the transaction does reach the chain and reverts, `CheckTx` returns `transaction failed txHash=...` with **no revert reason** — Ethereum receipts don't carry one, so `eth/client.go:1163` builds that string from `receipt.Status` alone. `isNonRetryableTicketErr` matches it and `MarkWinningTicketRedeemed` drops the ticket locally, even though a revert consumed nothing and the ticket is still redeemable.

Reachable when `-gasLimit` is set explicitly (skipping `EstimateGas`) or when the sender's balance drops between estimate and mining — the race #657 addresses.

Adding the new revert string to the denylist can't fix this: in this path the string never reaches Go. Instead, call `IsUsedTicket` (already used at `sendermonitor.go:363`) before marking a ticket redeemed and let the contract decide.

## 2. Doomed redemptions cost RPC every block (efficiency)

There is no client-side `availableFunds >= faceValue` check — the guards at `sendermonitor.go:389,392` both compare against `txCost` (~0.000024 ETH), not against the face value. So for a sender in the band `txCost < availableFunds < faceValue`, every block spends `IsUsedTicket` + `SuggestGasPrice` + `EstimateGas` before failing.

A comparison against the cached `availableFunds`, placed before those calls, would make the deferral free. Needs care: `availableFunds` subtracts in-flight `pendingAmount`, so it can be stricter than the contract and over-defer.

## 3. Head-of-queue is re-attempted N times per block (efficiency)

`handleBlockEvent` loops `numTickets` times but always re-selects the *earliest* unredeemed ticket, so a retryable failure re-attempts the same ticket once per queued ticket. Should stop and wait for the next block.

## 4. Benign retries are counted as errors (observability)

`monitor.TicketRedemptionError` fires on the retryable path (`sendermonitor.go:419`), so an underfunded sender inflates the redemption-error metric once per ticket per block.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Trace ticket redemption through pm/sendermonitor.go, pm/queue.go, eth/client.go, and handleBlockEvent, starting with the IsUsedTicket call at sendermonitor.go:363 and the retry path around line 419. Verify the behavior for reverted, underfunded, and retryable tickets, then ensure doomed attempts are deferred, the head ticket is not retried repeatedly in one block, and benign retries do not inflate the redemption-error metric.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, blockchain
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.