perf(crypto): enable configured SHA256 acceleration - #7573
Conversation
Walkthrough
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
|
✅ Final review complete — no blockers (commit 5881f94) |
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex/Sol only (Phase 2 disabled)
The exact-head diff is a focused four-line change that restores direct access to the generated SHA-256 feature macros, with optimized implementations conditionally built and linked under the same configuration flags. The guarded include exactly matches the relevant upstream bitcoin#29404 hunk, preserves DISABLE_OPTIMIZED_SHA256 behavior, and introduces no actionable defects.
Source: reviewer backends gpt-5.6-sol (general, dash-core-commit-history, and backport-reviewer); final verifier backend gpt-5.6-sol. openclaw-agent/cliproxy/gpt-5.6-sol was orchestration-only and is not reviewer evidence.
Validated zero-blocker Codex/Sol precheck evidence was promoted to final because Phase 2 (Sonnet/Opus) is temporarily disabled. This is Codex/Sol-only final validation, not Codex + Sonnet/Opus coverage.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— dash-core-commit-history (completed),gpt-5.6-sol— backport-reviewer (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet/Opus: not run (Phase 2 disabled — temporary Codex/Sol-only final)
- Secondary pass: disabled (
temporary_phase2_sonnet_disable)
Issue being fixed or feature implemented
Dash Core's SHA-256 runtime dispatcher did not include the generated configuration header. As a result, it could not see configured
ENABLE_*feature macros even when the corresponding optimized implementations were built and linked.On ARM64 this left
CSHA256using the portable implementation instead of the configured ARMv8 SHA2 implementation. On supported x86 builds it also prevented the dispatcher from selecting configured SHA-NI and multi-way implementations, although the existing single-way SSE4 fallback remained available.Regression history
The optimized SHA-256 implementations were inherited from Bitcoin Core and originally worked in Dash because
crypto/sha256.cppreceived the generated configuration macros transitively throughcrypto/common.h.Bitcoin Core removed that fragile transitive dependency in bitcoin/bitcoin#29404, merged on February 20, 2024. That PR added explicit
bitcoin-config.hincludes to translation units that consume configuration macros, including the exact include added here tocrypto/sha256.cpp.Bitcoin subsequently merged bitcoin/bitcoin#29263 on March 1, 2024, which removed
bitcoin-config.hfromcrypto/common.h. This was safe upstream because bitcoin#29404 had already made the direct dependencies explicit.Dash backported bitcoin#29263 in dashpay/dash#6378, merged on February 21, 2025, without first backporting bitcoin#29404. This removed the only path by which
crypto/sha256.cppsawENABLE_SSE41,ENABLE_AVX2,ENABLE_X86_SHANI, andENABLE_ARM_SHANI, silently disabling those configured runtime selections while preserving correct portable SHA-256 behavior.The regression is present in Dash Core releases v23.0.0 through v23.1.8. It is therefore a Dash backport dependency gap rather than a defect Bitcoin shipped.
dashpay/dash#7124 is an open, broader backport of Bitcoin bitcoin#29404 and contains the same
sha256.cpphunk among many configuration-include cleanups. This PR intentionally isolates the performance-critical SHA-256 fix so it can land independently.What was done?
Include
config/bitcoin-config.hfromcrypto/sha256.cppwhenHAVE_CONFIG_His defined, restoring the exact upstreamsha256.cppchange from Bitcoin bitcoin#29404.Runtime feature detection and the existing SHA-256 self-test remain responsible for selecting and validating the implementation. Builds using
DISABLE_OPTIMIZED_SHA256, includinglibdashconsensus, remain unaffected.How Has This Been Tested?
Tested on Apple Silicon using the depends-based build:
makebuildmake checkstandardtoarm_shani(1way,2way)The end-to-end comparison held the testnet data, rust-dashcore revision, proxy configuration, and other Dash Core changes constant; only the generated-config include differed between the two measured builds.
Breaking Changes
None.
Checklist:
This pull request was created by Codex.