fix: handle errors instead of panicking on corrupted database blobs

[lklimek]

Summary

• Replace unreachable!() in get_scheduled_votes with warning log + default for unexpected executed column values
• Change QualifiedIdentity::from_bytes() to return Result instead of panicking via .expect()
• Propagate deserialization errors as rusqlite::Error in all 6 callers so corrupted database is surfaced to the user rather than silently ignored or crashing the app
• Add CorruptedBlobError newtype in database/mod.rs to eliminate repeated FromSqlConversionFailure boilerplate

Closes #560

Test plan

• cargo clippy --all-features --all-targets -- -D warnings — clean
• cargo test --all-features --workspace — all tests pass

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Explicitly surface and log corrupted/invalid stored data during identity reconstruction instead of failing silently.
  • Avoid panics on unexpected database values; operations now log a warning and continue safely.

• Stability Improvements

  • Improved validation when reading stored data, reducing silent errors and making failures diagnosable.
  • Greater resilience across database workflows when encountering malformed or unexpected blobs.

[coderabbitai]

Warning

Rate limit exceeded

@lklimek has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 5 minutes and 33 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

This pull request addresses database corruption handling issues by introducing a new CorruptedBlobError type and converting panic-prone deserialization paths to graceful error handling. Identity blob parsing and scheduled vote processing now log warnings and skip/default corrupted entries instead of panicking.

Changes

Cohort / File(s)	Summary
Error Type Introduction src/database/mod.rs	Added CorruptedBlobError(pub String) with pub(crate) visibility and From impl to convert into rusqlite::Error, enabling corruption errors to propagate through the standard database error pathway.
Identity Deserialization src/database/identities.rs	Updated five identity loading paths (get_local_qualified_identities, get_local_voting_identities, get_local_user_identities, get_identity_by_id) to use .map_err(super::CorruptedBlobError)? when deserializing identity blobs, converting panics into catchable errors.
Graceful Corruption Handling src/database/wallet.rs, src/database/scheduled_votes.rs	Modified wallet identity reconstruction to log and skip corrupted entries instead of failing; replaced unreachable!() in get_scheduled_votes with warning log and false default for unexpected executed column values.
API Signature Change src/model/qualified_identity/mod.rs	Changed QualifiedIdentity::from_bytes() return type from Self to Result<Self, String>, replacing internal expect() with explicit error handling via bincode deserialization result.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A hop through the data so fine,
Where corruption once made the code pine,
Now errors are caught with a gentle care,
And graceful defaults float through the air,
No panics to break the database's line! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	All coding objectives from issue #560 are met: QualifiedIdentity::from_bytes returns Result [#560], get_scheduled_votes replaces unreachable!() with logging and default [#560], and callers propagate errors via CorruptedBlobError [#560].
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the two panic issues in #560: error handling in from_bytes, CorruptedBlobError conversion, get_scheduled_votes logging, and caller-side error propagation.
Title check	✅ Passed	The title accurately describes the main objective: converting panics on corrupted database blobs into error handling. All major changes align with this theme.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/560

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/database/identities.rs (1)

176-211: ⚠️ Potential issue | 🟠 Major

Same skip-vs-error concern: corrupted blobs should be skipped in batch-loading functions.

get_local_qualified_identities, get_local_qualified_identities_in_wallets, get_local_voting_identities, and get_local_user_identities all use map_err(super::CorruptedBlobError)? which causes a single corrupted blob to fail the entire collection. Per issue #560: corrupted entries should be skipped with a warning so that the remaining identities still load.

♻️ Suggested pattern (apply to all Vec-returning identity loaders)

         for row in identity_iter {
             let (data, alias, wallet_index, status) = row?;
-            let mut identity = QualifiedIdentity::from_bytes(&data)
-                .map_err(super::CorruptedBlobError)?;
+            let mut identity = match QualifiedIdentity::from_bytes(&data) {
+                Ok(id) => id,
+                Err(e) => {
+                    tracing::warn!(error = %e, "Skipping corrupted identity blob");
+                    continue;
+                }
+            };

🤖 Fix all issues with AI agents

In `@src/database/wallet.rs`:
- Around line 817-818: The call to QualifiedIdentity::from_bytes(&identity_data)
currently uses the ? operator which aborts get_wallets on a single corrupted
blob; change this to handle the Result explicitly (e.g., match or if let Err(e)
= QualifiedIdentity::from_bytes(&identity_data) { log a warning including e and
some context about the wallet/key being processed using your crate's logger
(e.g., warn! or tracing::warn!), then continue the enclosing loop } else bind
the Ok(identity) and proceed), so corrupted entries are skipped with a warning
instead of failing the whole operation; keep the CorruptedBlobError type
available if you need to count or metrics-log these skips.

[lklimek]

@claude please merge v1.0-dev into this branch and ensure tests pass.

[claude]

Claude finished @lklimek's task in 40s —— View job

---

Merging v1.0-dev and Running Tests

• Fetch latest v1.0-dev
• Merge v1.0-dev into fix/560
• Resolve any merge conflicts
• Run cargo test suite
• Run cargo clippy
• Push changes if successful

Issue Encountered

I need approval to run git commands (git fetch and git merge). These commands require user permission before I can execute them.

What you can do:

1. Approve the pending git commands when prompted
2. Or manually run these commands locally:

   git fetch origin v1.0-dev
   git merge origin/v1.0-dev
   cargo test --all-features --workspace
   cargo clippy --all-features --all-targets -- -D warnings
   git push origin fix/560

Once you approve the git commands, I'll be able to complete the merge and run the tests.

---

[lklimek]

@claude please merge v1.0-dev into this branch and ensure tests pass.

[claude]

Claude finished @lklimek's task in 34s —— View job

---

Merging v1.0-dev and Running Tests

• Fetch latest v1.0-dev
• Merge v1.0-dev into fix/560
• Resolve any merge conflicts
• Run cargo test suite
• Run cargo clippy
• Push changes if successful

Issue Encountered

I need approval to run git commands (git fetch and git merge). These commands require user permission before I can execute them.

What you can do:

1. Approve the pending git commands when prompted
2. Or manually run these commands locally:

   git fetch origin v1.0-dev
   git merge origin/v1.0-dev
   cargo test --all-features --workspace
   cargo clippy --all-features --all-targets -- -D warnings
   git push origin fix/560

Once you approve the git commands, I'll be able to complete the merge and run the tests.

---

View job

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/database/identities.rs (2)

176-211: ⚠️ Potential issue | 🟠 Major

One corrupted identity aborts loading all identities — consider skip-and-warn.

The ? operator on Line 183 causes the entire function to return an error if any single identity blob is corrupted. While this is a significant improvement over panicking, the linked issue #560 suggests skipping corrupted entries with a warning so the remaining identities still load.

Consider logging a warning and continuing instead:

Proposed skip-and-warn pattern

         for row in identity_iter {
             let (data, alias, wallet_index, status) = row?;
-            let mut identity =
-                QualifiedIdentity::from_bytes(&data).map_err(super::CorruptedBlobError)?;
+            let mut identity = match QualifiedIdentity::from_bytes(&data) {
+                Ok(id) => id,
+                Err(e) => {
+                    tracing::warn!("Skipping corrupted identity blob: {e}");
+                    continue;
+                }
+            };

The same pattern applies to the other four loading functions in this file (get_local_qualified_identities_in_wallets, get_identity_by_id, get_local_voting_identities, get_local_user_identities).

---

239-272: ⚠️ Potential issue | 🟡 Minor

get_local_qualified_identities_in_wallets doesn't set identity.status.

Unlike get_local_qualified_identities (Line 187), this function doesn't query or set identity.status. The SQL on Line 226 selects only data, alias, wallet_index — no status column. The identity will retain whatever default status comes from from_bytes deserialization, which may differ from the database value.

If this is intentional (e.g., the function is #[allow(dead_code)]), a comment would help. Otherwise, consider aligning with the other function.

🤖 Fix all issues with AI agents

In @.claude/hooks/session-start.sh:
- Around line 93-116: The script currently injects both TENDERDASH_DIR and
CARGO_TARGET_DIR into the global Cargo [env] table (via the sed / heredoc
logic), which can unintentionally affect other build scripts; change the
injection so only TENDERDASH_DIR is written into the [env] table (remove
CARGO_TARGET_DIR from both the sed insertion and the heredoc), or alternatively
make setting CARGO_TARGET_DIR conditional (e.g., only when a specific marker or
project context is detected) and prefer scoping such a cache path to the
Tenderdash build invocation rather than the global Cargo config; update the sed
insertion and the heredoc that reference CARGO_TARGET_DIR to reflect this
change.

🧹 Nitpick comments (2)

.github/workflows/claude.yml (1)

15-19: Consider restricting who can trigger Claude runs.

Any user who can comment on issues or PRs can trigger this workflow by mentioning @claude. This could lead to unintended API credit consumption or unauthorized code changes. Consider adding an actor/membership check, e.g.:

if: |
  github.event.sender.type == 'User' &&
  (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude') && 
   github.event.comment.author_association in ['OWNER', 'MEMBER', 'COLLABORATOR']) || ...

Or use github.event.comment.author_association to restrict to trusted contributors.

.claude/hooks/session-start.sh (1)

18-22: Hardcoded x86_64 architecture for protoc download.

The protoc zip filename is hardcoded to linux-x86_64. If the Claude Code remote environment runs on aarch64, this will install the wrong binary (or fail).

Consider detecting the architecture:

Proposed fix

   PROTOC_VERSION="25.2"
-  PROTOC_ZIP="protoc-${PROTOC_VERSION}-linux-x86_64.zip"
+  ARCH=$(uname -m)
+  case "$ARCH" in
+    x86_64)  PROTOC_ARCH="x86_64" ;;
+    aarch64) PROTOC_ARCH="aarch_64" ;;
+    *)       echo "[session-start] Unsupported architecture: $ARCH"; exit 1 ;;
+  esac
+  PROTOC_ZIP="protoc-${PROTOC_VERSION}-linux-${PROTOC_ARCH}.zip"

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@src/database/identities.rs`:
- Around line 303-304: The current use of query_map(...).collect() in
get_local_voting_identities and get_local_user_identities causes the iterator to
abort on the first Err (a corrupted blob) because
QualifiedIdentity::from_bytes(&data).map_err(...) returns Err and collect
short-circuits; change the processing to iterate with .filter_map or an explicit
loop that attempts QualifiedIdentity::from_bytes for each row, logs a warning on
Err (including identifying info), and only pushes successful identities into the
result so corrupted blobs are skipped instead of aborting the entire load.
Ensure you update the code paths that reference QualifiedIdentity::from_bytes
and the query_map result handling so they match this skip-and-warn behavior for
both functions.
- Around line 151-152: The code currently uses
QualifiedIdentity::from_bytes(&data).map_err(...) and the ? operator which
aborts the whole batch on the first corrupted blob; change the batch-loading
loops (the function containing QualifiedIdentity::from_bytes call and also
get_local_qualified_identities_in_wallets) to match on
QualifiedIdentity::from_bytes(&data) and on Err log a warning (include key/id
and error) and continue the loop instead of returning an error, while on Ok push
the identity as before; keep get_identity_by_id as-is (returning an error for a
single-identity lookup) unless you want the single-lookup behavior changed
explicitly.

[Copilot]

Pull request overview

This PR improves robustness around corrupted/invalid data in the SQLite database by removing panic paths and converting identity-blob decoding failures into recoverable errors (or warnings), so the app can handle unexpected DB state more safely.

Changes:

• Updated QualifiedIdentity::from_bytes() to return a Result instead of panicking on decode failure.
• Replaced an unreachable!() in scheduled vote loading with a warning + safe default.
• Introduced CorruptedBlobError to reduce boilerplate when converting decode failures into rusqlite::Error, and updated identity-loading call sites accordingly.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/model/qualified_identity/mod.rs	Makes identity deserialization fallible to avoid panics on corrupted blobs.
src/database/identities.rs	Propagates corrupted identity blob decode failures as rusqlite::Error via CorruptedBlobError.
src/database/wallet.rs	Avoids panicking on corrupted identity blobs when loading wallets by logging and skipping.
src/database/scheduled_votes.rs	Avoids panicking on unexpected executed values; logs warning and defaults safely.
src/database/mod.rs	Adds CorruptedBlobError newtype + From conversion into rusqlite::Error.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.