Skip to content

refactor(floresta-chain): apply strict error handling in flat_chain_store#1075

Open
Vedd-Patel wants to merge 1 commit into
getfloresta:masterfrom
Vedd-Patel:feat-error-handling-flat-chain-store
Open

refactor(floresta-chain): apply strict error handling in flat_chain_store#1075
Vedd-Patel wants to merge 1 commit into
getfloresta:masterfrom
Vedd-Patel:feat-error-handling-flat-chain-store

Conversation

@Vedd-Patel
Copy link
Copy Markdown

@Vedd-Patel Vedd-Patel commented May 19, 2026

Description and Notes

related issue: #1053

This PR improves systematic error handling in flat_chain_store.rs, focusing on clearer error semantics and correct propagation paths

what changed

  • reworked chainstore error modeling to use ChainstoreError with clearer variants and messages
  • preserved error sources for internal failures via Internal(Box<dyn Error + Send + Sync>)
  • improved index/probing error behavior so not-found/full-index paths are handled correctly
  • added explicit propagation for InvalidValidationIndex in save_roots_for_block
  • kept module-level strict clippy guards for error-handling pattern

Contributor Checklist

  • I've followed the contribution guidelines
  • I've verified one of the following:
    • Ran just pcc (recommended but slower)
    • Ran just lint-features '-- -D warnings' && cargo test --release
    • Confirmed CI passed on my fork
  • I've linked any related issue(s) in the sections above

@moisesPompilio moisesPompilio added the reliability Related to runtime reliability, stability and production readiness label May 19, 2026
@github-project-automation github-project-automation Bot moved this to Backlog in Floresta May 19, 2026
@moisesPompilio moisesPompilio added this to the SoB/2026 milestone May 19, 2026
@moisesPompilio moisesPompilio requested a review from jaoleal May 19, 2026 12:19
@moisesPompilio moisesPompilio moved this from Backlog to Needs review in Floresta May 19, 2026
@Vedd-Patel
Copy link
Copy Markdown
Author

@moisesPompilio you can also take a look!
I would love your feedbacks if any

@jaoleal jaoleal moved this from Needs review to In progress in Floresta May 19, 2026
Copy link
Copy Markdown
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.

A few architectural questions.

It also looks like there was a regression with how we handle dirs.

Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Copy link
Copy Markdown
Member

@jaoleal jaoleal left a comment

Choose a reason for hiding this comment

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

It would be interesting if you could split the changes, itll be easier to review

Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
@Vedd-Patel
Copy link
Copy Markdown
Author

It would be interesting if you could split the changes, itll be easier to review

from next time i'll keep this in mind

@jaoleal
Copy link
Copy Markdown
Member

jaoleal commented May 19, 2026

Guys, the only recoverable error inside the flatchainstore is HeadersNotFound... Should we review the plan ?

@Davidson-Souza

@Davidson-Souza
Copy link
Copy Markdown
Member

Guys, the only recoverable error inside the flatchainstore is HeadersNotFound

That's not true. OversizedAccumulator, CorruptedDatabase and InvalidValidationIndex are fixed by a reindex.

@Vedd-Patel Vedd-Patel force-pushed the feat-error-handling-flat-chain-store branch from 8c4069e to 4668b46 Compare May 29, 2026 05:29
Copy link
Copy Markdown
Contributor

@lorenzolfm lorenzolfm left a comment

Choose a reason for hiding this comment

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

Hi @Vedd-Patel. Thanks for your work on this. Your PR is going on the right direction :)

I've made some comments about simplifying the Internal variant, the value of some tests you added and notes about some changes that you may want to avoid to keep this PR on track with your goal and keep it easy for contributors to review.

Comment thread crates/floresta-chain/src/pruned_utreexo/chainstore.rs
Comment thread crates/floresta-chain/src/pruned_utreexo/chainstore.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/chainstore.rs
Comment thread crates/floresta-chain/src/pruned_utreexo/chainstore.rs
Comment thread crates/floresta-chain/src/pruned_utreexo/chainstore.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs Outdated
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs
Comment thread crates/floresta-chain/src/pruned_utreexo/flat_chain_store.rs
@Vedd-Patel Vedd-Patel force-pushed the feat-error-handling-flat-chain-store branch from 4668b46 to c22550a Compare June 5, 2026 12:33
@Vedd-Patel Vedd-Patel requested a review from lorenzolfm June 5, 2026 12:35
@lorenzolfm
Copy link
Copy Markdown
Contributor

Addressed some comments. By the way there are still some renames on variable names that I think you should avoid doing. The smaller the diff, the higher the quality of reviews you'll get :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reliability Related to runtime reliability, stability and production readiness

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

5 participants