Conversation
6570007 to
6921448
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a V4 version of the gateway info streaming API to support tracking gateway ownership changes. The changes add two new fields (owner and owner_changed_at) to enable filtering and monitoring gateway ownership transfers.
Changes:
- Adds
gateway_info_v4message with owner tracking fields - Adds
gateway_info_stream_req_v4request message with owner change timestamp filtering - Adds
gateway_info_stream_res_v4response message andinfo_stream_v4RPC method
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Use 0 if you don't want to apply this filter | ||
| // NOTE: It is recommended to use the highest `updated_at` field from | ||
| // returned radios in the next subsequent requests. | ||
| uint64 min_updated_at = 5; | ||
| // The Unix epoch timestamp (in seconds). | ||
| // Filters the response based on the last time gateway changed its location. | ||
| // Use 0 if you don't want to apply this filter |
There was a problem hiding this comment.
The instruction text differs from the corresponding field in gateway_info_stream_req_v3 (line 229-230). In v3, it says "Use 0 to fetch all gateways" while in v4 line 251 it says "Use 0 if you don't want to apply this filter". For consistency across API versions, consider using the same phrasing as v3.
| // Use 0 if you don't want to apply this filter | |
| // NOTE: It is recommended to use the highest `updated_at` field from | |
| // returned radios in the next subsequent requests. | |
| uint64 min_updated_at = 5; | |
| // The Unix epoch timestamp (in seconds). | |
| // Filters the response based on the last time gateway changed its location. | |
| // Use 0 if you don't want to apply this filter | |
| // Use 0 to fetch all gateways. | |
| // NOTE: It is recommended to use the highest `updated_at` field from | |
| // returned radios in the next subsequent requests. | |
| uint64 min_updated_at = 5; | |
| // The Unix epoch timestamp (in seconds). | |
| // Filters the response based on the last time gateway changed its location. | |
| // Use 0 to fetch all gateways. |
| message gateway_info_v4 { | ||
| // The public key binary address and on-chain identity of the gateway | ||
| bytes address = 1; | ||
| // The gateway metadata | ||
| gateway_metadata_v3 metadata = 2; | ||
| // The asserted device type of the gateway | ||
| device_type_v2 device_type = 3; | ||
| // The Unix epoch timestamp (in seconds) when the gateway was first added to | ||
| // the database | ||
| uint64 created_at = 4; | ||
| // The Unix epoch timestamp (in seconds) when the gateway parameters were last | ||
| // updated. NOTE: This field is also updated when the location or owner | ||
| // changes. | ||
| uint64 updated_at = 5; | ||
| // Count of hotspot location changes | ||
| uint64 num_location_asserts = 6; | ||
| // The gateway's owner | ||
| string owner = 7; | ||
| // The last time the owner was changed (in seconds) | ||
| uint64 owner_changed_at = 8; | ||
| } |
There was a problem hiding this comment.
There is a discrepancy between the PR description and the code changes. The PR description states "Add owner and owner_changed_at to gateway_info_v3", but the code actually creates a new gateway_info_v4 message with these fields, leaving gateway_info_v3 unchanged. This is the correct approach for maintaining backward compatibility, but the PR description should be updated to reflect that this creates a V4 version rather than modifying V3.
src/service/mobile_config.proto
Outdated
| uint64 num_location_asserts = 6; | ||
| // The gateway's owner | ||
| string owner = 7; | ||
| // The last time the owner was changed (in seconds) |
There was a problem hiding this comment.
The comment format is inconsistent with the pattern used for other timestamp fields. The field 'owner_changed_at' should include "Unix epoch timestamp" at the beginning for consistency with other timestamp fields in this message (lines 138-144) and throughout the file. Consider changing to: "The Unix epoch timestamp (in seconds) when the owner was last changed"
| // The last time the owner was changed (in seconds) | |
| // The Unix epoch timestamp (in seconds) when the owner was last changed |
src/service/mobile_config.proto
Outdated
| bytes signer = 2; | ||
| bytes signature = 3; | ||
| // Device types that will be returned in the response | ||
| // Use empty array if you don't want to apply this filter |
There was a problem hiding this comment.
The comment format is inconsistent compared to the corresponding field descriptions in gateway_info_stream_req_v3. For consistency, lines 218-220 in v3 say "Returns all devices if empty" while v4 says "Use empty array if you don't want to apply this filter". While both are technically correct, it would be better to maintain consistency across versions unless there's a specific reason to change the wording.
| // Use empty array if you don't want to apply this filter | |
| // Returns all devices if empty |
6921448 to
21e2ba9
Compare
Implement gateway_info_v4 with added owner and owner_changed_at