Skip to content

fix: preserve seeded accounts on restart - #196

Merged
snawaz merged 3 commits into
masterfrom
fix/preserve-seeded-accounts-on-restart
Nov 4, 2026
Merged

snawaz merged 3 commits into
masterfrom
fix/preserve-seeded-accounts-on-restart

Conversation

@snawaz

@snawaz snawaz commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

What changed

Keeper now seeds caller supplied startup accounts only when they are absent from AccountsDB. An existing account keeps its saved data when the node restarts; a missing account is still initialized. This preserves the validator's MagicContext intent ID across a leader restart, matching the V1 validator's behavior.

Closes #195

Impact

The KeeperBuilder.possibly_missing_accounts field makes the seed-if-absent behavior explicit. Builtin program and sysvar seeding is unchanged. The previous overwrite changed leader state without a replicated transaction, so the verifiers rejected the next state seal.

Reviewer notes

The new preserves_seeded_accounts_on_reopen regression test checks both saved-account preservation and creation of a missing account. Before this change, the validator's redhat/replication_settlement_recovery failed after the leader restart with matching transaction counts but different state checksums. With Engine commit 2f8a90d pinned, the scenario passes on validator PR #1867 and PR #1868.

@snawaz
snawaz requested a review from bmuddha November 3, 2026 14:07
@snawaz
snawaz marked this pull request as ready for review November 3, 2026 14:07
@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

Keeper checks whether the authority account and configured accounts exist before queuing them for seeding. The builder field is now named possibly_missing_accounts, and test setup uses that field. A new reopen test verifies that an existing account retains its stored lamports, data, and owner, while a newly configured account is seeded.

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: Minimal · up to 368c3

No actionable merge-blocking risk remains in the reviewed change.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: preserving seeded accounts when the Keeper restarts.
Description check Passed The description directly explains the seed-if-absent behavior, account preservation, regression test, impact, and validation results.
Linked Issues check Passed Issue #195 requires startup accounts to be seeded only when absent and existing account data to remain unchanged after reopening. keeper/src/builder.rs now checks possibly_missing_accounts with on…
Out of Scope Changes check Passed The changes remain within issue #195. The KeeperBuilder.accounts rename to possibly_missing_accounts documents the required seed-if-absent behavior. Updates in keeper/src/testkit.rs and `replica…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Linked Issues check

Explanation

Issue #195 requires startup accounts to be seeded only when absent and existing account data to remain unchanged after reopening. keeper/src/builder.rs now checks possibly_missing_accounts with one AccountsDB loader and stores only absent accounts. The existing authority sponsor check remains absent-only. preserves_seeded_accounts_on_reopen verifies persisted lamports, data, and owner remain unchanged and verifies that a missing account is created. This behavior preserves the account data that contains the validator intent ID.

Full details: Out of Scope Changes check

Explanation

The changes remain within issue #195. The KeeperBuilder.accounts rename to possibly_missing_accounts documents the required seed-if-absent behavior. Updates in keeper/src/testkit.rs and replicator/tests/integration.rs update callers for that API change. The new recovery test directly covers the issue. No unrelated production behavior is changed; builtin program and sysvar seeding is unchanged.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 4 files.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 908e924 and 368c327.

Files selected for processing: 1
  • keeper/src/tests/recovery.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  • Fix comments: Fix CodeRabbit comments on this PR

Comment thread keeper/src/tests/recovery.rs Resolved

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  • Fix comments: Fix CodeRabbit comments on this PR

Comment thread keeper/src/builder.rs Resolved
Strengthen preserves_seeded_accounts_on_reopen with non-empty account data and a distinct owner. Assert lamports, data, and owner after reopening so the regression catches partial overwrites that preserve only lamports.

Validated with cargo fmt --check and cargo test -p magicblock-keeper preserves_seeded_accounts_on_reopen -- --nocapture.
@snawaz
snawaz merged commit 56ab011 into master Nov 4, 2026
5 checks passed
@snawaz
snawaz deleted the fix/preserve-seeded-accounts-on-restart branch November 4, 2026 11:52
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.

Startup seeding resets persisted accounts after leader restart

1 participant