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.

Sign up for GitHub

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

Modify mine_block to use wallet V2 API #2892

Merged
merged 2 commits into from
Jun 13, 2019
Merged

Modify mine_block to use wallet V2 API #2892

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8108596
update mine_block to use V2 wallet API
yeastplume Jun 12, 2019
8030299
rustfmt
yeastplume Jun 12, 2019
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
52 changes: 43 additions & 9 deletions servers/src/mining/mine_block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use crate::util::RwLock;
use chrono::prelude::{DateTime, NaiveDateTime, Utc};
use rand::{thread_rng, Rng};
use serde_json::{json, Value};
use std::sync::Arc;
use std::thread;
use std::time::Duration;
Expand Down Expand Up @@ -265,15 +266,48 @@ fn get_coinbase(
/// Call the wallet API to create a coinbase output for the given block_fees.
/// Will retry based on default "retry forever with backoff" behavior.
fn create_coinbase(dest: &str, block_fees: &BlockFees) -> Result<CbData, Error> {
let url = format!("{}/v1/wallet/foreign/build_coinbase", dest);
match api::client::post(&url, None, &block_fees) {
Err(e) => {
error!(
"Failed to get coinbase from {}. Is the wallet listening?",
url
);
Err(Error::WalletComm(format!("{}", e)))
let url = format!("{}/v2/foreign", dest);
let req_body = json!({
"jsonrpc": "2.0",
"method": "build_coinbase",
"id": 1,
"params": {
"block_fees": block_fees
}
Ok(res) => Ok(res),
});

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 report = format!(
"Failed to get coinbase from {}. Is the wallet listening? {}",
dest, e
);
error!("{}", report);
Error::WalletComm(report)
})?;

let res: Value = serde_json::from_str(&res).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't be cleaner to create a type for the response (I assume it's Result-like) instead of dealing with raw json Value?

Copy link
Member Author

Choose a reason for hiding this comment

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

It just seemed simpler for this one-off in the code to do this way, also had the notion of making it obvious what the json structure is underneath for anyone looking at the code. If it were a full wallet API client, I'd say definitely, yes.

trace!("Response: {}", res);
if res["error"] != json!(null) {
let report = format!(
"Failed to get coinbase from {}: Error: {}, Message: {}",
dest, res["error"]["code"], res["error"]["message"]
);
error!("{}", report);
return Err(Error::WalletComm(report));
}

let cb_data = res["result"]["Ok"].clone();
Copy link
Contributor

Choose a reason for hiding this comment

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

for example it's hard to tell should Ok be in lower case or not (or does it matter)

trace!("cb_data: {}", cb_data);
let ret_val = match serde_json::from_value::<CbData>(cb_data) {
Ok(r) => r,
Err(e) => {
let report = format!("Couldn't deserialize CbData: {}", e);
error!("{}", report);
return Err(Error::WalletComm(report));
}
};

Ok(ret_val)
}