bitcoindevkit / bdk-cli

A CLI wallet library and REPL tool to demo and test the BDK library
Other
111 stars 65 forks source link

Remove unwraps in bdk_cli and more detailed message for ChecksumMismatch #23

Closed RCasatta closed 3 years ago

RCasatta commented 3 years ago

Description

This removes some unwraps in favor of ? during handling of the commands.

During development I frequently hit ChecksumMismatch while changing the descriptor and keeping the same wallet name, I imagined users may be confused so I detailed the message.

Notes to the reviewers

On top of #22

Error handling is a little nicer for end-user but it comes at the expense of losing stacktrace information in case of errors, so it may be controversial. However, we are now writing blog post and expanding user base so I think it is better.

Checklists

All Submissions:

New Features:

thunderbiscuit commented 3 years ago

Great idea! The ChecksumMismatch error message has been bugging me for some time now, and I think a more user-friendly explanation as to what is going on is a really good thing.

notmandatory commented 3 years ago

At some point I'd like to try adding anyhow to this project to add context to and help display errors, but I think we'd also need to add thiserror macro for bdk errors. For now I think the solution in the PR is a good step in the right direction.

afilini commented 3 years ago

This one probably needs rebasing after the force-push on #22

RCasatta commented 3 years ago

rebased