Skip to content
This repository has been archived by the owner on Jan 11, 2024. It is now read-only.

Include gateway addr in subnet config #137

Merged
merged 8 commits into from
Mar 30, 2023
Merged

Include gateway addr in subnet config #137

merged 8 commits into from
Mar 30, 2023

Conversation

cryptoAtwill
Copy link
Contributor

The changes in this PR:

  • Add gateway address and network name in subnet config.
  • Replace subnets from Hashmap<String, Subnet> to Hashmap<SubnetID, Subnet>, so that all the look ups we can directly use subnet id instead of converting to string.
  • Updated subnets config from key value map style to array.

@cryptoAtwill cryptoAtwill requested review from adlrocha and hmoniz March 28, 2023 09:29
src/manager/checkpoint.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@adlrocha adlrocha left a comment

Choose a reason for hiding this comment

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

A few minor comments (they are non-blocking, so feel free to disregard them if you don't feel they apply right now).

And the reason for the request for changes. IIRC we were using DEFAULT_GATEWAY_ADDR all over the place whenver there was a request that required the gateway as an argument. I don´t see that changed in the code, did I miss it or should we change that also before we merge and pick up the gateway address from the config.

jsonrpc_api_http = "https://example.org/rpc/v0"
jsonrpc_api_ws = "wss://example.org/rpc/v0"
auth_token = "YOUR ROOT AUTH TOKEN"

[subnets.child]
[[subnets]]
Copy link
Contributor

Choose a reason for hiding this comment

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

From total ignorance, would it be possible to make the config something like so that we use the id already as the key of the map?:

[subnets]

[[/root]]
....
[[/root/t01011]]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one I'm not so sure, but I think it cannot be done, because [[/root]] would translate to hashmap of "/root" as key and whatever in the ... as an array of deserialized struct. But I think we should avoid using subnet id string because of /, everytime we have to escape it with ", which is easy to make mistake.

id = "/root"
network_name = "root"
gateway_addr = 64
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of a curiosity, why did you use a u64 instead of an Address here? I can think of a few reasons why we may want to do this, but I don´t know if it would be the best approach for users. @hmoniz, thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

perhaps, it's just convenience I guess. Convert actor id to address is quite straightforward. If we want to use string, it works too.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's either use gateway_addr = "t064" or let's call this field gateway_id and make it an uint. Whatever works best for you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

use the first option, please check

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants