feat: Implement StateJournal and in-memory snapshots for state rollback - #134
Conversation
WalkthroughThe PR adds journaled account updates and bounded blockchain state snapshots. Transactions now commit or roll back through ChangesState consistency and snapshot management
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Blockchain
participant State
participant StateJournal
participant SnapshotDeque
Blockchain->>State: apply validated transaction
State->>StateJournal: track account mutations
StateJournal-->>State: commit or rollback changes
Blockchain->>State: finalize committed account state
Blockchain->>SnapshotDeque: store block state snapshot
Blockchain->>SnapshotDeque: restore cached fork-point snapshot during reorganization
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
| self._lock = threading.RLock() | ||
| import collections | ||
| self._state_snapshots = collections.OrderedDict() | ||
| self._max_snapshots = 10 |
There was a problem hiding this comment.
Make this a node-level configuration parameter.
| self.chain_id = "minichain-default" | ||
| self._lock = threading.RLock() | ||
| import collections | ||
| self._state_snapshots = collections.OrderedDict() |
There was a problem hiding this comment.
I wondered if OrderedDict is really the best data structure for this purpose.
Fror example, deque (https://realpython.com/python-deque/) could be a good alternative, because it automatically evicts old items from the queue.
Could you investigate this?
There was a problem hiding this comment.
You're right, I investigated it and you're right that deque(maxlen=MAX_STATE_SNAPSHOTS) handles eviction much cleaner, so I've updated it to use that
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
minichain/chain.py (1)
303-324: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftRebuild the snapshot cache after a successful reorganization.
Line 324 appends only the new tip snapshot. Snapshots for orphaned block hashes remain in
_state_snapshots. Snapshots for newly adopted intermediate blocks are not recorded.A later shallow reorganization at one of those intermediate blocks cannot find
snapshot_found. It falls back to replay even when the fork is withinMAX_STATE_SNAPSHOTS.Capture a snapshot after each successfully replayed block. Then replace
_state_snapshotswith snapshots for the recent canonical blocks inproposed_chainonly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@minichain/chain.py` around lines 303 - 324, The reorganization flow must rebuild _state_snapshots for the canonical chain instead of appending only the new tip. In the block replay loop within the reorganization method, capture a snapshot after each successfully applied block, then replace _state_snapshots with snapshots for only the recent blocks from proposed_chain, bounded by MAX_STATE_SNAPSHOTS; remove orphaned entries while retaining intermediate adopted-block snapshots for future shallow reorganizations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@minichain/chain.py`:
- Around line 303-324: The reorganization flow must rebuild _state_snapshots for
the canonical chain instead of appending only the new tip. In the block replay
loop within the reorganization method, capture a snapshot after each
successfully applied block, then replace _state_snapshots with snapshots for
only the recent blocks from proposed_chain, bounded by MAX_STATE_SNAPSHOTS;
remove orphaned entries while retaining intermediate adopted-block snapshots for
future shallow reorganizations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 920b0513-a7f2-4896-bd65-8446a6291426
📒 Files selected for processing (3)
minichain/chain.pyminichain/node_config.pyminichain/state.py
|
Please resolve the merge conflicts before review. Your PR will only be reviewed by a maintainer after all conflicts have been resolved. 📺 Watch this video to understand why conflicts occur and how to resolve them: |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
minichain/chain.py (3)
152-158: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCopy
self.chainwhile holding_lock.When
chain_listisNone, this code stores the mutableself.chainand releases the lock before summing it. A concurrentadd_blockcan change the sequence during iteration. Copy the chain inside the lock, for example withtuple(self.chain).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@minichain/chain.py` around lines 152 - 158, Update the chain_list initialization in the work-calculation method to copy self.chain while holding self._lock, using an immutable snapshot such as tuple(self.chain), before releasing the lock; preserve the existing summation over chain_list.
278-287: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winValidate incoming block hashes before calculating chain work.
Line 285 can raise
ValueErrororTypeErrorfor malformed incoming data, andresolve_conflictsdoes not convert that failure into(False, []). A fabricated hexadecimal hash can also pass this numeric check and inflatenew_work;_apply_blockrejects it only after the work comparison. Run header validation beforeget_total_work, or reject malformed hashes in this fast path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@minichain/chain.py` around lines 278 - 287, Update the fast PoW validation in resolve_conflicts to validate each incoming block hash before get_total_work calculates new_work. Catch malformed or non-hex hash values and reject them with (False, []) instead of allowing ValueError or TypeError to escape; ensure fabricated hashes cannot pass the numeric target comparison and inflate chain work, while preserving existing invalid-target rejection.
96-109: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate the genesis consensus parameters before use.
int(raw_target)silently converts JSON floats and booleans. An invalid hex string raises before the configuration error path logs the problem.target_block_timeandalphaare also accepted without constraints. A zerotarget_block_timecauses division by zero at Line 167. Validate the target format, require a positive integertarget_block_time, and requirealphato be between 0 and 1.Also applies to: 160-169
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@minichain/chain.py` around lines 96 - 109, Validate all genesis consensus parameters before assigning or using them in the chain initialization flow: accept target only as a valid positive integer or hexadecimal string, reject booleans, floats, and malformed hex through the existing configuration error path; require target_block_time to be a positive integer and alpha to be within 0 to 1. Ensure these checks occur before the difficulty adjustment logic that uses them, including the flow around avg_block_time and target recalculation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@minichain/chain.py`:
- Around line 303-305: Authenticate the incoming genesis block before reading
its target in the fork_idx == 0 path. Update the proposed_chain/new_chain_list
handling so the genesis header is recomputed and matched against the local
genesis snapshot, or replace the incoming genesis with self.chain[0] before
temp_target and subsequent validation use it; do not rely only on the mutable
hash field.
- Around line 350-351: Update the reorganization flow around snapshot validation
and the “Repopulate snapshots for the new chain tip” block to collect snapshots
for the retained portion of proposed_chain while validating it, then replace the
bounded snapshot deque only after the reorganization succeeds. Do not retain
old-branch snapshots or append only the new tip; preserve the deque’s configured
maximum length and ensure fork-point snapshots remain available for later
reorganizations.
---
Outside diff comments:
In `@minichain/chain.py`:
- Around line 152-158: Update the chain_list initialization in the
work-calculation method to copy self.chain while holding self._lock, using an
immutable snapshot such as tuple(self.chain), before releasing the lock;
preserve the existing summation over chain_list.
- Around line 278-287: Update the fast PoW validation in resolve_conflicts to
validate each incoming block hash before get_total_work calculates new_work.
Catch malformed or non-hex hash values and reject them with (False, []) instead
of allowing ValueError or TypeError to escape; ensure fabricated hashes cannot
pass the numeric target comparison and inflate chain work, while preserving
existing invalid-target rejection.
- Around line 96-109: Validate all genesis consensus parameters before assigning
or using them in the chain initialization flow: accept target only as a valid
positive integer or hexadecimal string, reject booleans, floats, and malformed
hex through the existing configuration error path; require target_block_time to
be a positive integer and alpha to be within 0 to 1. Ensure these checks occur
before the difficulty adjustment logic that uses them, including the flow around
avg_block_time and target recalculation.
🪄 Autofix
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 Plus
Run ID: 99717d9e-abb2-449c-aec5-6d0f1fc47f0c
📒 Files selected for processing (2)
minichain/chain.pyminichain/node_config.py
| fork_base_hash = self.chain[fork_idx - 1].hash if fork_idx > 0 else None | ||
|
|
||
| temp_target = proposed_chain[0].target |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Authenticate the incoming genesis block before using its target.
When fork_idx == 0, proposed_chain[0] comes from new_chain_list, but the validation loop starts at block 1. The code checks only the incoming object's mutable hash field. It can therefore use a different genesis target or state root with the local genesis snapshot. Recompute and compare the complete genesis header, or replace the incoming genesis object with self.chain[0] before using it.
Also applies to: 325-330
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@minichain/chain.py` around lines 303 - 305, Authenticate the incoming genesis
block before reading its target in the fork_idx == 0 path. Update the
proposed_chain/new_chain_list handling so the genesis header is recomputed and
matched against the local genesis snapshot, or replace the incoming genesis with
self.chain[0] before temp_target and subsequent validation use it; do not rely
only on the mutable hash field.
| # Repopulate snapshots for the new chain tip | ||
| self._state_snapshots.append((self.last_block.hash, self.state.snapshot())) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Rebuild the bounded snapshot deque after a reorganization.
This code appends only the new tip and retains snapshots from the old branch. After a deep reorganization, a later shallow reorganization on the new branch cannot find its fork-point snapshot and falls back to replay from genesis. Collect snapshots for the retained portion of proposed_chain while validating it, then replace the deque only after the reorganization succeeds.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@minichain/chain.py` around lines 350 - 351, Update the reorganization flow
around snapshot validation and the “Repopulate snapshots for the new chain tip”
block to collect snapshots for the retained portion of proposed_chain while
validating it, then replace the bounded snapshot deque only after the
reorganization succeeds. Do not retain old-branch snapshots or append only the
new tip; preserve the deque’s configured maximum length and ensure fork-point
snapshots remain available for later reorganizations.
| snapshot_found = snap | ||
| break | ||
|
|
||
| if snapshot_found is not None: |
There was a problem hiding this comment.
Wouldn't if snapshot_found: suffice here?
2 participants
Addressed Issues:
Previously, the node relied heavily on full copy.deepcopy() operations of the entire state dictionary. This PR replaces that with a highly optimized two-tier rollback architecture:
Problem: When validating transactions inside a block, the node was making an expensive deep copy of the state before every transaction to allow for gas refunds/reverts upon failure.
Solution: Introduced a StateJournal proxy dictionary in minichain/state.py. It acts as an in-memory cache that tracks diffs during transaction execution in O(1) time.
Impact: If a transaction fails mid-execution (e.g., out of gas), the journal simply discards the cache (rollback()). If it succeeds, the diff is flushed to the backing dictionary (commit()), completely eliminating the need for transaction-level deep copies.
Problem: During a chain reorganization (resolve_conflicts), the node had to fetch the genesis state and sequentially re-apply every single transaction in history up to the fork point.
Solution: Implemented _state_snapshots in minichain/chain.py (an OrderedDict limited to the 10 most recent blocks).
Impact: When a shallow reorg occurs, the node now fast-forwards the difficulty/timestamp calculations and restores the exact state at the fork point directly from memory. Replaying from genesis is now strictly a fallback mechanism for incredibly deep reorgs.
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: TODO
Checklist
Summary by CodeRabbit
Bug Fixes
Performance