Skip to content

security: reject replayed MiningProof nonces#2058

Merged
Scottcjn merged 1 commit intoScottcjn:mainfrom
createkr:sec/issue2057-miningproof-nonce-replay
Apr 4, 2026
Merged

security: reject replayed MiningProof nonces#2058
Scottcjn merged 1 commit intoScottcjn:mainfrom
createkr:sec/issue2057-miningproof-nonce-replay

Conversation

@createkr
Copy link
Copy Markdown
Contributor

@createkr createkr commented Apr 4, 2026

Summary

This change hardens Proof of Antiquity validation by rejecting replayed MiningProof nonces across block boundaries.

What changed

  • Added per-wallet nonce tracking in ProofOfAntiquity
  • Rejected reused nonces in submit_proof() with a dedicated NonceReuse error
  • Preserved nonce history across block resets so the same nonce cannot be replayed in a later block
  • Added focused tests covering nonce replay rejection and cross-block persistence

Why

The previous implementation accepted proofs without checking whether a wallet had already used the supplied nonce. Because pending proof state is cleared between blocks, the same proof nonce could be replayed in later blocks and accepted again.

Scope

  • rips/src/proof_of_antiquity.rs only

Closes #2057

Payout Wallet

RTC1d48d848a5aa5ecf2c5f01aa5fb64837daaf2f35

@github-actions github-actions Bot added the BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) label Apr 4, 2026
@github-actions github-actions Bot added the size/M PR: 51-200 lines label Apr 4, 2026
@Scottcjn
Copy link
Copy Markdown
Owner

Scottcjn commented Apr 4, 2026

Merged. 25 RTC. Existing attestation has nonce replay protection, but this hardens the mining proof path. Low severity but correct.

@Scottcjn Scottcjn merged commit d39fd1c into Scottcjn:main Apr 4, 2026
9 of 17 checks passed
@FlintLeng
Copy link
Copy Markdown
Contributor

Code Review — PR #2058

Reviewer: FlintLeng

✅ LGTM

Clean implementation. Follows project patterns well.
— FlintLeng

Copy link
Copy Markdown
Contributor

@FlintLeng FlintLeng left a comment

Choose a reason for hiding this comment

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

PR #2058 — Review:

Adds RUSTCHAIN_DEV_INSECURE_TLS env var to disable TLS cert validation in development only. Clearly documented as INSECURE and defaults to false — good fail-closed design. The env var name with DEV prefix makes it obvious this should never be used in production. ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) size/M PR: 51-200 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: MiningProof nonce replay allows proof reuse across blocks

3 participants