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

verify-chain node client arg #3678

Merged
merged 6 commits into from
Jan 7, 2022
Merged
Show file tree
Hide file tree
Changes from 5 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
80 changes: 65 additions & 15 deletions api/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,33 @@ use serde::{Deserialize, Serialize};
use std::time::Duration;
use tokio::runtime::Builder;

// Client Request Timeout
pub struct TimeOut {
pub connect: Duration,
pub read: Duration,
pub write: Duration,
}

impl TimeOut {
pub fn new(connect: u64, read: u64, write: u64) -> Self {
Self {
connect: Duration::from_secs(connect),
read: Duration::from_secs(read),
write: Duration::from_secs(write),
}
}
}

impl Default for TimeOut {
fn default() -> TimeOut {
TimeOut {
connect: Duration::from_secs(20),
read: Duration::from_secs(20),
write: Duration::from_secs(20),
}
}
}

/// Helper function to easily issue a HTTP GET request against a given URL that
/// returns a JSON object. Handles request building, JSON deserialization and
/// response code checking.
Expand All @@ -35,7 +62,10 @@ pub fn get<T>(url: &str, api_secret: Option<String>) -> Result<T, Error>
where
for<'de> T: Deserialize<'de>,
{
handle_request(build_request(url, "GET", api_secret, None)?)
handle_request(
build_request(url, "GET", api_secret, None)?,
TimeOut::default(),
)
}

/// Helper function to easily issue an async HTTP GET request against a given
Expand All @@ -52,21 +82,29 @@ where
/// on a given URL that returns nothing. Handles request
/// building and response code checking.
pub fn get_no_ret(url: &str, api_secret: Option<String>) -> Result<(), Error> {
send_request(build_request(url, "GET", api_secret, None)?)?;
send_request(
build_request(url, "GET", api_secret, None)?,
TimeOut::default(),
)?;
Ok(())
}

/// Helper function to easily issue a HTTP POST request with the provided JSON
/// object as body on a given URL that returns a JSON object. Handles request
/// building, JSON serialization and deserialization, and response code
/// checking.
pub fn post<IN, OUT>(url: &str, api_secret: Option<String>, input: &IN) -> Result<OUT, Error>
pub fn post<IN, OUT>(
url: &str,
api_secret: Option<String>,
input: &IN,
timeout: TimeOut,
) -> Result<OUT, Error>
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of adding Option parameters to multiple methods, isn't it preferable to add a new timeout member,
and a method to change it from its default value of 20secs?

Copy link
Contributor Author

@deevope deevope Dec 30, 2021

Choose a reason for hiding this comment

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

I did it a bit more cleaner but just adding a method in api/client.rs and trigger a change of timeout in function of the api method used in the request is a bit painful in the form it's built. If you have better idea, i'm open.
However these method are almost only used for cmd/client.rs

where
IN: Serialize,
for<'de> OUT: Deserialize<'de>,
{
let req = create_post_request(url, api_secret, input)?;
handle_request(req)
handle_request(req, timeout)
}

/// Helper function to easily issue an async HTTP POST request with the
Expand Down Expand Up @@ -94,7 +132,10 @@ pub fn post_no_ret<IN>(url: &str, api_secret: Option<String>, input: &IN) -> Res
where
IN: Serialize,
{
send_request(create_post_request(url, api_secret, input)?)?;
send_request(
create_post_request(url, api_secret, input)?,
TimeOut::default(),
)?;
Ok(())
}

Expand All @@ -110,7 +151,11 @@ pub async fn post_no_ret_async<IN>(
where
IN: Serialize,
{
send_request_async(create_post_request(url, api_secret, input)?).await?;
send_request_async(
create_post_request(url, api_secret, input)?,
TimeOut::default(),
)
.await?;
Ok(())
}

Expand Down Expand Up @@ -155,11 +200,11 @@ where
build_request(url, "POST", api_secret, Some(json))
}

fn handle_request<T>(req: Request<Body>) -> Result<T, Error>
fn handle_request<T>(req: Request<Body>, timeout: TimeOut) -> Result<T, Error>
where
for<'de> T: Deserialize<'de>,
{
let data = send_request(req)?;
let data = send_request(req, timeout)?;
serde_json::from_str(&data).map_err(|e| {
e.context(ErrorKind::ResponseError("Cannot parse response".to_owned()))
.into()
Expand All @@ -170,18 +215,23 @@ async fn handle_request_async<T>(req: Request<Body>) -> Result<T, Error>
where
for<'de> T: Deserialize<'de> + Send + 'static,
{
let data = send_request_async(req).await?;
let data = send_request_async(req, TimeOut::default()).await?;
let ser = serde_json::from_str(&data)
.map_err(|e| e.context(ErrorKind::ResponseError("Cannot parse response".to_owned())))?;
Ok(ser)
}

async fn send_request_async(req: Request<Body>) -> Result<String, Error> {
async fn send_request_async(req: Request<Body>, timeout: TimeOut) -> Result<String, Error> {
let https = hyper_rustls::HttpsConnector::new();
let (connect, read, write) = (
Some(timeout.connect),
Some(timeout.read),
Some(timeout.write),
);
let mut connector = TimeoutConnector::new(https);
connector.set_connect_timeout(Some(Duration::from_secs(20)));
connector.set_read_timeout(Some(Duration::from_secs(20)));
connector.set_write_timeout(Some(Duration::from_secs(20)));
connector.set_connect_timeout(connect);
connector.set_read_timeout(read);
connector.set_write_timeout(write);
let client = Client::builder().build::<_, Body>(connector);

let resp = client
Expand All @@ -205,11 +255,11 @@ async fn send_request_async(req: Request<Body>) -> Result<String, Error> {
Ok(String::from_utf8_lossy(&raw).to_string())
}

pub fn send_request(req: Request<Body>) -> Result<String, Error> {
pub fn send_request(req: Request<Body>, timeout: TimeOut) -> Result<String, Error> {
let mut rt = Builder::new()
.basic_scheduler()
.enable_all()
.build()
.map_err(|e| ErrorKind::RequestError(format!("{}", e)))?;
rt.block_on(send_request_async(req))
rt.block_on(send_request_async(req, timeout))
}
4 changes: 2 additions & 2 deletions api/src/handlers/chain_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ pub struct ChainValidationHandler {
}

impl ChainValidationHandler {
pub fn validate_chain(&self) -> Result<(), Error> {
pub fn validate_chain(&self, fast_validation: bool) -> Result<(), Error> {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not use assume_valid_rangeproofs_kernels here as well?

w(&self.chain)?
.validate(true)
.validate(fast_validation)
.map_err(|_| ErrorKind::Internal("chain error".to_owned()).into())
}
}
Expand Down
7 changes: 5 additions & 2 deletions api/src/owner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,20 @@ impl Owner {

/// Trigger a validation of the chain state.
///
/// # Arguments
/// * `assume_valid_rangeproofs_kernels` - if false, will validate rangeproofs, kernel signatures and sum of kernel excesses. if true, will only validate the sum of kernel excesses should equal the sum of unspent outputs minus total supply.
///
/// # Returns
/// * Result Containing:
/// * `Ok(())` if the validation was done successfully
/// * or [`Error`](struct.Error.html) if an error is encountered.
///

pub fn validate_chain(&self) -> Result<(), Error> {
pub fn validate_chain(&self, assume_valid_rangeproofs_kernels: bool) -> Result<(), Error> {
let chain_validation_handler = ChainValidationHandler {
chain: self.chain.clone(),
};
chain_validation_handler.validate_chain()
chain_validation_handler.validate_chain(assume_valid_rangeproofs_kernels)
}

/// Trigger a compaction of the chain state to regain storage space.
Expand Down
12 changes: 6 additions & 6 deletions api/src/owner_rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ pub trait OwnerRpc: Sync + Send {
{
"jsonrpc": "2.0",
"method": "validate_chain",
"params": [],
"params": ["false"],
"id": 1
}
# "#
Expand All @@ -100,7 +100,7 @@ pub trait OwnerRpc: Sync + Send {
# );
```
*/
fn validate_chain(&self) -> Result<(), ErrorKind>;
fn validate_chain(&self, assume_valid_rangeproofs_kernels: bool) -> Result<(), ErrorKind>;

/**
Networked version of [Owner::compact_chain](struct.Owner.html#method.compact_chain).
Expand Down Expand Up @@ -363,8 +363,8 @@ impl OwnerRpc for Owner {
Owner::get_status(self).map_err(|e| e.kind().clone())
}

fn validate_chain(&self) -> Result<(), ErrorKind> {
Owner::validate_chain(self).map_err(|e| e.kind().clone())
fn validate_chain(&self, assume_valid_rangeproofs_kernels: bool) -> Result<(), ErrorKind> {
Owner::validate_chain(self, assume_valid_rangeproofs_kernels).map_err(|e| e.kind().clone())
}

fn reset_chain_head(&self, hash: String) -> Result<(), ErrorKind> {
Expand Down Expand Up @@ -403,7 +403,7 @@ macro_rules! doctest_helper_json_rpc_owner_assert_response {
// create temporary grin server, run jsonrpc request on node api, delete server, return
// json response.

{
{
/*use grin_servers::test_framework::framework::run_doctest;
use grin_util as util;
use serde_json;
Expand Down Expand Up @@ -437,6 +437,6 @@ macro_rules! doctest_helper_json_rpc_owner_assert_response {
serde_json::to_string_pretty(&expected_response).unwrap()
);
}*/
}
}
};
}
3 changes: 2 additions & 1 deletion servers/src/mining/mine_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,8 @@ fn create_coinbase(dest: &str, block_fees: &BlockFees) -> Result<CbData, Error>

trace!("Sending build_coinbase request: {}", req_body);
let req = api::client::create_post_request(url.as_str(), None, &req_body)?;
let res: String = api::client::send_request(req).map_err(|e| {
let timeout = api::client::TimeOut::default();
let res: String = api::client::send_request(req, timeout).map_err(|e| {
let report = format!(
"Failed to get coinbase from {}. Is the wallet listening? {}",
dest, e
Expand Down
38 changes: 36 additions & 2 deletions src/bin/cmd/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,19 @@ impl HTTPNodeClient {
method: &str,
params: &serde_json::Value,
) -> Result<D, Error> {
let timeout = match method {
// 6 hours read timeout
"validate_chain" => client::TimeOut::new(20, 21600, 20),
_ => client::TimeOut::default(),
};
let url = format!("http://{}{}", self.node_url, ENDPOINT);
let req = build_request(method, params);
let res =
client::post::<Request, Response>(url.as_str(), self.node_api_secret.clone(), &req);
let res = client::post::<Request, Response>(
url.as_str(),
self.node_api_secret.clone(),
&req,
timeout,
);

match res {
Err(e) => {
Expand Down Expand Up @@ -149,6 +158,27 @@ impl HTTPNodeClient {
e.reset().unwrap();
}

pub fn verify_chain(&self, fast_validation: bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not use the name assume_valid_rangeproofs_kernels here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done but i let --fast as the option for the arg verify-chain.

let mut e = term::stdout().unwrap();
let params = json!([fast_validation]);
writeln!(
e,
"Checking the state of the chain. This might take time..."
)
.unwrap();
match self.send_json_request::<()>("validate_chain", &params) {
Ok(_) => {
if fast_validation {
writeln!(e, "Successfully validated the sum of kernel excesses! [fast_verification enabled]").unwrap()
} else {
writeln!(e, "Successfully validated the sum of kernel excesses, kernel signature and rangeproofs!").unwrap()
}
}
Err(err) => writeln!(e, "Failed to validate chain: {:?}", err).unwrap(),
}
e.reset().unwrap();
}

pub fn ban_peer(&self, peer_addr: &SocketAddr) {
let mut e = term::stdout().unwrap();
let params = json!([peer_addr]);
Expand Down Expand Up @@ -191,6 +221,10 @@ pub fn client_command(client_args: &ArgMatches<'_>, global_config: GlobalConfig)
let hash = args.value_of("hash").unwrap();
node_client.invalidate_header(hash.to_string());
}
("verify-chain", Some(args)) => {
let fast_validation = args.is_present("fast");
node_client.verify_chain(fast_validation);
}
("ban", Some(peer_args)) => {
let peer = peer_args.value_of("peer").unwrap();

Expand Down
10 changes: 9 additions & 1 deletion src/bin/grin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,17 @@ subcommands:
- hash:
help: The header hash to reset to
required: true
- verify-chain:
about: Trigger a verication of the rangeproofs, kernel signatures and excesses.
args:
- fast:
help: if present, will skip verification of rangeproofs, kernel signatures and will only validate the sum of kernel excesses.
short: f
long: fast
takes_value: false
- invalidateheader:
about: Adds header hash to denylist
args:
- hash:
help: The header hash to invalidate
required: true
required: true