Skip to content

remove replace#66

Closed
kpj2006 wants to merge 2 commits intoStabilityNexus:mainfrom
kpj2006:main
Closed

remove replace#66
kpj2006 wants to merge 2 commits intoStabilityNexus:mainfrom
kpj2006:main

Conversation

@kpj2006
Copy link

@kpj2006 kpj2006 commented Mar 21, 2026

Addressed Issues:

Fixes #(TODO:issue number)

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:

  • 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: TODO

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

  • Tests
    • Removed test coverage for transaction replay protection and nonce-order validation scenarios.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 21, 2026

Walkthrough

A test file is modified to remove 55 lines of test coverage for transaction replay protection and nonce validation, including tests for replay rejection, skipped nonce handling, and sequential nonce advancement.

Changes

Cohort / File(s) Summary
Test Coverage Removal
tests/test_transaction_signing_karun.py
Deleted test cases covering transaction replay protection (same nonce rejection), skipped nonce rejection with ledger state assertions, and sequential nonce acceptance with balance verification.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

Python Lang

Poem

🐰 A handful of tests hop away into the night,
Replay guards and nonce checks fade from sight,
The warren trusts what remains so bright,
Less code to keep, yet the heart feels light.

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'remove replace' is vague and does not clearly convey what was removed or replaced, making it impossible to understand the actual change from the title alone. Use a more descriptive title that clearly indicates what was removed or replaced, such as 'Remove replay protection nonce tests' or 'Remove transaction replay-protection test coverage'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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.

Tip

CodeRabbit can use your project's `ruff` configuration to improve the quality of Python code reviews.

Add a Ruff configuration file to your project to customize how CodeRabbit runs ruff.

Copy link
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: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/test_transaction_signing_karun.py`:
- Around line 143-144: Re-add the replay/nonce regression tests you removed in
tests/test_transaction_signing_karun.py to cover duplicate-nonce rejection,
skipped-nonce rejection, and valid sequential-nonce progression against the
enforcement in minichain.state.State (the nonce/validation logic around lines
referenced). Concretely: add three test cases that (1) submit the same nonce
twice and assert the second submit is rejected/raises, (2) submit a tx with a
nonce that skips the current account nonce and assert it is rejected/raises, and
(3) submit txs with increasing sequential nonces and assert they are accepted
and update the account nonce accordingly; use the same transaction
construction/helpers present in the test file and assert against
State.apply_transaction / State.process_transaction results or exceptions to
verify the behavior.
- Line 144: Remove the orphaned indented string literal "Bob's balance should be
20 after receiving two transfers." at the EOF of
tests/test_transaction_signing_karun.py (the standalone indented string on line
144) which causes the IndentationError; either delete that line or convert it
into a proper comment or docstring placed at the top of the test function,
ensuring no stray indented string remains so pytest can collect tests.
🪄 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: 099e5f79-3da1-4178-a668-f1a8e60e0d2e

📥 Commits

Reviewing files that changed from the base of the PR and between 1b61cae and 73fe996.

📒 Files selected for processing (1)
  • tests/test_transaction_signing_karun.py

Comment on lines 143 to +144

assert funded_state.get_account(alice_pk)["nonce"] == 2, \
"Alice's nonce should advance to 2 after two accepted transactions."
assert funded_state.get_account(alice_pk)["balance"] == 80, \
"Alice's balance should be 80 after two 10-coin transfers."
assert funded_state.get_account(bob_pk)["balance"] == 20, \
"Bob's balance should be 20 after receiving two transfers." No newline at end of file
"Bob's balance should be 20 after receiving two transfers."
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Re-add replay/nonce regression tests removed in this deletion area.

This change drops coverage for duplicate nonce rejection, skipped nonce rejection, and sequential nonce progression. Those behaviors are enforced in minichain/state.py (Line 37-43 and Line 79-126), so removing these tests meaningfully weakens replay-protection safety.

🧰 Tools
🪛 Ruff (0.15.6)

[warning] 144-144: Unexpected indentation

(invalid-syntax)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_transaction_signing_karun.py` around lines 143 - 144, Re-add the
replay/nonce regression tests you removed in
tests/test_transaction_signing_karun.py to cover duplicate-nonce rejection,
skipped-nonce rejection, and valid sequential-nonce progression against the
enforcement in minichain.state.State (the nonce/validation logic around lines
referenced). Concretely: add three test cases that (1) submit the same nonce
twice and assert the second submit is rejected/raises, (2) submit a tx with a
nonce that skips the current account nonce and assert it is rejected/raises, and
(3) submit txs with increasing sequential nonces and assert they are accepted
and update the account nonce accordingly; use the same transaction
construction/helpers present in the test file and assert against
State.apply_transaction / State.process_transaction results or exceptions to
verify the behavior.

"Alice's balance should be 80 after two 10-coin transfers."
assert funded_state.get_account(bob_pk)["balance"] == 20, \
"Bob's balance should be 20 after receiving two transfers." No newline at end of file
"Bob's balance should be 20 after receiving two transfers."
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

fd 'test_transaction_signing_karun\.py$' | while read -r f; do
  echo "== $f =="
  nl -ba "$f" | sed -n '132,160p'
done

Repository: StabilityNexus/MiniChain

Length of output: 173


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the file and display lines around 144 using cat
fd 'test_transaction_signing_karun\.py$' -type f | while read -r f; do
  echo "== $f =="
  echo "=== File size ==="
  wc -l "$f"
  echo "=== Lines 130-160 with line numbers ==="
  cat -n "$f" | sed -n '130,160p'
done

Repository: StabilityNexus/MiniChain

Length of output: 238


🏁 Script executed:

#!/bin/bash
# Verify Python syntax by attempting to compile the file
fd 'test_transaction_signing_karun\.py$' -type f | while read -r f; do
  echo "== Syntax check for $f =="
  python3 -m py_compile "$f" 2>&1 || echo "Syntax error detected"
done

Repository: StabilityNexus/MiniChain

Length of output: 238


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find the file with correct fd syntax
fd 'test_transaction_signing_karun\.py$' | while read -r f; do
  echo "== $f =="
  echo "=== File line count ==="
  wc -l "$f"
  echo "=== Lines 130-160 ==="
  cat -n "$f" | sed -n '130,160p'
done

Repository: StabilityNexus/MiniChain

Length of output: 709


🏁 Script executed:

#!/bin/bash
# Verify Python syntax
python3 -c "
import sys
import py_compile

try:
    py_compile.compile('tests/test_transaction_signing_karun.py', doraise=True)
    print('✓ File syntax is valid')
except py_compile.PyCompileError as e:
    print(f'✗ Syntax error detected:')
    print(e)
    sys.exit(1)
"

Repository: StabilityNexus/MiniChain

Length of output: 181


Remove the orphaned indented string at EOF — it causes a Python IndentationError and prevents pytest collection.

Line 144 is a standalone string literal with unexpected indentation at the end of the file. Python cannot parse it, causing an IndentationError: unexpected indent. This blocks all test execution.

Fix
-        "Bob's balance should be 20 after receiving two transfers."
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"Bob's balance should be 20 after receiving two transfers."
🧰 Tools
🪛 Ruff (0.15.6)

[warning] 144-144: Unexpected indentation

(invalid-syntax)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_transaction_signing_karun.py` at line 144, Remove the orphaned
indented string literal "Bob's balance should be 20 after receiving two
transfers." at the EOF of tests/test_transaction_signing_karun.py (the
standalone indented string on line 144) which causes the IndentationError;
either delete that line or convert it into a proper comment or docstring placed
at the top of the test function, ensuring no stray indented string remains so
pytest can collect tests.

@kpj2006 kpj2006 closed this Mar 21, 2026
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.

1 participant