Skip to content

Fix timestamp validation#80

Open
Moksha25-tech wants to merge 6 commits intoStabilityNexus:mainfrom
Moksha25-tech:fix-timestamp-validation
Open

Fix timestamp validation#80
Moksha25-tech wants to merge 6 commits intoStabilityNexus:mainfrom
Moksha25-tech:fix-timestamp-validation

Conversation

@Moksha25-tech
Copy link
Copy Markdown

@Moksha25-tech Moksha25-tech commented Mar 28, 2026

Addressed Issues:

Fixes #79

Results (Before/After) :

Before :
Screenshot 2026-03-28 142943
Screenshot 2026-03-28 142957

After:
Screenshot 2026-03-28 143723

Additional Notes:

  • Fixed timestamp validation vulnerability in blockchain.
  • Resolved lint errors across the project.
  • Ensured code follows project linting and formatting rules.
  • Removed venv from tracking and added it to .gitignore to prevent unnecessary files in commits.
  • No functional behavior of blockchain logic was changed except the timestamp validation fix.
  • All tests run successfully and no new warnings/errors were introduced.

This PR focuses only on improving code quality and fixing the timestamp validation issue to ensure better maintainability and consistency for future development.

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:

  • This PR does not contain AI-generated code at all.
  • This PR contains AI-generated code. I have read the AI Usage Policy and this PR complies with this policy. I have tested the code locally and I am responsible for it.

I have used the following AI models and tools:
ChatGPT (for lint error debugging and PR formatting guidance)

Checklist

  • My PR addresses a single issue, fixes a single bug or makes a single improvement.
  • My code follows the project's code style and conventions
  • If applicable, I have made corresponding changes or additions to the documentation
  • If applicable, I have made corresponding changes or additions to tests
  • My changes generate no new warnings or errors
  • I have joined the Discord server and I will share a link to this PR with the project maintainers there
  • I have read the Contribution Guidelines
  • Once I submit my PR, CodeRabbit AI will automatically review it and I will address CodeRabbit's comments.
  • I have filled this PR template completely and carefully, and I understand that my PR may be closed without review otherwise.

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Added timestamp validation for blocks to reject timestamps earlier than the previous block or exceeding current time by more than 60 seconds.
  • Chores

    • Updated project configuration for improved development environment setup (virtual environment and package metadata files).
  • Style

    • Code formatting and import organization improvements across the codebase for consistency and readability.
  • Tests

    • Added test utilities to verify timestamp handling behavior.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f9d52da4-3370-4600-b49f-4b3f2b33b586

📥 Commits

Reviewing files that changed from the base of the PR and between 316703e and c4363a4.

📒 Files selected for processing (1)
  • .gitignore

Walkthrough

This PR applies formatting and import reordering across the codebase while adding timestamp validation to block acceptance in the blockchain. The changes enforce that new blocks must have timestamps greater than or equal to the previous block and within 60 seconds of the current system time.

Changes

Cohort / File(s) Summary
Timestamp Validation
minichain/chain.py
Added block-level timestamp checks in add_block() to reject blocks with timestamps earlier than the previous block or exceeding current time by >60,000ms. Previous block capture, conditional branches for rejection, and corresponding warning logs were added to the validation flow.
Bug Reproduction Script
reproduce_timestamp_bug.py
New standalone test script demonstrating the timestamp vulnerability by creating and attempting to add blocks with past and future timestamps, reporting whether the blockchain accepts them.
Build & Environment Configuration
.gitignore, setup.py
Added virtual environment directory patterns (venv/, .venv/) and egg-info patterns to .gitignore. Reordered setup() and find_packages import order in setup.py.
Formatting & Import Reorganization
conftest.py, minichain/__init__.py, minichain/block.py, minichain/contract.py, minichain/mempool.py, minichain/p2p.py, minichain/persistence.py, minichain/pow.py, minichain/serialization.py, minichain/state.py, minichain/transaction.py
Reordered imports (moving standard library first, adjusting relative positioning of modules), normalized quote style from single to double quotes in dictionary access, collapsed/expanded multi-line expressions for readability, added/removed blank lines and whitespace. No functional behavior changes.
Test Suite Formatting
tests/test_core.py, tests/test_contract.py, tests/test_persistence.py, tests/test_protocol_hardening.py, tests/test_transaction_signing.py
Reordered imports, normalized quote style, reformatted assertions and block construction into multi-line expressions, updated test runner guard formatting. No test logic or assertion values changed.
Main Application Formatting
main.py
Reformatted multi-line imports, expanded logging statements, adjusted CLI argument setup and JSON message construction, changed function signature formatting. No functional behavior changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

Possibly related PRs

Suggested labels

Python Lang, Linter

Suggested reviewers

  • Zahnentferner

Poem

🐰 Timestamps now march in order true,
No time-warp tricks can slip on through,
Formatting clean from top to toe,
With validation's steady glow,
The chain stands strong, secure and bright!

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes extensive formatting and import reordering changes across all files (adding trailing commas, reformatting multi-line calls, normalizing quotes) that are not required by issue #79 and were only listed as addressing 'lint errors' without explicit justification. Separate formatting/linting changes from functional fixes into a dedicated PR, or document why each formatting change was necessary for the timestamp validation fix.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Fix timestamp validation' directly and clearly summarizes the primary change: implementing timestamp validation to prevent past and far-future timestamp blocks from being added to the blockchain, addressing issue #79.
Linked Issues check ✅ Passed The PR successfully implements all key coding requirements from issue #79: enforces monotonic timestamp progression (rejecting past timestamps), implements future timestamp bounds (60,000ms limit), updates chain.py add_block() validation logic, and includes a working reproduction script demonstrating the fix.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🤖 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.egg-info/dependency_links.txt`:
- Line 1: The repository is tracking generated packaging metadata (egg-info)
which should be untracked; remove the committed egg-info artifacts (e.g., the
dependency_links file) from version control, add the pattern *.egg-info/ to
.gitignore to prevent future commits, and untrack existing files (stop tracking
cached files) before committing the change so stale packaging metadata is not
stored in the repo.

In `@minichain.egg-info/PKG-INFO`:
- Around line 1-8: The committed generated metadata (minichain.egg-info/ and
files like PKG-INFO) should be removed and ignored; delete the generated
artifact from the repo and prevent future commits by adding minichain.egg-info/
to .gitignore, run git rm -r --cached minichain.egg-info (or remove the folder
and commit the deletion), and commit the updated .gitignore so the packaging
step still generates these files locally but they no longer appear in source
control.

In `@minichain.egg-info/SOURCES.txt`:
- Around line 1-27: The repo currently tracks the autogenerated metadata
directory minichain.egg-info which should be ignored; update the repository
.gitignore (in the Python section) to add the egg-info pattern (e.g., add
*.egg-info/ or minichain.egg-info/) so that the minichain.egg-info/ directory is
not committed, then remove the tracked directory from git history with git rm
--cached if already committed.

In `@minichain.egg-info/top_level.txt`:
- Around line 1-3: The package metadata is shipping the test package because
package discovery is including "tests"; open the setup configuration where
find_packages or setuptools.setup is called and change package discovery to
exclude tests (e.g., call find_packages with exclude=["tests", "tests.*"] or
otherwise remove "tests" from the packages list) so the top_level.txt no longer
lists "tests" in the distributed metadata.

In `@minichain/mempool.py`:
- Around line 56-63: Flatten the nested checks by combining the `if txs:` and
the inner comparison into one condition so you only evaluate the comparison when
`txs` is truthy; update the block that assigns `best_tx`/`best_sender` (the one
referencing variables txs, best_tx, best_sender, sender, best_tx.timestamp,
best_tx.nonce) to use a single if statement like `if txs and (best_tx is None or
(txs[0].timestamp, sender, txs[0].nonce) < (best_tx.timestamp, best_sender,
best_tx.nonce)):` and keep the existing assignments to `best_tx = txs[0]` and
`best_sender = sender`.

In `@reproduce_timestamp_bug.py`:
- Around line 46-48: The test constructs future_block with
previous_hash=past_block.hash which makes it fail due to bad linkage when
past_block is rejected; change the test to link future_block to the current
chain tip instead (use the chain's tip/hash accessor or the latest block
variable instead of past_block.hash) so the block is rejected specifically for
the future timestamp; update the call that creates future_block (Block(...
previous_hash=...)) to reference the chain tip (e.g., chain[-1].hash or
get_chain_tip().hash) while keeping timestamp=9999999999999 and transactions=[]
unchanged.
🪄 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: 75d08c47-1250-4e7f-951a-cc0d28d816cf

📥 Commits

Reviewing files that changed from the base of the PR and between 518c70a and 316703e.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (27)
  • .gitignore
  • conftest.py
  • main.py
  • minichain.egg-info/PKG-INFO
  • minichain.egg-info/SOURCES.txt
  • minichain.egg-info/dependency_links.txt
  • minichain.egg-info/entry_points.txt
  • minichain.egg-info/requires.txt
  • minichain.egg-info/top_level.txt
  • minichain/__init__.py
  • minichain/block.py
  • minichain/chain.py
  • minichain/contract.py
  • minichain/mempool.py
  • minichain/p2p.py
  • minichain/persistence.py
  • minichain/pow.py
  • minichain/serialization.py
  • minichain/state.py
  • minichain/transaction.py
  • reproduce_timestamp_bug.py
  • setup.py
  • tests/test_contract.py
  • tests/test_core.py
  • tests/test_persistence.py
  • tests/test_protocol_hardening.py
  • tests/test_transaction_signing.py

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.

[BUG]: Potential time-warp attack due to missing block timestamp validation

1 participant