fix: preserve first cross-node nonce owner

[Asti1982]

Summary

• prevent store_used_cross_node_nonce() from replacing an active nonce record that already belongs to another node/miner
• clean expired nonce rows before inserting so expired nonces can still be reused safely
• add regression coverage for first-owner preservation and expired-record reuse

Root cause

The cross-node nonce table is meant to preserve the first node/miner that observed an active nonce. The storage path used INSERT OR REPLACE, so a second store call for the same active nonce could overwrite the original miner_id and node_id, erasing the provenance needed for replay/audit decisions.

Verification

• python -m pytest bounties\\issue-2296\\tests\\test_cross_node_replay_defense.py -q -> 35 passed
• python -m py_compile bounties\\issue-2296\\src\\cross_node_replay_defense.py bounties\\issue-2296\\tests\\test_cross_node_replay_defense.py -> passed
• git diff --check -> passed

[github-actions]

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

• Your PR has a BCOS-L1 or BCOS-L2 label
• New code files include an SPDX license header
• You've tested your changes against the live node

Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150)

A maintainer will review your PR soon. Thanks for contributing!

[saim256]

Approved current head 8ef0a88.

This is a focused fix for the cross-node nonce provenance regression: store_used_cross_node_nonce() now removes expired rows before insertion and uses INSERT OR IGNORE, so a replaying node/miner cannot overwrite the first active nonce owner. Returning False on the ignored insert also gives callers/tests a clear signal that the nonce was already tracked, while expired nonce reuse still works after cleanup.

Validation performed locally:

• python -m pytest bounties\issue-2296\tests\test_cross_node_replay_defense.py -q -> 35 passed
• python -m py_compile bounties\issue-2296\src\cross_node_replay_defense.py bounties\issue-2296\tests\test_cross_node_replay_defense.py -> passed
• python tools\bcos_spdx_check.py --base-ref origin/main -> BCOS SPDX check: OK
• git diff --check origin/main...HEAD -- bounties/issue-2296/src/cross_node_replay_defense.py bounties/issue-2296/tests/test_cross_node_replay_defense.py -> passed
• python -m ruff check bounties\issue-2296\src\cross_node_replay_defense.py bounties\issue-2296\tests\test_cross_node_replay_defense.py --select E9,F821,F811 --output-format=concise -> All checks passed

No blocking issues found.

[godd-ctrl]

Approved current head 8ef0a88.

The change fixes the provenance issue by replacing INSERT OR REPLACE with cleanup plus INSERT OR IGNORE, so an active nonce keeps its first miner/node owner and a second store attempt cannot overwrite it. Expired nonce rows are cleaned before insertion, preserving safe reuse after TTL expiry. The new regressions cover both first-owner preservation and expired-record reuse.

Validation performed:

• git diff --name-status origin/main...HEAD -> bounties/issue-2296/src/cross_node_replay_defense.py, bounties/issue-2296/tests/test_cross_node_replay_defense.py
• git diff --check origin/main...HEAD -> passed
• python tools\bcos_spdx_check.py --base-ref origin/main -> passed
• python -m py_compile bounties\issue-2296\src\cross_node_replay_defense.py bounties\issue-2296\tests\test_cross_node_replay_defense.py -> passed
• python -m pytest bounties\issue-2296\tests\test_cross_node_replay_defense.py -q -> 35 passed

No blocking issues found in the reviewed diff.

[idan57570-art]

LGTM. Logic verified and follows project standards.

[loganoe]

Review for Scottcjn/rustchain-bounties#73. Approved current head 8ef0a88bf0b966e2445db18c77a5a8751afb1734.

The change preserves the first active nonce owner by switching the insert path from INSERT OR REPLACE to INSERT OR IGNORE, checking rowcount, and returning False without overwriting the existing row. Calling cleanup_expired_nonces() before insert keeps the intended reuse behavior for expired records. The added tests cover both the active-owner preservation path and the expired-then-reused path.

Validation performed:

• python3 -m py_compile bounties/issue-2296/src/cross_node_replay_defense.py bounties/issue-2296/tests/test_cross_node_replay_defense.py -> passed
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --no-project --with pytest python -m pytest bounties/issue-2296/tests/test_cross_node_replay_defense.py -q -> 35 passed
• git diff --check origin/main...HEAD -> passed
• python3 tools/bcos_spdx_check.py --base-ref origin/main -> OK

No blocking findings from this focused review.

[jaxint]

Great contribution! LGTM.

[jaxint]

Great contribution! LGTM.

[jaxint]

Great contribution! LGTM.

[jaxint]

Great contribution! LGTM.

[jaxint]

Great contribution! LGTM.

[508704820]

Code Review: Preserve First Cross-Node Nonce Owner

Summary

Changes cross-node nonce storage from INSERT OR REPLACE (which silently overwrites) to INSERT OR IGNORE (which preserves the first owner). This prevents a subtle attack where a second node could overwrite a nonce's original miner_id, breaking the nonce-miner binding invariant.

What Works Well

1. Critical security fix: INSERT OR REPLACE was silently overwriting nonce ownership
2. INSERT OR IGNORE: Correctly preserves the first claimant
3. rowcount check: Returns False when nonce already exists
4. Warning log: Alerts when duplicate nonce is detected
5. Expired nonce cleanup: Added before insert to free expired slots
6. Test coverage: 53-line test verifying first-owner preservation

Issues Found

1. Low — cleanup_expired_nonces() could be expensive
Called on every nonce insert. If the table is large, this could add latency. Consider periodic cleanup instead:

# Only cleanup every 100th insert
if random.random() < 0.01:
    cleanup_expired_nonces(conn, now_ts)

2. Nit — Return value not checked by callers
store_used_cross_node_nonce() now returns False for duplicates, but callers may not check. Consider making the return value contract explicit in the docstring.

Verdict: Approve

Important security fix for the nonce-miner binding invariant.

[jaxint]

Great contribution! LGTM.

[saim256]

Approved after focused validation. The change preserves the first active nonce owner by cleaning expired rows before insert, using INSERT OR IGNORE for active duplicates, and returning False when a replay tries to replace the existing owner. The added tests cover both the active-owner invariant and allowed reuse after TTL expiry.\n\nChecks run locally on the PR branch:\n- PYTHONPATH=bounties/issue-2296/src python -m pytest bounties/issue-2296/tests/test_cross_node_replay_defense.py -q -> 35 passed\n- python -m py_compile bounties/issue-2296/src/cross_node_replay_defense.py bounties/issue-2296/tests/test_cross_node_replay_defense.py -> passed\n- git diff --check origin/main...HEAD -- bounties/issue-2296/src/cross_node_replay_defense.py bounties/issue-2296/tests/test_cross_node_replay_defense.py -> passed\n- python tools\bcos_spdx_check.py --base-ref origin/main -> BCOS SPDX check: OK