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

Adding RPC Configuration #531

Merged
merged 5 commits into from
Jul 7, 2020
Merged

Adding RPC Configuration #531

merged 5 commits into from
Jul 7, 2020

Conversation

RajarupanSampanthan
Copy link
Contributor

Summary of changes
Notable changes introduced in this pull request:

  • Adds some basic RPC configuration via a TOML file.
  • Changes the Config struct in forest/src/cli/config.rs to have 3 new parameters
  • Updates the start_rpc function in node/rpc/src/lib.rs to take a rpc port and listen address

Reference issue to close (if applicable)

Closes #474
Closes #475

Other information and links

Copy link
Contributor

@austinabell austinabell left a comment

Choose a reason for hiding this comment

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

Ideally, the flags should be something like
--rpc=false to disable rpc (on by default)
--port="1234" to set the port

Anyone have opinions or suggestions for anything within this scope that should be added?

forest/src/cli/config.rs Outdated Show resolved Hide resolved
forest/src/cli/config.rs Outdated Show resolved Hide resolved
forest/src/cli/config.rs Outdated Show resolved Hide resolved
forest/src/cli/config.rs Outdated Show resolved Hide resolved
forest/src/daemon.rs Outdated Show resolved Hide resolved
node/rpc/src/lib.rs Outdated Show resolved Hide resolved
forest/src/daemon.rs Outdated Show resolved Hide resolved
forest/src/daemon.rs Outdated Show resolved Hide resolved
info!("Using {} as RPC port", rpc_port);
info!("Using {} as RPC listen", rpc_listen);
start_rpc(db_rpc, rpc_port, rpc_listen).await;
let rpc_listen = "127.0.0.1:".to_string() + &config.rpc_port;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let rpc_listen = "127.0.0.1:".to_string() + &config.rpc_port;
let rpc_listen = format!("127.0.0.1:{}", &config.rpc_port);

optional: just a bit more readable

@@ -35,7 +35,7 @@ pub(super) async fn start(config: Config) {
});

// Initialize database
let mut db = RocksDb::new(config.data_dir + "/db");
let mut db = RocksDb::new(config.data_dir.clone() + "/db");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let mut db = RocksDb::new(config.data_dir.clone() + "/db");
let mut db = RocksDb::new(config.data_dir + "/db");

nothing you added should change this, should it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was doing some dumb stuff earlier, before this set up i kept running into partial move issues. I thought my only way out of it was to clone stuff, should've changed this back when i started doing it this way.

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

Successfully merging this pull request may close these issues.

Enable/Disable RPC in CLI/Config Add CLI/Config for RPC Port and Address
5 participants