feat(remote_signer): Add job token signing endpoint for byoc job types#3869
feat(remote_signer): Add job token signing endpoint for byoc job types#3869
Conversation
…o orchestrator and update gRPC spec - Introduced ExternalCapabilities method in the orchestrator to retrieve registered external capabilities. - Updated OrchestratorInfo message in protobuf to include external capabilities information. - Enhanced gRPC client and server implementations to utilize new method and handle external capabilities. - Added new ExternalCapabilityInfo message to define external capabilities with attributes like name, description, capacity, and pricing. - Updated remote signer to support BYOC job type and handle capability-specific logic in payment processing. - Adjusted tests to accommodate new external capabilities functionality.
…ethods - Removed PriceInfo field from ExternalCapabilityInfo in both protobuf and Go files to streamline capability information. - Updated orchestratorInfoWithCaps function to reflect the removal of PriceInfo, ensuring compatibility with existing external capabilities logic. - Adjusted related comments and documentation to maintain clarity and accuracy.
- Added a new route for retrieving capabilities at "/byoc/capabilities". - Introduced GetCapabilities method to handle HTTP requests and return a JSON response with external capabilities information. - Updated Orchestrator interface to include ExternalCapabilities method for fetching registered capabilities. - Removed ExternalCapabilityInfo from protobuf and related files to streamline capability management. - Adjusted orchestratorInfoWithCaps function to reflect changes in external capabilities handling.
- Downgraded protoc-gen-go-grpc version from v1.6.0 to v1.2.0. - Updated gRPC support package version requirement from v1.64.0 to v1.32.0. - Replaced constant method names with direct string references for RPC calls in the OrchestratorClient and OrchestratorServer interfaces. - Adjusted comments for clarity and consistency in the generated code.
…g logic - Introduced Capability_BYOCExternal constant to represent the new BYOC external capability. - Updated CapabilityNameLookup to include a name for BYOC external capability. - Enhanced GetCapabilitiesPrices method in orchestrator to append pricing for BYOC external capabilities, ensuring seamless integration with existing pricing logic.
- Deleted the GetCapabilities method and its associated ExternalCapabilityInfo structure from the BYOC orchestrator. - Removed the "/byoc/capabilities" route from the HTTP mux. - Updated the Orchestrator interface to eliminate the ExternalCapabilities method, streamlining capability management.
- Added a test case for handling missing capability constraints in BYOC requests, ensuring proper error messaging for invalid configurations. - Updated the GenerateLivePayment function to validate the BYOC capability against the orchestrator's price info, improving robustness in payment processing.
…ricing logic - Removed redundant checks for orch.node.Recipient in PriceInfo and PriceInfoForCaps methods. - Updated PriceInfo method to include BYOC capability and constraint handling, allowing remote signers to identify BYOC prices. - Enhanced priceInfo method to differentiate pricing retrieval for BYOC capabilities, ensuring accurate price fetching for jobs. - Adjusted conditions for auto price adjustment based on node state.
j0sh
left a comment
There was a problem hiding this comment.
Partially reviewed; I'll hold off on the rest until the BYOC job signature is updated.
| } | ||
|
|
||
| func (orch *orchestrator) PriceInfo(sender ethcommon.Address, manifestID ManifestID) (*net.PriceInfo, error) { | ||
| if orch.node == nil || orch.node.Recipient == nil { |
There was a problem hiding this comment.
How exactly was this being triggered? Should not really happen with a properly configured on-chain orchestrator (and hopefully earlier checks would catch any misconfiguration)
| dataToSign := req.Request + req.Parameters | ||
| sig, err := gw.Sign([]byte(dataToSign)) |
There was a problem hiding this comment.
I am afraid that this is going to have to change on the BYOC side; it's essentially an open signing oracle that signs arbitrarily user supplied data, which can be very dangerous. At a minimum there should be a stronger delineation between the fields along with some un-forgeable data, so the signature is less likely to be useful elsewhere. Something like:
sig = Sign( "LP_BYOC_SIGN_V1" || len(request) || request || len(parameters) || parameters) )
JSON is probably okay too, eg similar to how the remote signer's state is protected right now.
📌 Summary
This PR refines the remote signer payment flow for BYOC jobs by deriving the BYOC capability from
OrchestratorInfo.price_info.constraintinstead of accepting it directly in the request payload.🧠 What Changed
✨ Behavior updates
capabilityfield fromRemotePaymentRequest.price_info.constraintwhenprice_info.capabilityisBYOC.manifest_idis not provided, BYOC requests use the resolved capability constraint as the manifest ID for shared balance tracking.400if the orchestrator metadata does not include a valid BYOC capability constraint.🔧 Implementation details
GenerateLivePaymentinserver/remote_signer.goto compute and validate BYOC capability from orchestrator price info.server/remote_signer_test.go.🧪 Testing
400with:missing BYOC capability in OrchestratorInfo price_info.constraint📎 Related
☑️ Checklist