fix(byoc): type:byoc payments + capabilities on orch discovery#41
fix(byoc): type:byoc payments + capabilities on orch discovery#41seanhanca wants to merge 3 commits into
Conversation
Use BYOC capability constraints for get_orch_info and /generate-live-payment so the signer can resolve per-cap pricing and usage labels without a legacy capability string field. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughBYOC payment generation now derives capability protobufs from the requested app, passes them to orchestrator discovery, and selects legacy or BYOC signer payload fields by signer host. Tests cover capability construction, routing, discovery threading, and payload encoding. ChangesBYOC payment signing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PaymentFlow
participant OrchestratorInfo
participant Signer
PaymentFlow->>PaymentFlow: Select payment type from signer host
alt BYOC signer
PaymentFlow->>OrchestratorInfo: get_orch_info(capabilities)
OrchestratorInfo-->>PaymentFlow: ticket_params
PaymentFlow->>Signer: generate-live-payment(type byoc, encoded capabilities)
else Legacy Daydream signer
PaymentFlow->>OrchestratorInfo: get_orch_info(capabilities=None)
OrchestratorInfo-->>PaymentFlow: ticket_params
PaymentFlow->>Signer: generate-live-payment(type lv2v, capability)
end
Signer-->>PaymentFlow: payment response
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/livepeer_gateway/byoc.py`:
- Around line 193-204: Build `byoc_caps` before the orchestrator discovery call
and pass that same protobuf as the `capabilities=` argument to `get_orch_info`;
retain it for constructing `payment_payload` so discovery and signing use
identical BYOC capability constraints.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74aa7270-c9b2-4860-94ff-2cce17d75c50
📒 Files selected for processing (2)
src/livepeer_gateway/byoc.pysrc/livepeer_gateway/capabilities.py
TicketParams must use PriceInfoForCaps when ByocPerCapPricing is enabled. _create_byoc_payment now threads the same BYOC capabilities protobuf into orch discovery and the signer payment payload. Co-authored-by: Cursor <cursoragent@cursor.com>
Addendum — per-cap TicketParams alignment (Run 33)Prerequisite check for enabling Fix (commit
Tests: Deploy: not run this session — merge + image rebuild pending |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_byoc_payment.py (1)
93-116: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
next()instead of single-element list slice.Ruff RUF015 flags line 108:
[r for r in reqs if ...][0]is less idiomatic thannext(...). In test code this is cosmetic, butnext()also gives a clearer error if no match is found.♻️ Optional refactor
- signer_req = [r for r in reqs if "generate-live-payment" in r.full_url][0] + signer_req = next(r for r in reqs if "generate-live-payment" in r.full_url)🤖 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 `@tests/test_byoc_payment.py` around lines 93 - 116, Replace the single-element list selection in test_create_byoc_payment_includes_capabilities_on_signer_payload with next() over a generator expression filtering reqs for "generate-live-payment", preserving the existing signer_req behavior while satisfying Ruff RUF015.Source: Linters/SAST tools
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@tests/test_byoc_payment.py`:
- Around line 93-116: Replace the single-element list selection in
test_create_byoc_payment_includes_capabilities_on_signer_payload with next()
over a generator expression filtering reqs for "generate-live-payment",
preserving the existing signer_req behavior while satisfying Ruff RUF015.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: caf2c653-7b37-4ced-acec-88168fbaa4dd
📒 Files selected for processing (3)
src/livepeer_gateway/byoc.pytests/test_byoc_payment.pytests/test_capabilities.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/livepeer_gateway/byoc.py
Gate type:byoc vs type:lv2v on signer host so sdk.daydream.monster can serve Daydream keys (signer.daydream.live) and NaaP keys (pymthouse DMZ) from one image without breaking legacy ticket signing. Co-authored-by: Cursor <cursoragent@cursor.com>
Run 35 follow-up: dual-path payment type (
|
| Signer host | Payment shape | Orch discovery |
|---|---|---|
signer.daydream.live |
type:lv2v + capability string |
no capabilities proto (bd8e780 behavior) |
pymthouse DMZ / other (e.g. pymthouse-production.up.railway.app) |
type:byoc + capabilities proto |
capabilities on get_orch_info (1114138 behavior) |
This lets one SDK image serve both Daydream (pmth_/sk_) and NaaP (naap_) callers on sdk-staging-1.
Tests
tests/test_byoc_payment.py — 6/6 passed:
- legacy Daydream signer → lv2v payload, no capabilities on orch discovery
- pymthouse DMZ signer → byoc payload + capabilities threading
Deploy note
Rebuild SDK image from 1bf13cd and redeploy sdk-staging-1 to replace the broken byoc-type-byoc-4e5870e-2026-07-10 image. Rollback to byoc-lv2v-bd8e7807 is no longer required once the new image is live.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_byoc_payment.py (1)
152-152: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
next(...)over single element list indexing.Building an entire list comprehension only to extract the first element can be replaced with the more idiomatic and lazily-evaluated
next(...)function.
tests/test_byoc_payment.py#L152-L152: replace[r for r in reqs if "generate-live-payment" in r.full_url][0]withnext(r for r in reqs if "generate-live-payment" in r.full_url).tests/test_byoc_payment.py#L182-L182: replace[r for r in reqs if "generate-live-payment" in r.full_url][0]withnext(r for r in reqs if "generate-live-payment" in r.full_url).🤖 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 `@tests/test_byoc_payment.py` at line 152, Replace the list-comprehension indexing used to assign signer_req with lazy next(...) generator expressions at tests/test_byoc_payment.py lines 152-152 and 182-182, preserving the existing generate-live-payment URL filter and first-match behavior.Source: Linters/SAST tools
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@tests/test_byoc_payment.py`:
- Line 152: Replace the list-comprehension indexing used to assign signer_req
with lazy next(...) generator expressions at tests/test_byoc_payment.py lines
152-152 and 182-182, preserving the existing generate-live-payment URL filter
and first-match behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: af6ef908-8e0d-4b3e-b4cd-0a220be0ee9e
📒 Files selected for processing (2)
src/livepeer_gateway/byoc.pytests/test_byoc_payment.py
…ge into ja/live-runner (#46) ## Why The production SDK (`sdk-service:byoc-dual-path-1bf13cd`) carries a **load-bearing byoc-payment fix that exists in no branch** — only in the running container. That's an operational liability and it blocks a unified gateway. This ports it onto `ja/live-runner` (the Live Runner + Scope branch, PR #20 → main) so the consolidated gateway keeps BYOC payment working while gaining LR. ## What - `_payment_type_for_signer(signer_url)` — **legacy Daydream signer** (`signer.daydream.live`) → `type:"lv2v"` + string `capability`; **modern signers** (pymthouse DMZ, …) → `type:"byoc"` + BYOC capabilities protobuf. - `_create_byoc_payment` — assemble the payment payload + orch-discovery capabilities per the resolved type. - `capabilities.py` — `CapabilityId.BYOC` + `byoc_capabilities_from_app()`. Two files, +52/−9. Byte-identical to what runs in prod today. ## Relationship to #41 PR #41 sends `type:"byoc"` **unconditionally** and depends on an undeployed go-livepeer signer+orch change. Against `signer.daydream.live` (which only accepts `lv2v` today) that reproduces the **2026-07-13 “invalid job type” outage**. This PR is the **superset**: per-signer switching keeps the legacy signer working *and* enables modern signers. Recommend closing #41 in favor of this. ## Follow-on The same per-signer type logic generalizes to the **live-runner** payment path, which will let us drop the `lr-gateway` `lv2v` workaround once this lands. ## Test - [ ] `python -m py_compile` (passes locally) - [ ] `submit_byoc_job` against `signer.daydream.live` → `type:lv2v` → 200 (no regression) - [ ] `submit_byoc_job` against a modern signer → `type:byoc` + caps proto → 200 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Summary
submit_byoc_job/_create_byoc_payment: Sendtype:"byoc"with BYOCcapabilitiesprotobuf on/generate-live-payment(no legacycapabilitystring field). Matches John’s c6d312f intent on the inference path.get_orch_info: Pass the same BYOC capabilities protobuf during orch discovery so advertisedCapabilitiesPricesmatch the job constraint.capabilities.py: AddCapabilityId.BYOC+byoc_capabilities_from_app()helper (required onmain).Depends on
type:byocand verify V1 job creds before this path works end-to-end.Test plan
python -m pytest tests/test_capabilities.py(add if missing — manual import check OK)submit_byoc_job(...)with composite bearer → payment 200 → orch 200byoc/flux-schnellnotunknownNote for John
Live-runner (
type:"byoc"inLivePaymentSession) remains onja/live-runner/ bd8e780 lineage — this PR covers one-shot inference only. Stack live-runner change separately when deploying SDK canary.Made with Cursor
Summary by CodeRabbit