+
Skip to content

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

Conversation

JoseSK999
Copy link
Contributor

What is the purpose of this pull request?

  • Bug fix
  • Documentation update
  • New feature
  • Test
  • Other: Error handling

Which crates are being modified?

  • floresta-chain
  • floresta-cli
  • floresta-common
  • floresta-compact-filters
  • floresta-electrum
  • floresta-watch-only
  • floresta-wire
  • floresta
  • florestad
  • Other:

Description

Relates to #463. This is a first step on hardening error handling.

Previously we were using unwrap for things like chainstore operations and Option values unwrapping in the ChainStateBuilder methods.

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).

@JoseSK999 JoseSK999 force-pushed the handle-chain-state-builder-errors branch from 69fbcca to 87b634a Compare May 5, 2025 14:50
@Davidson-Souza Davidson-Souza added enhancement New feature or request code quality Generally improves code readability and maintainability labels May 5, 2025
Copy link
Member

@Davidson-Souza Davidson-Souza left a 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

@JoseSK999 JoseSK999 force-pushed the handle-chain-state-builder-errors branch 2 times, most recently from 4ff7f6b to f8a10f0 Compare May 5, 2025 21:48
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).
@JoseSK999 JoseSK999 force-pushed the handle-chain-state-builder-errors branch from f8a10f0 to f648c65 Compare May 5, 2025 21:56
@JoseSK999
Copy link
Contributor Author

Addressed the helpful comments from @Davidson-Souza

Copy link
Member

@Davidson-Souza Davidson-Souza left a comment

Choose a reason for hiding this comment

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

ACK f648c65

@Davidson-Souza Davidson-Souza merged commit efbad0c into vinteumorg:master May 6, 2025
10 checks passed
luisschwab pushed a commit to luisschwab/Floresta that referenced this pull request May 7, 2025
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).
luisschwab pushed a commit to luisschwab/Floresta that referenced this pull request May 8, 2025
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).
@JoseSK999 JoseSK999 deleted the handle-chain-state-builder-errors branch May 14, 2025 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Generally improves code readability and maintainability enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载