Repository navigation
pow%refac: flesh out util:memops, magic blob reductions, move derivations to const expressions - #91
Conversation
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title names three changes: fleshing out util:memops, magic blob reductions, and moving derivations to const expressions. The summary supports all three. The memops helpers are added and remove… |
| Description check | |
The description says the change depends on another PR, lists no breaking changes, and says the code was tested with just comb. It also discloses AI assistance and gives a checklist. These statements… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Title check
-
Explanation
The title names three changes: fleshing out
util:memops, magic blob reductions, and moving derivations toconstexpressions. The summary supports all three. The memops helpers are added and removed, the constant tables and IVs are now derived in const contexts, and the blob-style tables are reduced. The title is long and lists several topics, but it still relates to the main changes. Theutil:memopstypo is minor.
Full details: Description check
-
Explanation
The description says the change depends on another PR, lists no breaking changes, and says the code was tested with
just comb. It also discloses AI assistance and gives a checklist. These statements relate to a refactor of the pow crate, so the description is related to the changeset. It does not explain the changes in detail, which the lenient check does not require.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
kwvg
changed the title
util:memops, magic blob reductions, move derivation to const expressionsutil:memops, magic blob reductions, move derivations to const expressions
|
✅ Final review complete — no blockers (commit d49f11d) · triage: critical |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
The refactor preserves the reviewed Blake, CubeHash, JH, Luffa, Shavite, Skein, and Echo transformations based on direct source comparison and the existing expected-value coverage. One non-blocking test-coverage gap was introduced: the focused assertion that rounds 10–15 repeat rows 0–5 was removed, while the remaining permutation test cannot detect an incorrect repeat schedule. CI checks for this head were still queued.
🟡 1 suggestion(s)
1 finding(s) not shown inline (the lines are not part of this PR's diff)
🟡 Suggestion: Pin the derived Blake sigma repeat schedule
pkgs/pow/src/consts/blake.rs:55-68
The new SIGMA table is generated with SIGMA_ROWS[r % 10], but the previous test that explicitly asserted rounds 10–15 repeat rounds 0–5 was removed. sigma_is_permutation only verifies that each row is a permutation, so it would still pass if the repeated rows were shifted or otherwise selected incorrectly. Retain a direct assertion tying every generated row to SIGMA_ROWS[r % 10] so this const derivation is independently pinned.
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn sigma_rows_repeat_after_ten() {
for r in 0..SIGMA.len() {
assert_eq!(SIGMA[r], SIGMA_ROWS[r % SIGMA_ROWS.len()]);
}
}
#[test]
fn sigma_is_permutation() {
for row in &SIGMA {
let mut sorted = *row;
sorted.sort();
let expected: [usize; 16] = core::array::from_fn(|i| i);
assert_eq!(sorted, expected);
}
}
}
source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 5: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 6: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — This is a large, intricate refactor that directly changes multiple cryptographic proof-of-work hash implementations and their constant derivations, including low-level memory operations in files such as pkgs/pow/src/consts/jh.rs and pkgs/pow/src/util/memops.rs. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity quota lookup failed),glm-5.3-flash(not used above high effort; tier asks max) - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer - Model comparison: every Phase-2 reviewer also ran on
gpt-6-astra; the verifier weighed both sets without knowing which model wrote which
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `pkgs/pow/src/consts/blake.rs`:
- [SUGGESTION] pkgs/pow/src/consts/blake.rs:55-68: Pin the derived Blake sigma repeat schedule
The new `SIGMA` table is generated with `SIGMA_ROWS[r % 10]`, but the previous test that explicitly asserted rounds 10–15 repeat rounds 0–5 was removed. `sigma_is_permutation` only verifies that each row is a permutation, so it would still pass if the repeated rows were shifted or otherwise selected incorrectly. Retain a direct assertion tying every generated row to `SIGMA_ROWS[r % 10]` so this const derivation is independently pinned.
|
Note This pull request has no conflicts! 🎊 🎉 🎊 |
github-actions
Bot
added
the
topic:Primitives
Additional Information
Breaking Changes
None expected
How Has This Been Tested?
Checklist
Agent disclosure
Code generated with Claude Code using Opus 5.5, medium effort. Reviewed with Codex using GPT-6-Astra, extra high effort as adversarial review and Claude Code with Fable 5.1, xhigh effort.