Skip to content

Commit

Permalink
Merge #61: Improve error handlings.
Browse files Browse the repository at this point in the history 
7e65add Handle errors properly instead of unwraps. (rajarshimaitra)

Pull request description:

  <!-- You can erase any parts of this template not applicable to your Pull Request. -->

  ### Description

  <!-- Describe the purpose of this PR, what's being adding and/or fixed -->

  bdk-cli had lots of ugly unwraps through out the code. All the unwraps inside the library (except tests) were removed and errors were propagated with `bdk::Error` type.

  <!-- In this section you can include notes directed to the reviewers, like explaining why some parts
  of the PR were done in a specific way -->

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk-cli/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

Top commit has no ACKs.

Tree-SHA512: 5c4a46af66341e300ac25339a402ea6ba9c8b81172873dc1b7ab5fa081fcbd6546fe2e372acce2a2b6f227034159724e8fc083e02c598de6a44cc4f843e1f07f
  • Loading branch information
@notmandatory
notmandatory committed Dec 12, 2021
2 parents ceafbed + 7e65add commit f3d126e
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 65 deletions.
56 changes: 25 additions & 31 deletions src/bdk_cli.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,31 +100,33 @@ pub enum ReplSubCommand {
Exit,
}

fn prepare_home_dir() -> PathBuf {
fn prepare_home_dir() -> Result<PathBuf, Error> {
let mut dir = PathBuf::new();
dir.push(&dirs_next::home_dir().unwrap());
dir.push(
&dirs_next::home_dir().ok_or_else(|| Error::Generic("home dir not found".to_string()))?,
);
dir.push(".bdk-bitcoin");

if !dir.exists() {
info!("Creating home directory {}", dir.as_path().display());
fs::create_dir(&dir).unwrap();
fs::create_dir(&dir).map_err(|e| Error::Generic(e.to_string()))?;
}

#[cfg(not(feature = "compact_filters"))]
dir.push("database.sled");

#[cfg(feature = "compact_filters")]
dir.push("compact_filters");
dir
Ok(dir)
}

fn open_database(wallet_opts: &WalletOpts) -> Tree {
let mut database_path = prepare_home_dir();
fn open_database(wallet_opts: &WalletOpts) -> Result<Tree, Error> {
let mut database_path = prepare_home_dir()?;
database_path.push(wallet_opts.wallet.clone());
let database = sled::open(database_path).unwrap();
let tree = database.open_tree(&wallet_opts.wallet).unwrap();
let database = sled::open(database_path)?;
let tree = database.open_tree(&wallet_opts.wallet)?;
debug!("database opened successfully");
tree
Ok(tree)
}

#[cfg(any(
Expand Down Expand Up @@ -183,7 +185,10 @@ where
AnyBlockchainConfig::CompactFilters(CompactFiltersBlockchainConfig {
peers,
network,
storage_dir: prepare_home_dir().into_os_string().into_string().unwrap(),
storage_dir: prepare_home_dir()?
.into_os_string()
.into_string()
.map_err(|_| Error::Generic("Internal OS_String conversion error".to_string()))?,
skip_blocks: Some(wallet_opts.compactfilter_opts.skip_blocks),
})
};
Expand Down Expand Up @@ -277,7 +282,7 @@ fn handle_command(cli_opts: CliOpts, network: Network) -> Result<String, Error>
wallet_opts,
subcommand: WalletSubCommand::OnlineWalletSubCommand(online_subcommand),
} => {
let database = open_database(&wallet_opts);
let database = open_database(&wallet_opts)?;
let wallet = new_online_wallet(network, &wallet_opts, database)?;
let result = bdk_cli::handle_online_wallet_subcommand(&wallet, online_subcommand)?;
serde_json::to_string_pretty(&result)?
Expand All @@ -286,7 +291,7 @@ fn handle_command(cli_opts: CliOpts, network: Network) -> Result<String, Error>
wallet_opts,
subcommand: WalletSubCommand::OfflineWalletSubCommand(offline_subcommand),
} => {
let database = open_database(&wallet_opts);
let database = open_database(&wallet_opts)?;
let wallet = new_offline_wallet(network, &wallet_opts, database)?;
let result = bdk_cli::handle_offline_wallet_subcommand(
&wallet,
Expand All @@ -311,7 +316,7 @@ fn handle_command(cli_opts: CliOpts, network: Network) -> Result<String, Error>
}
#[cfg(feature = "repl")]
CliSubCommand::Repl { wallet_opts } => {
let database = open_database(&wallet_opts);
let database = open_database(&wallet_opts)?;

#[cfg(any(
feature = "electrum",
Expand Down Expand Up @@ -349,24 +354,17 @@ fn handle_command(cli_opts: CliOpts, network: Network) -> Result<String, Error>
let split_line: Vec<&str> = split_regex
.captures_iter(&line)
.map(|c| {
c.get(1)
Ok(c.get(1)
.or_else(|| c.get(2))
.or_else(|| c.get(3))
.unwrap()
.as_str()
.ok_or_else(|| Error::Generic("Invalid commands".to_string()))?
.as_str())
})
.collect();
let repl_subcommand: Result<ReplSubCommand, clap::Error> =
ReplSubCommand::from_iter_safe(split_line);
.collect::<Result<Vec<_>, Error>>()?;
let repl_subcommand = ReplSubCommand::from_iter_safe(split_line)
.map_err(|e| Error::Generic(e.to_string()))?;
debug!("repl_subcommand = {:?}", repl_subcommand);

if let Err(err) = repl_subcommand {
println!("{}", err.message);
continue;
}

let repl_subcommand = repl_subcommand.unwrap();

let result = match repl_subcommand {
#[cfg(any(
feature = "electrum",
Expand All @@ -390,10 +388,7 @@ fn handle_command(cli_opts: CliOpts, network: Network) -> Result<String, Error>
ReplSubCommand::Exit => break,
};

println!(
"{}",
serde_json::to_string_pretty(&result.unwrap()).unwrap()
);
println!("{}", serde_json::to_string_pretty(&result?)?);
}
Err(ReadlineError::Interrupted) => continue,
Err(ReadlineError::Eof) => break,
Expand All @@ -404,7 +399,6 @@ fn handle_command(cli_opts: CliOpts, network: Network) -> Result<String, Error>
}
}

// rl.save_history("history.txt").unwrap();
"Exiting REPL".to_string()
}
#[cfg(all(feature = "reserves", feature = "electrum"))]
Expand Down
66 changes: 32 additions & 34 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -877,17 +877,10 @@ fn parse_recipient(s: &str) -> Result<(Script, u64), String> {
if parts.len() != 2 {
return Err("Invalid format".to_string());
}
let addr = Address::from_str(parts[0]).map_err(|e| e.to_string())?;
let val = u64::from_str(parts[1]).map_err(|e| e.to_string())?;

let addr = Address::from_str(parts[0]);
if let Err(e) = addr {
return Err(format!("{:?}", e));
}
let val = u64::from_str(parts[1]);
if let Err(e) = val {
return Err(format!("{:?}", e));
}

Ok((addr.unwrap().script_pubkey(), val.unwrap()))
Ok((addr.script_pubkey(), val))
}
#[cfg(any(
feature = "electrum",
Expand All @@ -908,7 +901,7 @@ fn parse_proxy_auth(s: &str) -> Result<(String, String), String> {
}

fn parse_outpoint(s: &str) -> Result<OutPoint, String> {
OutPoint::from_str(s).map_err(|e| format!("{:?}", e))
OutPoint::from_str(s).map_err(|e| e.to_string())
}

/// Execute an offline wallet sub-command
Expand Down Expand Up @@ -1034,8 +1027,7 @@ where
assume_height,
trust_witness_utxo,
} => {
let psbt = base64::decode(&psbt)
.map_err(|e| Error::Generic(format!("Base64 decode error: {:?}", e)))?;
let psbt = base64::decode(&psbt).map_err(|e| Error::Generic(e.to_string()))?;
let mut psbt: PartiallySignedTransaction = deserialize(&psbt)?;
let signopt = SignOptions {
assume_height,
Expand All @@ -1052,17 +1044,17 @@ where
}
}
ExtractPsbt { psbt } => {
let psbt = base64::decode(&psbt).unwrap();
let psbt: PartiallySignedTransaction = deserialize(&psbt).unwrap();
let psbt = base64::decode(&psbt).map_err(|e| Error::Generic(e.to_string()))?;
let psbt: PartiallySignedTransaction = deserialize(&psbt)?;
Ok(json!({"raw_tx": serialize_hex(&psbt.extract_tx()),}))
}
FinalizePsbt {
psbt,
assume_height,
trust_witness_utxo,
} => {
let psbt = base64::decode(&psbt).unwrap();
let mut psbt: PartiallySignedTransaction = deserialize(&psbt).unwrap();
let psbt = base64::decode(&psbt).map_err(|e| Error::Generic(e.to_string()))?;
let mut psbt: PartiallySignedTransaction = deserialize(&psbt)?;

let signopt = SignOptions {
assume_height,
Expand All @@ -1082,13 +1074,15 @@ where
let mut psbts = psbt
.iter()
.map(|s| {
let psbt = base64::decode(&s).unwrap();
let psbt: PartiallySignedTransaction = deserialize(&psbt).unwrap();
psbt
let psbt = base64::decode(&s).map_err(|e| Error::Generic(e.to_string()))?;
let psbt: PartiallySignedTransaction = deserialize(&psbt)?;
Ok(psbt)
})
.collect::<Vec<_>>();
.collect::<Result<Vec<_>, Error>>()?;

let init_psbt = psbts.pop().unwrap();
let init_psbt = psbts
.pop()
.ok_or_else(|| Error::Generic("Invalid PSBT input".to_string()))?;
let final_psbt = psbts
.into_iter()
.try_fold::<_, _, Result<PartiallySignedTransaction, Error>>(
Expand Down Expand Up @@ -1130,11 +1124,11 @@ where
Broadcast { psbt, tx } => {
let tx = match (psbt, tx) {
(Some(psbt), None) => {
let psbt = base64::decode(&psbt).unwrap();
let psbt: PartiallySignedTransaction = deserialize(&psbt).unwrap();
let psbt = base64::decode(&psbt).map_err(|e| Error::Generic(e.to_string()))?;
let psbt: PartiallySignedTransaction = deserialize(&psbt)?;
psbt.extract_tx()
}
(None, Some(tx)) => deserialize(&Vec::<u8>::from_hex(&tx).unwrap()).unwrap(),
(None, Some(tx)) => deserialize(&Vec::<u8>::from_hex(&tx)?)?,
(Some(_), Some(_)) => panic!("Both `psbt` and `tx` options not allowed"),
(None, None) => panic!("Missing `psbt` and `tx` option"),
};
Expand Down Expand Up @@ -1261,11 +1255,13 @@ pub fn handle_key_subcommand(
_ => WordCount::Words24,
};
let mnemonic: GeneratedKey<_, miniscript::BareCtx> =
Mnemonic::generate((mnemonic_type, Language::English)).unwrap();
//.map_err(|e| KeyError::from(e.unwrap()))?;
Mnemonic::generate((mnemonic_type, Language::English))
.map_err(|_| Error::Generic("Mnemonic generation error".to_string()))?;
let mnemonic = mnemonic.into_key();
let xkey: ExtendedKey = (mnemonic.clone(), password).into_extended_key()?;
let xprv = xkey.into_xprv(network).unwrap();
let xprv = xkey.into_xprv(network).ok_or_else(|| {
Error::Generic("Privatekey info not found (should not happen)".to_string())
})?;
let fingerprint = xprv.fingerprint(&secp);
let phrase = mnemonic
.word_iter()
Expand All @@ -1277,12 +1273,12 @@ pub fn handle_key_subcommand(
)
}
KeySubCommand::Restore { mnemonic, password } => {
let mnemonic = Mnemonic::parse(mnemonic).unwrap();
// .map_err(|e| {
// KeyError::from(e.downcast::<bdk::keys::bip39::ErrorKind>().unwrap())
// })?;
let mnemonic = Mnemonic::parse_in(Language::English, mnemonic)
.map_err(|e| Error::Generic(e.to_string()))?;
let xkey: ExtendedKey = (mnemonic, password).into_extended_key()?;
let xprv = xkey.into_xprv(network).unwrap();
let xprv = xkey.into_xprv(network).ok_or_else(|| {
Error::Generic("Privatekey info not found (should not happen)".to_string())
})?;
let fingerprint = xprv.fingerprint(&secp);

Ok(json!({ "xprv": xprv.to_string(), "fingerprint": fingerprint.to_string() }))
Expand All @@ -1299,7 +1295,9 @@ pub fn handle_key_subcommand(
derived_xprv.into_descriptor_key(Some(origin), DerivationPath::default())?;

if let Secret(desc_seckey, _, _) = derived_xprv_desc_key {
let desc_pubkey = desc_seckey.as_public(&secp).unwrap();
let desc_pubkey = desc_seckey
.as_public(&secp)
.map_err(|e| Error::Generic(e.to_string()))?;
Ok(json!({"xpub": desc_pubkey.to_string(), "xprv": desc_seckey.to_string()}))
} else {
Err(Error::Key(Message("Invalid key variant".to_string())))
Expand Down

0 comments on commit f3d126e

Please sign in to comment.