Precise HTTP or WebSockets for JSON-RPC options #9027

Tbaut · 2018-07-02T14:24:56Z

Align with https://github.com/ethereum/go-ethereum/wiki/Command-Line-Options for the naming
Make sure the "JSON-RPC" naming is consistent across the CLI options

- Align with https://github.com/ethereum/go-ethereum/wiki/Command-Line-Options - Make sure the JSON-RPC naming is consistent across the CLI options

parity-cla-bot · 2018-07-02T14:24:59Z

It looks like @Tbaut signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

5chdn · 2018-07-02T14:29:44Z

parity/cli/mod.rs

@@ -315,7 +315,7 @@ usage! {

 			ARG arg_ports_shift: (u16) = 0u16, or |c: &Config| c.misc.as_ref()?.ports_shift,
 			"--ports-shift=[SHIFT]",
-			"Add SHIFT to all port numbers Parity is listening on. Includes network port and all servers (RPC, WebSockets, UI, IPFS, SecretStore).",
+			"Add SHIFT to all port numbers Parity is listening on. Includes network port and all servers (HTTP JSON-RPC, WebSockets JSON-RPC, UI, IPFS, SecretStore).",


Off-topic: Can you remove UI here while we are at it?

5chdn · 2018-07-02T14:31:08Z

parity/cli/mod.rs

@@ -646,7 +646,7 @@ usage! {

 			FLAG flag_tx_queue_no_unfamiliar_locals: (bool) = false, or |c: &Config| c.mining.as_ref()?.tx_queue_no_unfamiliar_locals.clone(),
 			"--tx-queue-no-unfamiliar-locals",
-			"Transactions received via local means (RPC, WS, etc) will be treated as external if the sending account is unknown.",
+			"Transactions received via local means (HTTP JSON-RPC, WS JSON-RPC, etc) will be treated as external if the sending account is unknown.",


Here: WebSockets or WS?

👍 I left it as "ws" was not part of the flag, but you're right, I'll change it to WebSockets.

dvdplm

Some wording/spelling grumbles.

dvdplm · 2018-07-03T10:32:47Z

parity/cli/mod.rs

@@ -315,7 +315,7 @@ usage! {

 			ARG arg_ports_shift: (u16) = 0u16, or |c: &Config| c.misc.as_ref()?.ports_shift,
 			"--ports-shift=[SHIFT]",
-			"Add SHIFT to all port numbers Parity is listening on. Includes network port and all servers (RPC, WebSockets, UI, IPFS, SecretStore).",
+			"Add SHIFT to all port numbers Parity is listening on. Includes network port and all servers (HTTP JSON-RPC, WebSockets JSON-RPC, IPFS, SecretStore).",


s/port/ports/ (plural s)

Also, not sure what is meant by "all servers" – perhaps "services" is better? That is, how is "notwork ports" different from other ports?

Maybe this: "Add SHIFT to all network port numbers used by Parity. This includes ports used by any enabled services (JSON-RPC over HTTP/WS, IPFS and SecretStore)." ?

the plural is on the "number*s", I think it's correct.
Network as in network discovery (30303 per default). This is the port used to connect to other peers.
Parity run http and ws servers to listen to incoming calls, I think "server" is fine here.

dvdplm · 2018-07-03T10:33:35Z

parity/cli/mod.rs

@@ -449,59 +449,59 @@ usage! {
 			"--reserved-peers=[FILE]",
 			"Provide a file containing enodes, one per line. These nodes will always have a reserved slot on top of the normal maximum peers.",

-		["API and Console Options – RPC"]
+		["API and Console Options – HTTP JSON-RPC"]


Maybe "…JSON-RPC over HTTP"?

I'd like to keep it short.

dvdplm · 2018-07-03T10:33:54Z

parity/cli/mod.rs

 			FLAG flag_no_jsonrpc: (bool) = false, or |c: &Config| c.rpc.as_ref()?.disable.clone(),
 			"--no-jsonrpc",
-			"Disable the JSON-RPC API server.",
+			"Disable the HTTP JSON-RPC API server.",


maybe "JSON-RPC HTTP API server"?

dvdplm · 2018-07-03T10:35:42Z

parity/cli/mod.rs


 			ARG arg_jsonrpc_apis: (String) = "web3,eth,pubsub,net,parity,private,parity_pubsub,traces,rpc,shh,shh_pubsub", or |c: &Config| c.rpc.as_ref()?.apis.as_ref().map(|vec| vec.join(",")),
 			"--jsonrpc-apis=[APIS]",
-			"Specify the APIs available through the JSONRPC interface using a comma-delimited list of API names. Possible names are: all, safe, web3, net, eth, pubsub, personal, signer, parity, parity_pubsub, parity_accounts, parity_set, traces, rpc, secretstore, shh, shh_pubsub. You can also disable a specific API by putting '-' in the front, example: all,-personal. safe contains following apis: web3, net, eth, pubsub, parity, parity_pubsub, traces, rpc, shh, shh_pubsub",
+			"Specify the APIs available through the HTTP JSON-RPC interface using a comma-delimited list of API names. Possible names are: all, safe, web3, net, eth, pubsub, personal, signer, parity, parity_pubsub, parity_accounts, parity_set, traces, rpc, secretstore, shh, shh_pubsub. You can also disable a specific API by putting '-' in the front, example: all,-personal. safe contains following apis: web3, net, eth, pubsub, parity, parity_pubsub, traces, rpc, shh, shh_pubsub",


maybe . "safe" enables the following APIs: web3, net … …?

dvdplm · 2018-07-03T10:37:57Z

parity/cli/mod.rs


 			ARG arg_jsonrpc_hosts: (String) = "none", or |c: &Config| c.rpc.as_ref()?.hosts.as_ref().map(|vec| vec.join(",")),
 			"--jsonrpc-hosts=[HOSTS]",
 			"List of allowed Host header values. This option will validate the Host header sent by the browser, it is additional security against some attack vectors. Special options: \"all\", \"none\",.",

 			ARG arg_jsonrpc_threads: (usize) = 4usize, or |c: &Config| c.rpc.as_ref()?.processing_threads,
 			"--jsonrpc-threads=[THREADS]",
-			"Turn on additional processing threads in all RPC servers. Setting this to non-zero value allows parallel cpu-heavy queries execution.",
+			"Turn on additional processing threads in all HTTP JSON-RPC servers. Setting this to non-zero value allows parallel cpu-heavy queries execution.",


s/parallel cpu-heavy queries execution./parallel execution of cpu-heavy queries/

dvdplm · 2018-07-03T10:40:20Z

parity/cli/mod.rs


 			ARG arg_ws_apis: (String) = "web3,eth,pubsub,net,parity,parity_pubsub,private,traces,rpc,shh,shh_pubsub", or |c: &Config| c.websockets.as_ref()?.apis.as_ref().map(|vec| vec.join(",")),
 			"--ws-apis=[APIS]",
-			"Specify the APIs available through the WebSockets interface using a comma-delimited list of API names. Possible names are: all, safe, web3, net, eth, pubsub, personal, signer, parity, parity_pubsub, parity_accounts, parity_set, traces, rpc, secretstore, shh, shh_pubsub. You can also disable a specific API by putting '-' in the front, example: all,-personal. safe contains following apis: web3, net, eth, pubsub, parity, parity_pubsub, traces, rpc, shh, shh_pubsub",
+			"Specify the JSON-RPC APIs available through the WebSockets interface using a comma-delimited list of API names. Possible names are: all, safe, web3, net, eth, pubsub, personal, signer, parity, parity_pubsub, parity_accounts, parity_set, traces, rpc, secretstore, shh, shh_pubsub. You can also disable a specific API by putting '-' in the front, example: all,-personal. safe contains following apis: web3, net, eth, pubsub, parity, parity_pubsub, traces, rpc, shh, shh_pubsub",


maybe . "safe" enables the following APIs: web3, net … …?

dvdplm · 2018-07-03T10:40:54Z

parity/cli/mod.rs

@@ -513,7 +513,7 @@ usage! {

 			ARG arg_ws_max_connections: (usize) = 100usize, or |c: &Config| c.websockets.as_ref()?.max_connections,
 			"--ws-max-connections=[CONN]",
-			"Maximal number of allowed concurrent WS connections.",
+			"Maximal number of allowed concurrent WebSockets JSON-RPC connections.",


s/Maximal/Maximum/ (or just max)

dvdplm · 2018-07-03T10:44:15Z

parity/cli/mod.rs

@@ -646,7 +646,7 @@ usage! {

 			FLAG flag_tx_queue_no_unfamiliar_locals: (bool) = false, or |c: &Config| c.mining.as_ref()?.tx_queue_no_unfamiliar_locals.clone(),
 			"--tx-queue-no-unfamiliar-locals",
-			"Transactions received via local means (RPC, WS, etc) will be treated as external if the sending account is unknown.",
+			"Transactions received via local means (HTTP JSON-RPC, WebSockets JSON-RPC, etc) will be treated as external if the sending account is unknown.",


The ambiguity of "means" in english makes me stumble when I read this.
How about "Transactions received through JSON-RPC (HTTP or WS) will be treated as external if the sending account is unknown."

dvdplm · 2018-07-03T10:44:59Z

parity/cli/mod.rs

@@ -885,7 +885,7 @@ usage! {

 			FLAG flag_dapps_apis_all: (bool) = false, or |_| None,
 			"--dapps-apis-all",
-			"Dapps server is merged with RPC server. Use --jsonrpc-apis.",
+			"Dapps server is merged with HTTP JSON-RPC server. Use --jsonrpc-apis.",


Should we prefix with "DEPRECATED. Dapps server is merged… …"?

dvdplm · 2018-07-03T10:45:46Z

parity/cli/mod.rs

@@ -913,7 +913,7 @@ usage! {

 			FLAG flag_jsonrpc: (bool) = false, or |_| None,
 			"-j, --jsonrpc",
-			"Does nothing; JSON-RPC is on by default now.",
+			"Does nothing; HTTP JSON-RPC is on by default now.",


Need DEPRECATED prefix? Also, I for one prefer "JSON-RPC over HTTP".

(same below)

Tbaut · 2018-07-03T12:12:23Z

Valid comments but out of scope for this PR @dvdplm I'm afraid :(

5chdn · 2018-07-03T13:44:08Z

@dvdplm the deprecated prefix is not necessary as those options are not displayed in --help anymore. It's more for historic reasons.

Tbaut · 2018-07-06T13:38:31Z

@dvdplm added some of your suggestions #9060

…rp_sync_on_light_client * 'master' of https://github.com/paritytech/parity: Never drop local transactions from different senders. (openethereum#9002) Precise HTTP or WebSockets for JSON-RPC options (openethereum#9027) Recently rejected cache for transaction queue (openethereum#9005) Make HashDB generic (openethereum#8739) Only return error log for rustls (openethereum#9025) Update Changelogs for 1.10.8 and 1.11.5 (openethereum#9012)

Precise HTTP or WS for JSON-RPC options

c56fa90

- Align with https://github.com/ethereum/go-ethereum/wiki/Command-Line-Options - Make sure the JSON-RPC naming is consistent across the CLI options

Tbaut added A0-pleasereview 🤓 Pull request needs code review. A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). M4-core ⛓ Core client code / Rust. labels Jul 2, 2018

Tbaut requested review from folsen and dvdplm July 2, 2018 14:24

5chdn added this to the 1.12 milestone Jul 2, 2018

5chdn approved these changes Jul 2, 2018

View reviewed changes

fix comments

43ff5ad

Tbaut changed the title ~~Precise HTTP or WS for JSON-RPC options~~ Precise HTTP or WebSockets for JSON-RPC options Jul 2, 2018

sorpaas approved these changes Jul 2, 2018

View reviewed changes

5chdn added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 3, 2018

5chdn merged commit f310721 into master Jul 3, 2018

5chdn deleted the Tbaut-patch-JSON-RPC branch July 3, 2018 09:34

dvdplm reviewed Jul 3, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Precise HTTP or WebSockets for JSON-RPC options #9027

Precise HTTP or WebSockets for JSON-RPC options #9027

Tbaut commented Jul 2, 2018

parity-cla-bot commented Jul 2, 2018

5chdn Jul 2, 2018

Tbaut Jul 2, 2018

5chdn Jul 2, 2018

Tbaut Jul 2, 2018

dvdplm left a comment

dvdplm Jul 3, 2018

Tbaut Jul 6, 2018 •

edited

Loading

dvdplm Jul 3, 2018

Tbaut Jul 6, 2018

dvdplm Jul 3, 2018

dvdplm Jul 3, 2018

Tbaut Jul 6, 2018

dvdplm Jul 3, 2018

Tbaut Jul 6, 2018

dvdplm Jul 3, 2018

Tbaut Jul 6, 2018

dvdplm Jul 3, 2018

Tbaut Jul 6, 2018

dvdplm Jul 3, 2018

Tbaut Jul 6, 2018

dvdplm Jul 3, 2018

dvdplm Jul 3, 2018

dvdplm Jul 3, 2018

Tbaut commented Jul 3, 2018

5chdn commented Jul 3, 2018

Tbaut commented Jul 6, 2018

Precise HTTP or WebSockets for JSON-RPC options #9027

Precise HTTP or WebSockets for JSON-RPC options #9027

Conversation

Tbaut commented Jul 2, 2018

parity-cla-bot commented Jul 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dvdplm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tbaut Jul 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tbaut commented Jul 3, 2018

5chdn commented Jul 3, 2018

Tbaut commented Jul 6, 2018

Tbaut Jul 6, 2018 •

edited

Loading