Ecdsa#2
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the protocol type handling by moving protocol-related code into a dedicated protocol.go file and adds support for gRPC-based signing services. The changes consolidate protocol type definitions, add gRPC service definitions for signing operations, and update dependency versions.
Key changes:
- Extracted protocol type definitions and helper functions from
message.gointo a newprotocol.gofile - Refactored
TrackingID.GetProtocolType()to use the newProtocolTypeFromInt()helper function - Added gRPC service definitions for signer service with bidirectional streaming support
- Updated Go version and dependencies (gRPC, protobuf) with improved protobuf generation workflow
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| protocol.go | New file containing protocol type definitions, conversions, and validation functions extracted from message.go |
| util.go | Simplified GetProtocolType() to delegate to ProtocolTypeFromInt() |
| message.go | Removed protocol type definitions that were moved to protocol.go |
| service/signer/signer.pb.go | Generated protobuf code for signer service messages |
| service/signer/signer_grpc.pb.go | Generated gRPC service code for bidirectional signing streams |
| proto/signer.proto | New protobuf definition for signer service with SignRequest, SignResponse, and SignStatus messages |
| proto/io.proto | Updated go_package path to use full GitHub module path |
| io.pb.go | Regenerated with updated go_package path |
| go.mod | Updated Go version to 1.24.0, added gRPC dependency, updated protobuf version |
| go.sum | Added checksums for new and updated dependencies |
| Makefile | Enhanced protobuf generation to support multiple proto files and gRPC generation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // Integer protocol identifiers (useful for internal indexing, enums, etc.) | ||
| const ( | ||
| protocolTypeMin = iota |
There was a problem hiding this comment.
You could define a type called ProtocolType and define it to be uint8
and then use iota to set the constants. This would save needing protocolTypeMin == 0 (since it's uint)
There was a problem hiding this comment.
I'm not sure about that, since protocolType.ToInt() returns -1 for a bad protocol type.
Should it return an error instead?
* new signer api: key updating * docs * changed keyType * added comments to the new api
Little refactor and utilities.
Added the Signer protocol to create a signer service.