persist blockchain across sessions via save/load#56
Open
aniket866 wants to merge 3 commits intoStabilityNexus:mainfrom
Open
persist blockchain across sessions via save/load#56aniket866 wants to merge 3 commits intoStabilityNexus:mainfrom
aniket866 wants to merge 3 commits intoStabilityNexus:mainfrom
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughA new persistence module is added to save and load blockchain state to JSON files. The save function writes chain and account data, while load reconstructs the blockchain from saved JSON. The module is exported via Changes
Sequence DiagramsequenceDiagram
participant User as User/Client
participant Save as save()
participant FS as Filesystem
participant Load as load()
participant Blockchain as Blockchain<br/>(reconstructed)
User->>Save: save(blockchain, path)
Save->>FS: Write blockchain.json<br/>(chain data)
Save->>FS: Write state.json<br/>(accounts data)
FS-->>Save: Files created
User->>Load: load(path)
Load->>FS: Read blockchain.json
FS-->>Load: Chain data
Load->>FS: Read state.json
FS-->>Load: Accounts data
Load->>Load: Deserialize Blocks &<br/>Transactions
Load->>Blockchain: Reconstruct state &<br/>initialize ContractMachine
Blockchain-->>Load: Initialized instance
Load-->>User: Blockchain object
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@minichain/persistence.py`:
- Around line 75-88: The function currently validates raw_blocks but assigns
raw_accounts directly; add a validation step before creating blockchain.state to
ensure raw_accounts is a dict (and non-empty if desired), e.g. verify
isinstance(raw_accounts, dict) and raise a ValueError with context including
chain_path if the check fails; then proceed to create State via
State.__new__(State) and assign blockchain.state.accounts = raw_accounts and
blockchain.state.contract_machine = ContractMachine(blockchain.state) as before,
referencing the existing symbols raw_accounts, State, ContractMachine,
blockchain.state, and chain_path to locate the insertion point.
- Around line 79-81: Move the standard-library import "threading" out of the
local scope and place it at module-level with the other imports (so remove the
inline "import threading" inside the function) and update any references to
threading.Thread or threading.Lock in this module accordingly; specifically,
remove the local import in the function whereState and ContractMachine are used
and add a single top-level "import threading" next to the other imports so the
module uses the shared top-level threading name.
- Line 109: Remove the unnecessary explicit "r" mode when opening files: locate
the with open(filepath, "r", encoding="utf-8") as f usage in persistence.py (the
open call that reads the file into f) and change it to with open(filepath,
encoding="utf-8") as f so the default read mode is used and functionality
remains identical.
In `@tests/test_persistence.py`:
- Line 116: The test unpacks alice_pk and bob_pk from self._chain_with_tx() but
never uses them; change the unpacking to prefix unused variables with
underscores (e.g., bc, _alice_pk, _bob_pk) to signal they are intentionally
ignored and avoid lint warnings—update the tuple assignment where
self._chain_with_tx() is called in the test to use the underscore-prefixed
names.
- Line 71: Update the zip call in the test loop that iterates over bc.chain and
restored.chain to use strict=True so the comparison fails if lengths differ;
specifically modify the loop in tests/test_persistence.py (the for original,
loaded in zip(bc.chain, restored.chain) loop) to pass strict=True to zip() to
enforce equal-length iteration.
- Around line 24-25: The setUp method creates a temp directory (self.tmpdir) but
never removes it; add a tearDown method that checks for and removes self.tmpdir
(e.g., using shutil.rmtree) to clean up after each test. Implement tearDown in
the same test class that defines setUp, guard against missing/None self.tmpdir,
and handle exceptions (or ignore errors) so test teardown never fails.
- Around line 169-170: The test test_contract_storage_preserved currently only
asserts restored.state.get_account(contract_addr)["code"] equals code; add an
assertion to also verify the contract storage survived the save/load cycle by
fetching the account via restored.state.get_account(contract_addr) and asserting
that its storage (e.g. contract["storage"]["hits"] or the appropriate storage
key) equals 1, ensuring storage["hits"] is present and has the expected value
after restore.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 66734a3a-c93b-4296-bffc-fe3db8634f61
📒 Files selected for processing (3)
minichain/__init__.pyminichain/persistence.pytests/test_persistence.py
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addressed Issues:
Fixes #57
What
Adds
minichain/persistence.py— a minimal module that can write the full blockchain and account state to disk and restore it across sessions.Why
Previously the entire chain lived in memory and was lost on every restart. This made multi-session research, demos, and testnet usage impossible.
API
load()returns a fully workingBlockchaininstance — new blocks can be mined and added immediately after loading.Files Changed
Tests
All 9 tests cover: file creation, block hash integrity, account balances, nonces, transaction fields, contract storage, adding new blocks after load, missing-file error, and genesis-only chains.
Notes
jsonandosmodules.load()usesBlockchain.__new__()to skip genesis-block creation, then wires up the restored chain and state directly. This avoids any hidden side effects from__init__.merkle_rootis preserved as-is rather than recomputed, so the module stays correct even if the hashing algorithm changes in the future.Screenshots/Recordings:
TODO: If applicable, add screenshots or recordings that demonstrate the interface before and after the changes.
Additional Notes:
AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: Chatgpt
Checklist
Summary by CodeRabbit
New Features
Tests