Implement RocksDB-backed state persistence for accounts and bonds by Copilot · Pull Request #81 · Steake/BitCell

Copilot · 2025-12-06T23:10:44Z

Original prompt

This section details on the original issue you should resolve

<issue_title>Implement State Persistence via RocksDB Backend</issue_title>
<issue_description>Part of Epic: #69 - RC1: Core Transaction & State Infrastructure

Goal: Persist blockchain state and account data across node restarts using RocksDB.

Requirements:

Replace in-memory HashMap backend with RocksDB in bitcell-state module

Store Account and BondState data serialized for persistence

Update StateManager to use RocksDB

Ensure transactions update and reflect real state even after restart

Files:

crates/bitcell-state/src/storage.rs

Acceptance Criteria:

State persists correctly across node shutdown/restarts

No data loss in normal operation

Refer to [todo_now.md](https://github.com/Steake/BitCell/blob/master/todo_now. md) for critical path.</issue_description>

Comments on the Issue (you are @copilot in this section)
@Steake @copilot please work on this issue.
Dependencies completed:

No dependencies

Please create a PR when ready.</comment_new>

Fixes Implement State Persistence via RocksDB Backend #37

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

- Add data_dir config option for persistent storage - Update StateManager to use storage fallback for get operations - Add Blockchain::with_storage() constructor - Update ValidatorNode and MinerNode to use persistent storage when data_dir is set - Add comprehensive persistence tests for accounts and bonds - Storage persists across node restarts ensuring no data loss Co-authored-by: Steake <530040+Steake@users.noreply.github.com>

- Add test_blockchain_with_persistent_storage test - Add tempfile as dev dependency for bitcell-node - Verify accounts persist correctly across blockchain restarts Co-authored-by: Steake <530040+Steake@users.noreply.github.com>

- Only compute hex encoding when trace level is enabled - Addresses code review feedback for performance optimization Co-authored-by: Steake <530040+Steake@users.noreply.github.com>

Copilot

Pull request overview

This PR implements RocksDB-backed persistent storage for blockchain state, addressing the critical issue of data loss across node restarts. The implementation adds storage-backed variants of state access methods (get_account_owned(), get_bond_owned()), a new Blockchain::with_storage() constructor, and CLI configuration via the --data-dir flag. When data_dir is set, nodes use persistent RocksDB storage; when omitted, they fall back to in-memory storage for testing.

Key Changes

StateManager and Blockchain now support persistent storage through RocksDB-backed constructors
Storage fallback pattern implemented with _owned() variants that check cache then storage
CLI configuration added for all node types (validator, miner, full node) with --data-dir flag
Comprehensive persistence tests added verifying state survives storage reopening

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
crates/bitcell-state/src/storage.rs	Added persistence tests for accounts and bonds that verify data survives storage reopening
crates/bitcell-state/src/lib.rs	Added storage-backed getter methods (`get_account_owned()`, `get_bond_owned()`) with cache-first fallback pattern and trace logging for cache misses; added comprehensive persistence tests
crates/bitcell-node/src/validator.rs	Implemented conditional blockchain initialization with persistent storage when `data_dir` is configured, with helpful console messages
crates/bitcell-node/src/miner.rs	Mirrored validator implementation for consistent storage initialization across node types
crates/bitcell-node/src/main.rs	Wired `--data-dir` CLI parameter to NodeConfig for all three node types (validator, miner, full node)
crates/bitcell-node/src/config.rs	Added `data_dir` field to NodeConfig with proper documentation and None default for testing
crates/bitcell-node/src/blockchain.rs	Added `with_storage()` constructor that initializes RocksDB at `{data_dir}/state/` with comprehensive documentation; added persistence integration test
crates/bitcell-node/Cargo.toml	Added tempfile dev dependency for testing

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

crates/bitcell-node/src/validator.rs

Copilot · 2025-12-07T07:59:02Z

crates/bitcell-node/src/miner.rs

+            std::fs::create_dir_all(data_path)
+                .expect("Failed to create data directory");
+
+            println!("📦 Using persistent storage at: {}", data_path.display());
+            Blockchain::with_storage(secret_key.clone(), metrics.clone(), data_path)
+                .expect("Failed to initialize blockchain with storage")


Error handling issue: Using .expect() for directory creation and blockchain initialization will cause the process to panic instead of returning a proper error. Consider using proper error handling that returns a Result, especially since with_key() could be called from contexts that expect graceful error handling.

@copilot handle this please

Fixed in commit c180ba8. Changed both with_key() and new() methods to return Result and updated all call sites to handle errors gracefully. Errors are now properly returned instead of causing panics.

Steake

@copilot fix recommended Issues

…panic - Change ValidatorNode::with_key() and MinerNode::with_key() to return Result - Change ValidatorNode::new() and MinerNode::new() to return Result - Update all call sites in main.rs to handle errors gracefully - Update test cases to unwrap Results - Addresses PR review feedback about using .expect() causing panics Co-authored-by: Steake <530040+Steake@users.noreply.github.com>

- Remove unused import COIN from blockchain.rs - Remove unused import SecretKey from main.rs - Remove empty line after doc comment in miner.rs - Remove empty line after doc comment in blockchain.rs Co-authored-by: Steake <530040+Steake@users.noreply.github.com>

Copilot · 2025-12-08T17:06:15Z

@copilot fix recommended Issues

Fixed in commit e76b46d. Resolved clippy warnings for bitcell-node:

Removed unused import COIN from blockchain.rs
Removed unused import SecretKey from main.rs
Fixed doc comment formatting in miner.rs and blockchain.rs

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

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

Copilot · 2025-12-08T20:05:59Z

crates/bitcell-node/src/miner.rs

@@ -1,5 +1,4 @@
 ///! Miner node implementation


Incorrect doc comment syntax. Module-level doc comments should use //! not ///!. The triple slash with exclamation (///!) is invalid - use either /// for item-level docs or //! for module-level docs.

Suggested change

///! Miner node implementation

//! Miner node implementation

Fixed in commit 480c46e. Changed ///! to //! for proper module-level doc comment syntax.

Copilot · 2025-12-08T20:06:00Z

crates/bitcell-node/src/validator.rs

-        Self {
+        Ok(Self {
            config,
            state: StateManager::new(),


Inconsistent state management: ValidatorNode creates a separate in-memory StateManager (line 62) while Blockchain has its own StateManager with storage (lines 44-55). This creates two independent state managers that could diverge. Consider either:

Removing the ValidatorNode's separate state field if Blockchain manages state, or

Sharing the same StateManager instance between ValidatorNode and Blockchain by extracting it from the blockchain after creation.

The same issue exists in MinerNode.

Fixed in commit 480c46e. Removed the separate StateManager from ValidatorNode. The node now uses the Blockchain's StateManager which includes storage support when configured with --data-dir.

Copilot · 2025-12-08T20:06:00Z

crates/bitcell-node/src/miner.rs

+        Ok(Self {
            config,
            secret_key,
            state: StateManager::new(),


Inconsistent state management: MinerNode creates a separate in-memory StateManager (line 48) while Blockchain has its own StateManager with storage (lines 30-41). This creates two independent state managers that could diverge. Consider either:

Removing the MinerNode's separate state field if Blockchain manages state, or

Sharing the same StateManager instance between MinerNode and Blockchain by extracting it from the blockchain after creation.

The same issue exists in ValidatorNode.

Fixed in commit 480c46e. Removed the separate StateManager from MinerNode. The node now uses the Blockchain's StateManager which includes storage support when configured with --data-dir.

Steake · 2025-12-08T21:39:37Z

@copilot Please address the comments from the code review on this branch.

- Fix doc comment syntax in miner.rs (use //! instead of ///!) - Remove duplicate StateManager from ValidatorNode and MinerNode - Both nodes now use Blockchain's StateManager with storage support - Update test to access state through blockchain.state() method - Resolves inconsistent state management between nodes and blockchain Co-authored-by: Steake <530040+Steake@users.noreply.github.com>

Copilot · 2025-12-08T21:52:46Z

@copilot Please address the comments from the code review on this branch.

Addressed all code review comments in commit 480c46e:

Fixed doc comment syntax (///! → //!)
Removed duplicate StateManager from ValidatorNode and MinerNode
Both nodes now use Blockchain's StateManager with storage support

Steake · 2025-12-09T11:42:46Z

@copilot fix the merge conflict with this branch into master, and commit the merge-ready commit to this branch

- Resolved merge conflict in crates/bitcell-node/src/blockchain.rs - Kept both test_blockchain_with_persistent_storage and new VRF tests - Integrated wallet documentation and VRF integration test from master - All conflicts resolved and ready for merge Co-authored-by: Steake <530040+Steake@users.noreply.github.com>

Initial plan

09f3047

Copilot AI assigned Copilot and Steake Dec 6, 2025

Copilot started work on behalf of Steake December 6, 2025 23:11 View session

Copilot AI and others added 3 commits December 6, 2025 23:31

Add integration test for blockchain persistence

b8d4a76

- Add test_blockchain_with_persistent_storage test - Add tempfile as dev dependency for bitcell-node - Verify accounts persist correctly across blockchain restarts Co-authored-by: Steake <530040+Steake@users.noreply.github.com>

Optimize trace logging to avoid unnecessary hex encoding

e9ec102

- Only compute hex encoding when trace level is enabled - Addresses code review feedback for performance optimization Co-authored-by: Steake <530040+Steake@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Implement state persistence using RocksDB backend~~ Implement RocksDB-backed state persistence for accounts and bonds Dec 6, 2025

Copilot AI requested a review from Steake December 6, 2025 23:49

Copilot finished work on behalf of Steake December 6, 2025 23:49

Steake requested a review from Copilot December 7, 2025 07:53

Copilot started reviewing on behalf of Steake December 7, 2025 07:54 View session

Copilot AI reviewed Dec 7, 2025

View reviewed changes

Steake removed their assignment Dec 7, 2025

Copilot started work on behalf of Steake December 8, 2025 16:37 View session

Steake reviewed Dec 8, 2025

View reviewed changes

Copilot AI requested a review from Steake December 8, 2025 16:51

Copilot finished work on behalf of Steake December 8, 2025 16:51

Copilot started work on behalf of Steake December 8, 2025 16:51 View session

Copilot finished work on behalf of Steake December 8, 2025 17:07

Steake marked this pull request as ready for review December 8, 2025 20:01

Copilot AI review requested due to automatic review settings December 8, 2025 20:01

Copilot started reviewing on behalf of Steake December 8, 2025 20:02 View session

Copilot AI reviewed Dec 8, 2025

View reviewed changes

Copilot started work on behalf of Steake December 8, 2025 21:40 View session

Copilot finished work on behalf of Steake December 8, 2025 21:54

Steake self-assigned this Dec 9, 2025

Copilot started work on behalf of Steake December 9, 2025 11:43 View session

Copilot finished work on behalf of Steake December 9, 2025 12:39

Merge branch 'master' into copilot/implement-state-persistence-rocksdb

2e3b4aa

Steake merged commit 81091cc into master Dec 10, 2025
2 of 8 checks passed

Comments

Conversation

Copilot AI commented Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comments on the Issue (you are @copilot in this section)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

Uh oh!

Steake Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Steake left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Dec 8, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Steake commented Dec 8, 2025

Uh oh!

Copilot AI commented Dec 8, 2025

Uh oh!

Steake commented Dec 9, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Dec 6, 2025 •

edited

Loading