Skip to content

Conversation

@j0sh
Copy link
Collaborator

@j0sh j0sh commented Oct 22, 2025

TODO explain

@github-actions github-actions bot added the go Pull requests that update Go code label Oct 22, 2025
@codecov
Copy link

codecov bot commented Oct 22, 2025

Codecov Report

❌ Patch coverage is 4.00000% with 96 lines in your changes missing coverage. Please review.
✅ Project coverage is 31.46184%. Comparing base (aca2275) to head (f059422).

Files with missing lines Patch % Lines
server/remote_signer.go 0.00000% 73 Missing ⚠️
cmd/livepeer/starter/starter.go 16.00000% 21 Missing ⚠️
cmd/livepeer/starter/flags.go 0.00000% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##              master       #3791         +/-   ##
===================================================
- Coverage   31.52544%   31.46184%   -0.06360%     
===================================================
  Files            159         160          +1     
  Lines          47901       48001        +100     
===================================================
+ Hits           15101       15102          +1     
- Misses         31911       32009         +98     
- Partials         889         890          +1     
Files with missing lines Coverage Δ
core/livepeernode.go 60.22099% <ø> (ø)
cmd/livepeer/starter/flags.go 0.00000% <0.00000%> (ø)
cmd/livepeer/starter/starter.go 21.90476% <16.00000%> (-0.08388%) ⬇️
server/remote_signer.go 0.00000% <0.00000%> (ø)

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aca2275...f059422. Read the comment docs.

Files with missing lines Coverage Δ
core/livepeernode.go 60.22099% <ø> (ø)
cmd/livepeer/starter/flags.go 0.00000% <0.00000%> (ø)
cmd/livepeer/starter/starter.go 21.90476% <16.00000%> (-0.08388%) ⬇️
server/remote_signer.go 0.00000% <0.00000%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Collaborator

@ad-astra-video ad-astra-video left a comment

Choose a reason for hiding this comment

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

Some initial thoughts as I went through this PR:

  • We should add an Authorization secret to protect the remote signer endpoints. WDYT?

  • I think we should add a standard port at top of starter.go similar to other modes for go-livepeer

  • I think the signed Gateway address to send in GetOrchestrator request should be a GET request since no payload is required. We probably should cache this on the Gateway as well since it doesn't change and would lower the requests to the signer service.

    • Seems like this request should be sent at Gateway startup and the Gateway should fail to startup if the request fails.
  • Need a POST request endpoint on remote signer expecting json payload of { nonces: [list of nonces to use], ticket_params: [orch-ticket-params] that returns list of "tickets" which is just a list of TicketSenderParams that is nonce and signature for all requested nonces.

  • Where should the Sender and Sender Monitor live? I think the Gateway (or local SDK) should still be responsible for this since it is directly communicating with the Orchestrator. Keeping the remote signer lightweight will assist in payment services being built since it does one thing which is use a ethereum address to sign a specific payload.

  • When we settle on the implementation we should add a doc/remote_signer.md for basic instructions to run Remote Signer.

@j0sh
Copy link
Collaborator Author

j0sh commented Nov 13, 2025

We should add an Authorization secret to protect the remote signer endpoints.

Auth will absolutely be needed for clearinghouses. A header in conjunction with webhooks is probably the way to go. But I would defer implementing auth for now until the clearinghouse requirements are clearer. Our other endpoints are unauthenticated and generally sit behind a layer of proxies, so they should never be exposed to the world. This specific mode is also opt-in via the -remoteSigner flag and few operators will actually use it in prod, so the risk should be low for now.

I think we should add a standard port at top of starter.go similar to other modes for go-livepeer

Not sure if that is necessary since you can still set the port that the remote signer uses with the -httpAddr flag, the same way we set the port for the other modes. Is there an issue with the current default port?

I think the signed Gateway address to send in GetOrchestrator request should be a GET request since no payload is required

We’ll probably need payloads soon enough, since GetOrchestrator requests are parameterized (capabilities, etc). I just haven’t thought through that mapping yet, and will probably punt on that until after this PR.

Need a POST request endpoint on remote signer expecting json payload ...

Yes, that would be for ticket signing which isn't part of this PR.

Where should the Sender and Sender Monitor live? I think the Gateway (or local SDK) should still be responsible for this since it is directly communicating with the Orchestrator.  Keeping the remote signer lightweight will assist in payment services being built since it does one thing which is use a ethereum address to sign a specific payload.

My thinking is actually the opposite. There will probably be only one remote signer implementation - this one - but many SDK implementations. Livepeer payments are extremely complicated, and we don't want to require a Javascript or Python developer to wade through that. So we should handle as much in the remote signer as possible, and keep SDKs straightforward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go Pull requests that update Go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants