FIX(#4186): Replace predictable PRNG with crypto-safe secrets in PoA validator selection#4187
Conversation
…roper coin selection Two fixes in utxo_ledger.py: 1. **Broken rollback causes fund destruction**: When apply_transaction() fails during output creation, the except block returned False but did NOT restore the already-spent boxes. This permanently destroyed funds. Fix: restore spent_boxes to _boxes and _by_address, remove from _spent. 2. **BalanceTracker.transfer() uses all UTXOs**: The transfer method consumed every UTXO box as input, which was inefficient, privacy-leaking, and created unnecessarily large transactions. Fix: greedy coin selection (smallest-first) to minimize inputs. Note: node/utxo_db.py already has correct rollback via SQLite transactions. This fix brings the in-memory ledger to the same safety standard. Wallet: RTC9d7caca3039130d3b26d41f7343d8f4ef4592360
…safe secrets module in PoA validator selection - random.choice() → secrets.randbelow() for zero-score fallback - random.uniform() → secrets.randbelow(int(total_as * 1M)) / 1M for weighted lottery - import secrets added - Mersenne Twister is reconstructable from 624 outputs, enabling validator prediction attacks
cerredz
left a comment
There was a problem hiding this comment.
Requesting changes mainly for scope hygiene. The PoA CSPRNG change is the right direction, but this PR also modifies rips/rustchain-core/ledger/utxo_ledger.py with rollback and coin-selection changes for #4182. That is unrelated to #4186 and duplicates the dedicated UTXO PR #4183, so it should be removed from this branch before merge.
Keeping this PR to only rips/rustchain-core/consensus/poa.py will make the security claim reviewable and avoid coupling a validator-selection fix to separate UTXO ledger behavior.
One smaller PoA edge case to cover while you respin: secrets.randbelow(int(total_as * PRECISION)) raises ValueError if total_as is positive but less than one precision unit. That may not happen with normal antiquity scores, but the old random.uniform(0, total_as) handled any positive float, so a focused regression test around very small positive weights would make the replacement safer. A max(1, int(...)) style guard, or an integer-weight conversion that documents minimum precision, would avoid that sharp edge.
Validation I ran:
python -m py_compile rips\\rustchain-core\\consensus\\poa.py rips\\rustchain-core\\ledger\\utxo_ledger.py=> passedgit diff --check origin/main...HEAD=> passed
Suggested next step: strip utxo_ledger.py from this PR, add a small unit test for select_validator() using patched secrets.randbelow(), and keep the #4182 ledger work in #4183.
Fix for Bug #4186: Predictable Validator Selection
Wallet:
RTC9d7caca3039130d3b26d41f7343d8f4ef4592360Problem
The
select_validator()function inrips/rustchain-core/consensus/poa.pyuses Python'srandom.uniform()andrandom.choice()for weighted lottery selection of block validators. Therandommodule uses Mersenne Twister PRNG, which is reconstructable from 624 consecutive outputs — allowing attackers to predict future validator selections.Changes
import secrets(line 19)random.choice(proofs)→proofs[secrets.randbelow(len(proofs))]— crypto-safe fallback for zero-score case (line 191)random.uniform(0, total_as)→secrets.randbelow(int(total_as * 1_000_000)) / 1_000_000.0— crypto-safe weighted selection (line 194)Why
secrets?secretsmodule usesos.urandom()— OS-level CSPRNGVerification
Impact
Closes #4186