Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/675 on chain ouis devaddrs #435

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 11 additions & 79 deletions src/service/iot_config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@ import "region.proto";
// are signed by the config service to allow the recipient to validate
// the authenticity of the data returned.
//
// - Every key called `owner`, `payer` and `delegate_keys` are binary
// encoded public keys, Rust encoding example here:
// https://github.com/helium/helium-crypto-rs/blob/main/src/public_key.rs#L347-L354
// - Every key called `owner`, and `delegate_keys` are solana pubkeys
// https://docs.rs/solana-sdk/1.18.0/solana_sdk/pubkey/struct.Pubkey.html
//
// == DevAddrs ==
//
Expand All @@ -43,14 +42,18 @@ enum action_v1 {
// Define an organisation
message org_v1 {
uint64 oui = 1;
// On chain address
bytes address = 2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can't change this proto like this without breaking any existing clients. we can add fields and that should be relatively safe for the old stuff to handle but changing the order of existing fields or adding fields in order ahead of existing fields will need an org_v2 message to avoid a breaking change. unless we want to consider updating the "official cli" and the client in the iot oracles the only priority and let any other consumers deal with the fallout.

maybe that's acceptable in this special case since the shift to solana for apis using this particular data structure are going to be broken without an update no matter what ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point actually, we need to decide if we want to v2 or nuke v1? I think we would be broken no matter what but maybe we v2, deprecate v1 and remove it at a later date.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only reason I don’t think it’s straightforward is b/c there are still methods that return that data structure that by the proto spec would be expected to still work

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the data structure is going to change completely once we do the shift. I can do the v2 and deprecate v1 if thats the consensus?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fair to me

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think I deprecated it properly, lemme know if thats how we want to do it

// Org admin key
bytes owner = 2;
// Key only used for DC payments
bytes payer = 3;
bytes owner = 3;
// Used to infer escrow address for DC payments
string escrow_key = 4;
// List of keys allowed some specific actions, see services.
repeated bytes delegate_keys = 4;
repeated bytes delegate_keys = 5;
// Is org approved on chain
bool approved = 6;
// Is org locked because of no payment
bool locked = 5;
bool locked = 7;
}

// Device address range, ex: 16#00000001 to 16#0000000A
Expand Down Expand Up @@ -150,69 +153,6 @@ message org_list_res_v1 {

message org_get_req_v1 { uint64 oui = 1; }

message org_create_helium_req_v1 {
enum helium_net_id {
type0_0x00003c = 0;
type3_0x60002d = 1;
type6_0xc00053 = 2;
}

bytes owner = 1;
bytes payer = 2;
// Number of device address needed
// Even number required, minimum of 8
uint64 devaddrs = 3;
// in milliseconds since unix epoch
uint64 timestamp = 4;
bytes signature = 5;
repeated bytes delegate_keys = 6;
// pubkey binary of the signing keypair
bytes signer = 7;
helium_net_id net_id = 8;
}

message org_create_roamer_req_v1 {
bytes owner = 1;
bytes payer = 2;
uint32 net_id = 3;
// in milliseconds since unix epoch
uint64 timestamp = 4;
bytes signature = 5;
repeated bytes delegate_keys = 6;
// pubkey binary of the signing keypair
bytes signer = 7;
}

message org_update_req_v1 {
message delegate_key_update_v1 {
bytes delegate_key = 1;
action_v1 action = 2;
}

message devaddr_constraint_update_v1 {
devaddr_constraint_v1 constraint = 1;
action_v1 action = 2;
}

message update_v1 {
oneof update {
bytes owner = 1;
bytes payer = 2;
delegate_key_update_v1 delegate_key = 3;
// count of devaddrs to add, in even numbers
uint64 devaddrs = 4;
// devaddr constraints to explicitly add or remove
devaddr_constraint_update_v1 constraint = 5;
}
}

uint64 oui = 1;
repeated update_v1 updates = 2;
uint64 timestamp = 3;
bytes signer = 4;
bytes signature = 5;
}

message org_res_v1 {
org_v1 org = 1;
uint32 net_id = 2;
Expand Down Expand Up @@ -649,14 +589,6 @@ service org {
rpc list(org_list_req_v1) returns (org_list_res_v1);
// Get Org (no auth)
rpc get(org_get_req_v1) returns (org_res_v1);
// Create Org on Helium Network (auth admin only)
rpc create_helium(org_create_helium_req_v1) returns (org_res_v1);
// Create Org on any network (auth admin only)
rpc create_roamer(org_create_roamer_req_v1) returns (org_res_v1);
// Update any Org (Helium or Roaming)
// Modify payer and add/remove delegate keys (owner/admin)
// Modify owner and add/remove devaddr constraints (auth admin only)
rpc update(org_update_req_v1) returns (org_res_v1);
// Disable an org, this sends a stream route delete update to HPR
// for all associated routes (auth admin only)
rpc disable(org_disable_req_v1) returns (org_disable_res_v1);
Expand Down
Loading