Skip to content

Add minimal test suite for transaction signing and verification#55

Open
shrilakshmikakati wants to merge 4 commits intoStabilityNexus:mainfrom
shrilakshmikakati:add-transaction-signing-tests
Open

Add minimal test suite for transaction signing and verification#55
shrilakshmikakati wants to merge 4 commits intoStabilityNexus:mainfrom
shrilakshmikakati:add-transaction-signing-tests

Conversation

@shrilakshmikakati
Copy link

@shrilakshmikakati shrilakshmikakati commented Mar 10, 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.

After Adding Testcases
Screenshot from 2026-03-10 13-14-12

Summary by CodeRabbit

  • Tests
    • Added comprehensive transaction-signing and verification tests covering valid signatures, tampering detection (amount/receiver/nonce), invalid-key and forged-sender rejection, unsigned-transaction rejection, and replay protection via nonce ordering.
    • Tests also exercise balance and nonce updates and are implemented as standalone test functions using cryptographic signing and state validation.

Signed-off-by: shrilakshmikakati <shrilakshmikakati@gmail.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 10, 2026

Warning

Rate limit exceeded

@shrilakshmikakati has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 14 minutes and 5 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e0871480-d622-49fc-a225-199bf4e99baf

📥 Commits

Reviewing files that changed from the base of the PR and between 16ccfdc and 7127de8.

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

Walkthrough

Adds a new pytest module that verifies MiniChain Transaction signing and verification, tamper detection (amount/receiver/nonce), invalid/mismatched sender handling, unsigned transaction rejection, and nonce-based replay protection with state updates.

Changes

Cohort / File(s) Summary
Transaction Signing Tests
tests/test_transaction_signing.py
New test module (~194 lines) adding fixtures for Alice/Bob signing keys and a funded state; tests for valid signatures, tampering (amount/receiver/nonce), invalid-key/mismatched-sender scenarios, unsigned transaction rejection, and nonce replay/out-of-order acceptance with balance/nonce updates.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Possibly related PRs

  • Feature/testnet demo #46: Modifies Transaction.init timestamp handling which could affect tests that construct Transaction instances and rely on default timestamp behavior.

Suggested labels

Python Lang

Suggested reviewers

  • Zahnentferner

Poem

🐰 I hopped with keys beneath the moon,

Signed a coin and hummed a tune,
Tamper tried to spoil the game,
Nonces kept the ledger tame,
Hooray — tests prove the chain's true boon!

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add minimal test suite for transaction signing and verification' directly and accurately describes the main change: addition of a new test module with 10 test cases covering transaction signing and verification scenarios.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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
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: 1

🤖 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.py`:
- Line 20: The test currently uses a unittest.TestCase class
(TestTransactionSigning); migrate to pytest-style by replacing the
TestTransactionSigning class and its methods with standalone test_ functions,
switch unittest assertions (e.g., self.assertEqual, self.assertTrue,
self.assertRaises) to plain assert statements and pytest.raises for exception
checks, and remove any unittest.TestCase inheritance and setUp/tearDown in favor
of fixtures or module-level setup; update imports to remove unittest and add
pytest where needed.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6161b5ab-bd00-4b1b-8f82-88534df2f92f

📥 Commits

Reviewing files that changed from the base of the PR and between ff5e83a and 3550f72.

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

Signed-off-by: shrilakshmikakati <shrilakshmikakati@gmail.com>
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.py`:
- Around line 168-184: Add an assertion that the sender's nonce advanced to 2
after applying two sequential transactions in test_sequential_nonces_accepted:
after the existing balance checks, fetch Alice's account via
funded_state.get_account(alice_pk) and assert its "nonce" field equals 2 to
ensure replay-protection bookkeeping is updated (reference:
test_sequential_nonces_accepted, funded_state.get_account).
- Around line 145-165: In both test_replay_attack_same_nonce_rejected and
test_out_of_order_nonce_rejected, snapshot Alice’s pre-call account state
(balance and nonce) from funded_state for alice_pk, call
funded_state.apply_transaction(tx) as currently done, and after the rejected
call assert that funded_state still reports the same balance and nonce for
alice_pk (i.e., no mutation occurred). Reference the Transaction instance (tx),
the funded_state.apply_transaction method, and the alice_pk key when taking and
comparing the before/after snapshots to ensure rejected submissions are
side-effect free.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2171a8ad-ed1b-4a4b-b72f-4d620e8e4641

📥 Commits

Reviewing files that changed from the base of the PR and between 3550f72 and 380e834.

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

…ancement

Signed-off-by: shrilakshmikakati <shrilakshmikakati@gmail.com>
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: 1

🤖 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.py`:
- Around line 6-11: Update the module docstring summary so bullet 3 accurately
reflects the API: instead of saying the wrong sender key "fails verification,"
state that Transaction.sign(...) raises a ValueError when the provided signing
key does not match the transaction sender; locate the header in
tests/test_transaction_signing.py and adjust the wording to mention signing-time
rejection by Transaction.sign rather than post-signature verification.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ff082096-6caf-404b-b96a-a59f0f1f4ca7

📥 Commits

Reviewing files that changed from the base of the PR and between 380e834 and 16ccfdc.

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

…re for case 3

Signed-off-by: shrilakshmikakati <shrilakshmikakati@gmail.com>
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