Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the gRPC GenerateKey response shape by extracting a dedicated KeyGenerationResult message and nesting it under GenerateKeyResponse.result, updating the worker processor and documentation accordingly.
Changes:
- Update
GenerateKeyResponsein both protobuf and TypeScript types to wrap key material inresult: KeyGenerationResult. - Adjust key-generation queue processor to read
publicKey/publicKeyPackagefromresponse.result. - Tweak README API docs (add
Algorithmenum reference link; remove section separators).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/queue/key-generation.processor.ts | Updates processor to consume the new nested GenerateKeyResponse.result shape. |
| src/grpc/grpc.types.ts | Introduces KeyGenerationResult and refactors GenerateKeyResponse to reference it. |
| proto/engine/v1/engine.proto | Extracts KeyGenerationResult message and nests it under GenerateKeyResponse. |
| README.md | Adds Algorithm enum documentation link and adjusts markdown formatting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const publicKey: string = Buffer.from( | ||
| result.value.result.publicKey, | ||
| ).toString("hex"); | ||
| const publicKeyPackage: string = Buffer.from( | ||
| result.value.publicKeyPackage, | ||
| result.value.result.publicKeyPackage, |
There was a problem hiding this comment.
GenerateKeyResponse is now expected to contain a nested result message. This PR updates the processor, but existing unit tests/mocks still return the old shape { publicKey, publicKeyPackage } (e.g. src/queue/key-generation.processor.spec.ts), which will make the test suite fail. Update the mocks (and any other call sites) to return { result: { publicKey, publicKeyPackage } } instead.
| /** Generated public key and public key package. */ | ||
| KeyGenerationResult result = 1; |
There was a problem hiding this comment.
This changes the wire shape of GenerateKeyResponse from two top-level bytes fields to a nested message (result). That is a breaking protobuf change: older clients will interpret field 1 as raw bytes of the embedded message (not the canonical public key) and will fail in non-obvious ways. If backward compatibility matters, keep the original fields (deprecated) alongside the new result, or introduce a new RPC / service version instead of changing the existing response shape.
| /** Generated public key and public key package. */ | |
| KeyGenerationResult result = 1; | |
| /** | |
| * Canonical public key bytes. | |
| * Deprecated: prefer result.public_key. Kept for wire compatibility with | |
| * older clients. | |
| */ | |
| bytes public_key = 1 [deprecated = true]; | |
| /** | |
| * Opaque protocol-internal serialization of the public key package. | |
| * Deprecated: prefer result.public_key_package. Kept for wire compatibility | |
| * with older clients. | |
| */ | |
| bytes public_key_package = 2 [deprecated = true]; | |
| /** Generated public key and public key package. */ | |
| KeyGenerationResult result = 3; |
| - `Algorithm`: see [documentation](https://shreeed-app.github.io/multi-party-computation-controller-api/enums/grpc_grpc.types.Algorithm.html) for possible enum values. | ||
|
|
||
| **Response:** `202 Accepted` |
There was a problem hiding this comment.
README still documents the engine's GenerateKey response as { publicKey, publicKeyPackage } in the key-generation Mermaid sequence diagram, but the proto/types in this PR now wrap those fields under result. Please update the diagram (and any other references) to match the new response shape to avoid misleading API consumers.
…yResponse structure
No description provided.