-
Notifications
You must be signed in to change notification settings - Fork 62
Handle ChainStateBuilder errors, avoid unwraps #471
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
Handle ChainStateBuilder errors, avoid unwraps #471
Conversation
69fbcca
to
87b634a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strong Concept ACK
I'm leaving some comments about things I find weird about the current code, I think they may be on scope for this PR. But let me know what you think
crates/floresta-chain/src/pruned_utreexo/chain_state_builder.rs
Outdated
Show resolved
Hide resolved
crates/floresta-chain/src/pruned_utreexo/chain_state_builder.rs
Outdated
Show resolved
Hide resolved
crates/floresta-chain/src/pruned_utreexo/chain_state_builder.rs
Outdated
Show resolved
Hide resolved
crates/floresta-chain/src/pruned_utreexo/chain_state_builder.rs
Outdated
Show resolved
Hide resolved
4ff7f6b
to
f8a10f0
Compare
Previously we were using `unwrap` for things like chainstore operations and `Option` values unwrapping. I have replaced all unwraps in the file with proper error handling. Added the new `IncompleteTip` and `Database` error variants. Polished a bit the comments. Also instead of implementing `From` ChainStateBuilder for Chainstate I implement `TryFrom` (it's fallible, failing if the chainstore and chain params are not set).
f8a10f0
to
f648c65
Compare
Addressed the helpful comments from @Davidson-Souza |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK f648c65
Previously we were using `unwrap` for things like chainstore operations and `Option` values unwrapping. I have replaced all unwraps in the file with proper error handling. Added the new `IncompleteTip` and `Database` error variants. Polished a bit the comments. Also instead of implementing `From` ChainStateBuilder for Chainstate I implement `TryFrom` (it's fallible, failing if the chainstore and chain params are not set).
Previously we were using `unwrap` for things like chainstore operations and `Option` values unwrapping. I have replaced all unwraps in the file with proper error handling. Added the new `IncompleteTip` and `Database` error variants. Polished a bit the comments. Also instead of implementing `From` ChainStateBuilder for Chainstate I implement `TryFrom` (it's fallible, failing if the chainstore and chain params are not set).
What is the purpose of this pull request?
Which crates are being modified?
Description
Relates to #463. This is a first step on hardening error handling.
Previously we were using
unwrap
for things like chainstore operations andOption
values unwrapping in theChainStateBuilder
methods.I have replaced all unwraps in the file with proper error handling. Added the new
IncompleteTip
andDatabase
error variants. Polished a bit the comments.Also instead of implementing
From
ChainStateBuilder for Chainstate I implementTryFrom
(it's fallible, failing if the chainstore and chain params are not set).