-
Notifications
You must be signed in to change notification settings - Fork 0
Add cryptographically secure random number generator option #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: hugefiver <18693500+hugefiver@users.noreply.github.com>
Co-authored-by: hugefiver <18693500+hugefiver@users.noreply.github.com>
Co-authored-by: hugefiver <18693500+hugefiver@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
randstr/config.rkt
Outdated
| ;; Calculate the number of bytes needed to represent values up to n | ||
| ;; Uses ceiling to ensure we have enough bytes for the full range | ||
| (define (bytes-needed-for n) | ||
| (max MIN-BYTES (ceiling (/ (+ 1 (integer-length n)) BITS-PER-BYTE)))) |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function bytes-needed-for uses ceiling which returns an inexact number when the input is inexact. However, the result needs to be an exact integer for use in crypto-random-bytes. This could cause a contract violation if (integer-length n) or the division produces an inexact result.
Consider using inexact->exact after ceiling or ensuring the entire calculation uses exact arithmetic:
(max MIN-BYTES (inexact->exact (ceiling (/ (+ 1 (integer-length n)) BITS-PER-BYTE))))| (max MIN-BYTES (ceiling (/ (+ 1 (integer-length n)) BITS-PER-BYTE)))) | |
| (max MIN-BYTES (inexact->exact (ceiling (/ (+ 1 (integer-length n)) BITS-PER-BYTE))))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applied in commit 64b605a. Added inexact->exact wrapper to ensure the function returns an exact integer for crypto-random-bytes.
| [val (for/fold ([acc 0]) ([i (in-range byte-count)]) | ||
| (+ (bytes-ref bytes i) (* acc VALUES-PER-BYTE)))]) |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The byte-to-integer conversion accumulates bytes in little-endian order (least significant byte first), but the comment doesn't clarify this. The order doesn't affect randomness, but for code clarity and potential debugging, consider adding a comment about the byte order being used, or using a more explicit loop structure that makes the order clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applied in commit 64b605a. Added comment clarifying big-endian byte order in the conversion.
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: hugefiver <18693500+hugefiver@users.noreply.github.com>
Done. Applied both changes from the review in commit 64b605a. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
randstr-secure-random?parameter inconfig.rktto enable/disable cryptographically secure randomrandstr-randomhelper function inconfig.rktthat usescrypto-random-byteswhen secure mode is onchar-classes.rktto use the newrandstr-randomfunctiongenerator.rktto use the newrandstr-randomfunction-s/--secureto enable secure random mode2^64,BITS-PER-BYTE,VALUES-PER-BYTE,MIN-BYTES)bytes-needed-forinexact->exactto ensurebytes-needed-forreturns exact integerOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.