Skip to content

Remove weak helper randomness#74

Merged
punk6529 merged 1 commit into
mainfrom
codex/remove-weak-helper-randomness
Jun 10, 2026
Merged

Remove weak helper randomness#74
punk6529 merged 1 commit into
mainfrom
codex/remove-weak-helper-randomness

Conversation

@punk6529

@punk6529 punk6529 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • remove the concrete XRandoms block-derived helper from production source
  • keep IXRandoms and the inline test mock for the legacy RandomizerNXT boundary, which remains non-production-configurable
  • refresh ADR, roadmap, Slither baseline, status, known-blocker, test, and autonomous-run traceability for weak-prng=0

Closes #73.

Validation

  • forge test --match-test testNxtRandomizerCannotBeConfiguredForProductionCollections -vvv
  • make check
  • powershell -ExecutionPolicy Bypass -File scripts\check.ps1
  • forge fmt --check smart-contracts\IXRandoms.sol smart-contracts\RandomizerNXT.sol test\StreamRandomizerLifecycle.t.sol
  • git diff --check
  • rg -n "^#|^##|^###" ops\ROADMAP.md ops\SLITHER_BASELINE.md ops\AUTONOMOUS_RUN.md docs\adr\0005-randomness.md docs\known-blockers.md docs\status.md test\README.md
  • Slither JSON summary: {"arbitrary_send_eth":0,"high":4,"informational":575,"low":63,"medium":28,"optimization":6,"reentrancy_eth":0,"slither_exit":-1,"total":676,"uninitialized_state":0,"weak_prng":0}

Slither still exits nonzero because the accepted baseline has remaining non-weak-prng findings.

Summary by CodeRabbit

  • Documentation

    • Updated architecture decision records, status pages, and roadmaps to reflect recent changes.
    • Refined randomness configuration documentation and blockers.
    • Updated Slither baseline findings and scan results.
  • Tests

    • Adjusted test coverage for randomness helpers and mock implementations.
  • Removals

    • Production randomness helper component removed from source.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7ede04b5-3445-4c73-901f-e0a0fd3be867

📥 Commits

Reviewing files that changed from the base of the PR and between ba2f0cd and 4ce6054.

📒 Files selected for processing (9)
  • docs/adr/0005-randomness.md
  • docs/known-blockers.md
  • docs/status.md
  • ops/AUTONOMOUS_RUN.md
  • ops/ROADMAP.md
  • ops/SLITHER_BASELINE.md
  • smart-contracts/IXRandoms.sol
  • smart-contracts/XRandoms.sol
  • test/README.md
💤 Files with no reviewable changes (1)
  • smart-contracts/XRandoms.sol

📝 Walkthrough

Walkthrough

This PR removes the concrete XRandoms weak-prng production contract and updates architecture, baseline, and roadmap documentation to confirm zero weak-prng findings and finalize production-scope boundaries for RandomizerNXT. The deleted contract is replaced with test-only mocks where needed for legacy boundary coverage.

Changes

Weak randomness helper removal

Layer / File(s) Summary
Contract removal and interface cleanup
smart-contracts/IXRandoms.sol, smart-contracts/XRandoms.sol
XRandoms.sol deleted from production; IXRandoms interface retained for test-only use with minor formatting adjustments.
ADR 0005 architecture update
docs/adr/0005-randomness.md
Metadata table revised to remove XRandoms.sol and add IRandomizer.sol/IArrngController.sol; narrative clarified to document XRandoms removal in P0-RAND-008, confirm weak-prng=0, remove final weak-helper handling from remaining work, and mark completion in follow-ups.
Slither baseline updated to weak-prng=0
ops/SLITHER_BASELINE.md
Timestamp, impact counts, and detector counts refreshed; weak-prng findings reduced to zero; randomNumber() and randomWord() rows marked Fixed with P0-RAND-008 resolution; informational delta adjusted.
Roadmap refined with completion evidence
ops/ROADMAP.md
Current status timestamp advanced; P0-RAND-008 task documented with concrete removal outcome; RandomizerNXT confirmed non-production-eligible; baseline appendices, test matrix, and enforcement statements updated to reflect zero weak-prng and production-scope boundary.
Status and blocker documentation clarified
docs/status.md, docs/known-blockers.md, test/README.md
Gate A baseline clarified that RandomizerNXT cannot be configured as production randomizer after XRandoms removal; blockers refined to exclude weak-helper handling; Slither scope clarified (vendored/test-only findings remain); test README notes production helper removal with inline mock retention.
Autonomous run worklog and queue update
ops/AUTONOMOUS_RUN.md
Repository state updated with active PR branch and merged PR #72 details; PR Queue advanced (Item 31 merged, Item 32 ready); PR #72 worklog marked merged; PR #73 full scope, validation plan, and Slither evidence added; Decision Log extended with merge and progression timeline.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • 6529-Collections/6529Stream#44: Prior work updating the randomness ADR and Slither baseline to target weak-prng rows for randomNumber()/randomWord(); this PR executes the completion plan by removing smart-contracts/XRandoms.sol and aligning all docs.
  • 6529-Collections/6529Stream#7: Established the Slither baseline documentation and workflow; this PR updates baseline artifacts to record weak-prng fixed to zero.

Poem

🐰 The weak randomness helper hops away,
Production source now clean as day,
XRandoms gone, but IXRandoms stays,
For test-only mocks and legacy ways,
Slither smiles: weak-prng counts zero! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Remove weak helper randomness' directly describes the main change: deletion of the XRandoms contract and removal of weak-prng findings from production source.
Linked Issues check ✅ Passed The PR fully addresses issue #73 objectives: XRandoms.sol deleted, IXRandoms interface retained, RandomizerNXT kept non-production-configurable, documentation updated, and weak-prng detector now reports zero for first-party source.
Out of Scope Changes check ✅ Passed All changes (contract removal, interface retention, documentation updates, and Slither baseline refresh) align with the stated objectives of removing weak helper randomness and updating traceability.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/remove-weak-helper-randomness

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@punk6529 punk6529 merged commit 8ced3ef into main Jun 10, 2026
2 checks passed
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.

[P0-RAND-008] Remove weak XRandoms helper from production source

1 participant