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

[Sui CLI] - Support full node #4404

Merged
merged 2 commits into from
Sep 8, 2022
Merged

Conversation

patrickkuo
Copy link
Contributor

@patrickkuo patrickkuo commented Aug 31, 2022

  • removed fullnode from client config (client.yaml)
  • renamed gateway config to rpc, this can be used to point to either gateway or fullnode
  • retrieve RPC server supported methods using rpc.discover method and determine if the client is connecting to gateway or fullnode
  • print rpc server's available methods on sui console startup
  • fixed issue Sui Cli Bug: URL scheme not supported #4164

todos:

  • Support event subscription in Sui CLI
  • Support calling all rpc methods
  • Use fullnode rpc in test instead of gateway

@patrickkuo patrickkuo linked an issue Aug 31, 2022 that may be closed by this pull request
@patrickkuo patrickkuo marked this pull request as ready for review September 1, 2022 15:01
@patrickkuo patrickkuo self-assigned this Sep 1, 2022
@github-actions github-actions bot added the Type: Documentation Improvements or additions to documentation label Sep 1, 2022
@patrickkuo patrickkuo requested review from oxade and removed request for Clay-Mysten September 6, 2022 17:33
Copy link
Contributor

@oxade oxade left a comment

Choose a reason for hiding this comment

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

Looks good, but a small request as usual whenever we change CLI stuff: can we show a sample usage/output?

Comment on lines +173 to +179
(ServerHandle::HttpHandler(handle, addr), "JSON-RPC")
}
ServerBuilder::WsBuilder(ws_builder) => {
let server = ws_builder.build(listen_address).await?;
let addr = server.local_addr()?;
let handle = server.start(self.module)?;
(ServerHandle::WsHandle(handle), addr, "Websocket")
(ServerHandle::WsHandle(handle, addr), "Websocket")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit, but please use constant for the strs

.wallet_sync_api()
.sync_account_state(player_x)
.await?;
if self.client.is_gateway() {
Copy link
Contributor

Choose a reason for hiding this comment

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

We will remove gateway completely right? When?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, we need to do some test to make sure equivocation will rarely happens in a reasonable sized network (20 validators) before we can safely remove the gateway.

Comment on lines +75 to +122
pub async fn new(http: &str, ws: Option<&str>) -> Result<Self, anyhow::Error> {
let http = HttpClientBuilder::default().build(http)?;
let ws = if let Some(url) = ws {
Some(WsClientBuilder::default().build(url).await?)
} else {
None
};
let info = Self::get_server_info(&http, &ws).await?;
Ok(Self { http, ws, info })
}

async fn get_server_info(
http: &HttpClient,
ws: &Option<WsClient>,
) -> Result<ServerInfo, anyhow::Error> {
let rpc_spec: Value = http
.request("rpc.discover", None)
.await
.map_err(|e| anyhow!("Fail to connect to the RPC server: {e}"))?;
let version = rpc_spec
.pointer("/info/version")
.and_then(|v| v.as_str())
.ok_or_else(|| anyhow!("Fail parsing server version from rpc.discover endpoint."))?;
let rpc_methods = Self::parse_methods(&rpc_spec)?;

let subscriptions = if let Some(ws) = ws {
let rpc_spec: Value = ws
.request("rpc.discover", None)
.await
.map_err(|e| anyhow!("Fail to connect to the Websocket server: {e}"))?;
Self::parse_methods(&rpc_spec)?
} else {
Vec::new()
};
Ok(ServerInfo {
rpc_methods,
subscriptions,
version: version.to_string(),
})
}

fn parse_methods(server_spec: &Value) -> Result<Vec<String>, anyhow::Error> {
let methods = server_spec
.pointer("/methods")
.and_then(|methods| methods.as_array())
.ok_or_else(|| {
anyhow!("Fail parsing server information from rpc.discover endpoint.")
})?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit. Raw strings like this really bother me :p but it could be okay

…connecting to gateway or fullnode

fixed issue #4164
print rpc server's available methods on sui console startup
@patrickkuo
Copy link
Contributor Author

FYI @brad-mysten @tharbert this PR changed the client.yaml, this might break operations?

@patrickkuo patrickkuo merged commit cf5584f into main Sep 8, 2022
@patrickkuo patrickkuo deleted the pat/sui_cli_full_node_support branch September 8, 2022 23:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sui Cli Bug: URL scheme not supported
2 participants