Skip to content

Fix broken chains after restore#904

Merged
tompro merged 4 commits into
masterfrom
fix-broken-chains-after-restore
Apr 23, 2026
Merged

Fix broken chains after restore#904
tompro merged 4 commits into
masterfrom
fix-broken-chains-after-restore

Conversation

@tompro
Copy link
Copy Markdown
Collaborator

@tompro tompro commented Apr 22, 2026

📝 Description

Fixes #902. Issue was that on restore we only restored the plain block but not the event chain cache. This leads to disconnected block published after restore. Now we write the cache when processing restore blocks. I discovered another issue (blockchains disconnect after re-sync for the same reason, cache can have invalid tips) which is also fixed with this PR.

Relates to #902


✅ Checklist

Please ensure the following tasks are completed before requesting a review:

  • My code adheres to the coding guidelines of this project.
  • I have run cargo fmt.
  • I have run cargo clippy.
  • I have added or updated tests (if applicable).
  • All CI/CD steps were successful.
  • I have updated the documentation (if applicable).
  • I have checked that there are no console errors or warnings.
  • I have verified that the application builds without errors.
  • I've described the changes made to the API. (modification, addition, deletion).

🚀 Changes Made

  • Bug Fixes:

    • Recover event chain together with blockchain
    • Also recover other event chains when a chain re-sync happens
  • Other Changes:

    • Only store the selected valid chain in event chain (others are deleted on invalid)

💡 How to Test

Please provide clear instructions on how reviewers can test your changes:

  1. Create account, backup seed
  2. Restore account from seed, create company
  3. Restore account again and observe that all identity chain block (incl. company) are used on recover

📋 Review Guidelines

Please focus on the following while reviewing:

  • Does the code follow the repository's contribution guidelines?
  • Are there any potential bugs or performance issues?
  • Are there any typos or grammatical errors in the code or comments?

Copilot AI review requested due to automatic review settings April 22, 2026 13:32
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

❌ Patch coverage is 90.00000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.59%. Comparing base (a31da52) to head (8a3be8b).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
.../bcr-ebill-persistence/src/db/nostr_chain_event.rs 50.00% 2 Missing ⚠️
...ansport/src/handler/company_chain_event_handler.rs 50.00% 1 Missing ⚠️
...nsport/src/handler/identity_chain_event_handler.rs 75.00% 1 Missing ⚠️
crates/bcr-ebill-transport/src/handler/restore.rs 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #904      +/-   ##
==========================================
+ Coverage   70.51%   70.59%   +0.08%     
==========================================
  Files         139      139              
  Lines       27685    27704      +19     
==========================================
+ Hits        19522    19558      +36     
+ Misses       8163     8146      -17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes broken blockchain continuity after account restore/resync by ensuring the Nostr chain-event cache is rebuilt/invalidated alongside restored/resynced blocks, and by preventing publishing blocks when the previous-chain event is missing.

Changes:

  • Rebuild and invalidate Nostr chain-event cache during restore and chain resync (identity/company/bill).
  • Remove the valid flag from NostrChainEvent and add remove_chain_events to purge stale cache entries.
  • Add a publish-time guard to reject publishing non-genesis blocks when the previous event is missing.

Reviewed changes

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

Show a summary per file
File Description
crates/bcr-ebill-transport/src/test_utils.rs Extends mocks for the new remove_chain_events API.
crates/bcr-ebill-transport/src/nostr_transport.rs Updates persisted chain-event payload to match the new NostrChainEvent shape.
crates/bcr-ebill-transport/src/lib.rs Wires nostr_chain_event_store into processors/restore service constructors.
crates/bcr-ebill-transport/src/handler/restore.rs Stores recovered identity-chain events into the chain-event store during restore.
crates/bcr-ebill-transport/src/handler/public_chain_helpers.rs Removes valid from as_chain_store_event and its call sites.
crates/bcr-ebill-transport/src/handler/mod.rs Updates mock trait surface to include remove_chain_events.
crates/bcr-ebill-transport/src/handler/identity_chain_event_processor.rs Purges and repopulates identity chain-event cache during resync.
crates/bcr-ebill-transport/src/handler/identity_chain_event_handler.rs Adapts event persistence to new NostrChainEvent shape.
crates/bcr-ebill-transport/src/handler/company_invite_handler.rs Stops persisting “invalid/malicious” fork events; stores only inserted chain events.
crates/bcr-ebill-transport/src/handler/company_chain_event_processor.rs Purges and repopulates company chain-event cache during resync.
crates/bcr-ebill-transport/src/handler/company_chain_event_handler.rs Adapts event persistence to new NostrChainEvent shape.
crates/bcr-ebill-transport/src/handler/bill_invite_handler.rs Stops persisting “invalid/malicious” fork events; stores only inserted chain events.
crates/bcr-ebill-transport/src/handler/bill_chain_event_processor.rs Purges and repopulates bill chain-event cache during resync.
crates/bcr-ebill-transport/src/handler/bill_chain_event_handler.rs Adapts event persistence to new NostrChainEvent shape.
crates/bcr-ebill-transport/src/block_transport.rs Adds publish-time validation to avoid orphaned blocks (plus tests).
crates/bcr-ebill-persistence/src/nostr.rs Adds remove_chain_events and removes valid from NostrChainEvent.
crates/bcr-ebill-persistence/src/db/nostr_chain_event.rs Drops valid column usage; updates queries and adds delete-by-chain helper.
crates/bcr-ebill-core/src/protocol/blockchain/bill/chain.rs Minor closure variable rename in sort comparator.
crates/bcr-ebill-api/src/tests/mod.rs Updates mock trait surface to include remove_chain_events.
Comments suppressed due to low confidence (3)

crates/bcr-ebill-transport/src/handler/company_chain_event_handler.rs:113

  • The error log message in store_event says "Failed to store bill chain..." even though this is the company chain handler. This makes logs misleading when diagnosing chain resync/restore behavior; update the message to refer to the company chain/event handler.
        {
            error!("Failed to store bill chain nostr event into event store {e}");
        }

crates/bcr-ebill-persistence/src/db/nostr_chain_event.rs:55

  • find_chain_events / find_latest_block_events / find_root_event now query all chain events without filtering on the legacy valid flag. Existing databases may already contain rows with valid = false from the previous implementation; after this change those previously-invalid events will be returned and can become "latest" tips, reintroducing disconnected-chain behavior. Consider a migration/cleanup step (e.g., delete valid = false rows) or keep filtering to valid = true for backwards compatibility until old data is purged.
    async fn find_all_chain_events(
        &self,
        chain_id: String,
        chain_type: BlockchainType,
    ) -> Result<Vec<NostrChainEventDb>> {
        let mut bindings = Bindings::default();
        bindings.add(DB_TABLE, Self::TABLE)?;
        bindings.add(CHAIN_ID, chain_id.to_owned())?;
        bindings.add(CHAIN_TYPE, chain_type)?;

        let result: Vec<NostrChainEventDb> = self.db
                .query(format!(
                    "SELECT * FROM type::table(${DB_TABLE}) WHERE {CHAIN_ID} = ${CHAIN_ID} AND {CHAIN_TYPE} = ${CHAIN_TYPE} ORDER BY {BLOCK_HEIGHT} DESC"
                ).as_str(), bindings)
                .await?;
        Ok(result)
    }

crates/bcr-ebill-transport/src/handler/identity_chain_event_handler.rs:112

  • The error log message in store_event says "Failed to store bill chain..." even though this is the identity chain handler. This makes production logs misleading when diagnosing restore/resync issues; update the message to refer to the identity chain/event handler.
        {
            error!("Failed to store bill chain nostr event into event store {e}");
        }

Comment thread crates/bcr-ebill-transport/src/handler/bill_chain_event_handler.rs
Comment thread crates/bcr-ebill-transport/src/handler/restore.rs
Comment thread crates/bcr-ebill-transport/src/block_transport.rs
@tompro tompro self-assigned this Apr 22, 2026
Copy link
Copy Markdown
Collaborator

@zupzup zupzup left a comment

Choose a reason for hiding this comment

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

Very Nice Find! 💪

@tompro tompro merged commit 79db6a5 into master Apr 23, 2026
10 checks passed
@tompro tompro deleted the fix-broken-chains-after-restore branch April 23, 2026 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

issue with restoring identity

4 participants