Add remote signer endpoint for GetOrchestratorInfo#3791
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3791 +/- ##
===================================================
- Coverage 32.24436% 32.15842% -0.08594%
===================================================
Files 169 170 +1
Lines 41201 41308 +107
===================================================
- Hits 13285 13284 -1
- Misses 26915 27023 +108
Partials 1001 1001
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
ad-astra-video
left a comment
ad-astra-video
left a comment
There was a problem hiding this comment.
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 ofTicketSenderParamsthat is nonce and signature for all requested nonces. -
Where should the
SenderandSender Monitorlive? 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.mdfor basic instructions to run Remote Signer.
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
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?
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.
Yes, that would be for ticket signing which isn't part of this PR.
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. |
|
|
||
| // Calls the remote signer service to get a signature for GetOrchInfo | ||
| func GetOrchInfoSig(remoteSignerHost *url.URL) (*OrchInfoSigResponse, error) { | ||
|
|
There was a problem hiding this comment.
Small non important formatting improvement.
server/remote_signer.go
Outdated
| Signature HexBytes `json:"signature"` | ||
| } | ||
|
|
||
| // Calls the remote signer service to get a signature for GetOrchInfo |
There was a problem hiding this comment.
@j0sh another small maintainability improvement making it directly clear this is a gateway helper and not Signer method.
| // Calls the remote signer service to get a signature for GetOrchInfo | |
| // GetOrchInfoSig is a gateway helper that calls the remote signer to get the GetOrchInfo signature |
I may be overlooking some details since I’m less in the weeds than the two of you, but I agree with @j0sh on this: it’s best to keep all payment logic centralized in the new node and outside the gateway and orchestration layers. This allows the node to better live up to the vision outlined in the payment clearinghouse spec, where it significantly simplifies the stack and enables lightweight gateways. The main caveat is that the payment logic needs to be general enough to support multiple job types. I haven’t fully thought this through yet, but a reasonable starting point would be to further decouple it from Additionally, we should ensure that state management, or a well-defined stateless communication model, as discussed by Josh in the tech spec, is implemented in a way that allows all parties to dispute transactions if needed. This requirement likely only becomes relevant once stronger compute verification is in place, potentially in 2026 (see this roadmap item). |
rickstaa
left a comment
rickstaa
left a comment
There was a problem hiding this comment.
The technical implementation of this pull request makes sense to me. It can be merged once you and @ad-astra-video are satisfied with it and it has been tested.
cmd/livepeer/starter/flags.go
Outdated
| // flags | ||
| cfg.TestOrchAvail = fs.Bool("startupAvailabilityCheck", *cfg.TestOrchAvail, "Set to false to disable the startup Orchestrator availability check on the configured serviceAddr") | ||
| cfg.RemoteSigner = fs.Bool("remoteSigner", *cfg.RemoteSigner, "Set to true to run remote signer service") | ||
| cfg.RemoteSignerAddr = fs.String("remoteSignerAddr", *cfg.RemoteSignerAddr, "URL of remote signer service to use (e.g., http://localhost:8935). Gateway only.") |
There was a problem hiding this comment.
wdyt about renaming RemoteSignerAddr to RemoteSignerUrl? It might help prevent users from confusing as a remote signer flag for binding to interface/port (which -httpAddr is used for).
There was a problem hiding this comment.
Yeah good idea. We use both (eg, -orchAddr) but agree that the url suffix is better here. ab87e74
j0sh
force-pushed
the
ja/remote-signer-orchinfo
branch
from
210bcbf to
e958057
Compare
4 participants
This PR is the first part of the remote signing feature, implementing the GetOrchestratorInfo request. See the design background for additional motivation and design detail around remote signers.
Remote signing support for Live AI (live-video-to-video) consists of two parts:
This PR adds a new mode to go-livepeer:
-remoteSigner.The remote signer exposes a HTTP POST endpoint at
/sign-orchestrator-info. It currently does only does one thing: produces a signature for use in theOrchestratorInfogRPC call. The response includes the signer's Ethereum address as well as the signature itself.Like the other types of mode flags (gateway, orchestrator, redeemer, etc) the remote signer cannot be combined with other modes.
The gateway adds a new
-remoteSignerAddrflag which specifies the base address of the remote signer to use (host:port). When configured, the gateway pre-fetches the remote signature for OrchestratorInfo at start-up time and caches it, so subsequent calls to OrchestratorInfo do not have to incur additional remote calls.One might note that this OrchestratorInfo signing scheme doesn't actually accomplish much at all: the signature effectively static, not scoped to any one orchestrator, and never expires. This is a legacy aspect of the OrchestratorInfo flow that we have to accommodate for now; it is a vestigial corner of the codebase, but one that we've judged internally to be harmless.