Repository navigation
pow%refac!: unify AES backends, drop scalar backends and switch to wide, inline KAT tests - #66
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| 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. |
| Description check | |
The description directly explains the stable-toolchain motivation, removal of scalar implementations, SIMD backend changes, testing plans, and related dependency. |
| Title check | |
The title clearly summarizes the main changes: AES backend unification, scalar backend removal, migration to wide, and inline KAT tests. |
- 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.
|
Note This pull request has no conflicts! 🎊 🎉 🎊 |
github-actions
Bot
added
the
topic:Crypto
5 of 6 tasks
`git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space`
kwvg
force-pushed
the
pow_p0
branch
2 times, most recently
from
October 17, 2026 18:44
e7789a9 to
e659d41
Compare
kwvg
marked this pull request as ready for review
October 18, 2026 06:09
5 of 6 tasks
kwvg
added a commit
to kwvg/base-sdk
that referenced
this pull request
Oct 19, 2026
…mmit corpora, restore carryforward behaviour in CodeCov reporting, (dashpay#68) * [071af5f] sdk%fix(ci): carry forward coverage of every crate on codecov (Kittywhiskers Van Gogh) * [c6fde12] sdk%lint: list files through git, skip ignored paths and submodules (Kittywhiskers Van Gogh) * [5c2ca17] sdk%feat: create, list and destroy ramdisks with `manage_ramdisk.py` (Kittywhiskers Van Gogh) * [2a92b1d] pow%test(blake): fuzz against sphlib with AFL++, commit corpora (Kittywhiskers Van Gogh) * [ce81ec3] pow%test(bmw): fuzz against sphlib, commit corpora (Kittywhiskers Van Gogh) * [dc37811] pow%test(groestl): fuzz against sphlib, commit corpora (Kittywhiskers Van Gogh) * [c9e3aba] pow%test(skein): fuzz against sphlib, commit corpora (Kittywhiskers Van Gogh) * [93c6fda] pow%test(jh): fuzz against sphlib, commit corpora (Kittywhiskers Van Gogh) * [2341f17] pow%test(keccak): fuzz against sphlib, commit corpora (Kittywhiskers Van Gogh) * [785ef45] pow%test(luffa): fuzz against sphlib, commit corpora (Kittywhiskers Van Gogh) * [828ec84] pow%test(cubehash): fuzz against sphlib, commit corpora (Kittywhiskers Van Gogh) * [1f3c4cb] pow%test(shavite): fuzz against sphlib, commit corpora (Kittywhiskers Van Gogh) * [41150bb] pow%test(simd): fuzz against sphlib, commit corpora (Kittywhiskers Van Gogh) * [e7dd1f3] pow%test(echo): fuzz against sphlib, commit corpora (Kittywhiskers Van Gogh) * [e9e9368] pow%test: fuzz the X11 chain against sphlib, commit corpora (Kittywhiskers Van Gogh) * [eaa6bdf] sdk%test: replay the fuzz corpora in `just comb`, add to CI workflow (Kittywhiskers Van Gogh) Description: ````` ## Description In preparation for further work on `dash-pow`, it becomes necessary to establish a basic framework for testing the implementation's correctness. Agentic work on optimising the SIMD (now only) had yielded buggy implementations that still passed NIST's test cases and but caught by the tests written by the agents. This is not a viable testing methodology, so we use the underlying implementation used by Dash Core ([source](https://github.com/dashpay/dash/tree/v24.0.0-rc.3/src/crypto/x11)) as a truth oracle and run fuzzing campaigns against it to stress test the implementation. The initial corpora was generated locally through the contents of a four day campaign using AFL++ 0.18.2 and reduced using `afl-tmin`. ## Additional Information * Depends on dashpay#66 * Depends on dashpay#69 * This pull request includes a ramdisk management script `manage_ramdisk.py` for use in fuzzing to avoid trashing the device's SSD. On Linux, we use `noswap` to prevent contradicting the purpose of the ramdisk but **no such option is available on macOS**, `hdiutil attach -kernel` does not work on macOS 26. * This means, on macOS, callers are expected to size the ramdisk and moderate their system usage to reduce likelihood on swap (or use a Linux runner, which may end up being preferable in the long run). ## Breaking Changes None expected. ## How Has This Been Tested? ``` ./contrib/git_filter.py --fast-fail --env NEXTEST_PROFILE=fail-fast develop pow_p1~1 -- just test lint ./contrib/git_filter.py --fast-fail --env NEXTEST_PROFILE=fail-fast pow_p1~1 pow_p1 -- just test lint fuzz::test ``` ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional tests - [ ] I have made corresponding changes to the documentation - [x] I did not use agentic assistance (i.e. LLMs, AI) except as [disclosed](#agent-disclosure) and can answer questions, comments and concerns without said assistance. I assume authorship of the code submitted. - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ### 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 using Fable 5.1, xhigh effort. `````
3 of 6 tasks
kwvg
added a commit
that referenced
this pull request
Oct 20, 2026
…ds, fuse fuzz corpora validation with MSRV, CodeCov reset take two (#78) * [82facb4] sdk%ci: remove CodeCov upload from workspace build (Kittywhiskers Van Gogh) * [0003285] sdk%ci: swap MSRV to per-crate builds and stable to workspace builds (Kittywhiskers Van Gogh) * [c6877ec] sdk%fix(ci): restore CodeCov upload to per-crate build (Kittywhiskers Van Gogh) * [db4a8c4] sdk%refac(ci): fuse fuzz corpora replay into per-crate MSRV build (Kittywhiskers Van Gogh) * [d2cc23e] sdk%fix(ci): repair broken CodeCov stats (again) with hard reset (2/2) (Kittywhiskers Van Gogh) Description: ````` ## Additional Information * [base-sdk#66](#66) in removal of the pinned nightly task moved CodeCov uploads to MSRV, responsible for workspace-wide builds instead of stable, responsible for per-crate builds. This corrupted the per-crate coverage statistics. * In combination with the muddied water by the attempted CodeCov cleanup in [base-sdk#55](#55) and partial rollback in [base-sdk#66](#68), full rollback to the old configuration in [base-sdk#76](#76) only after a full reset done by two pushes to `develop` directly, 51bf7f8 and ad340e5, was the cause of the problem identified to be the above change. * This pull request also includes re-enablement of the carryforward flag to complete a reset, completing 1101702 ## Checklist - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional tests - [ ] I have made corresponding changes to the documentation - [x] I did not use agentic assistance (i.e. LLMs, AI) except as [disclosed](#agent-disclosure) and can answer questions, comments and concerns without said assistance. I assume authorship of the code submitted. - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ### Agent disclosure Code generated with Claude Code using Opus 5.5, medium effort. `````
kwvg
added a commit
that referenced
this pull request
Oct 21, 2026
…ith the reference implementation (#67) * [59981d4] p2p_core%fix!: make `ProtocolVersion` signed (pasta) * [e12661a] p2p_core%refac!: carry the decode error in `P2pDecodeError` (pasta) * [8ca2966] p2p_core%fix!: bound every command by the 3 MiB message limit (pasta) * [3413681] p2p_core%fix!: bound V2 contents before reading the type (pasta) * [49ef62a] p2p_core%fix!: take V2 message types as the reference transport does (pasta) * [8cfb0a3] p2p_core%fix!: refuse commands the reference V1 transport refuses (pasta) * [8626fe1] p2p_core%feat!: name every inventory type and map it to its command (pasta) * [a222647] p2p_core%fix: accept up to 8000 headers in `headers2` (pasta) * [b04e311] p2p_core%fix: bound header counts by the remaining input (pasta) * [cfbc119] p2p_core%feat: add `Headers2::max_count` for the negotiated limit (pasta) * [606d44d] p2p_core%refac: decode the V2 message type through `BaseCodec` (pasta) * [06c6803] p2p_core%fix!: check commands against the V2 rule only (pasta) * [07a739e] p2p_core%test: check the V2 transport vectors from `tests` (pasta) * [ebc5c06] p2p_core%feat: export `MAX_P2P_PAYLOAD_SIZE` and link limits in docs (pasta) * [46e4704] p2p_core%refac: suffix size constants with `_SIZE` (pasta) * [8b43b25] p2p_core%refac: derive the V2 short ID table counts from the messages (pasta) * [ab11ad1] p2p_core%test: decode a full batch of minimally compressed headers (pasta) Description: ````` ## Motivation `dash-p2p-core` diverges from the reference implementation (`dashpay/dash` [`v24.0.0-rc.3`](https://github.com/dashpay/dash/tree/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18)) in how it frames and bounds P2P messages, which inventory types it knows, and how many headers it accepts. In particular: - an oversized message is not refused whatever its command; - invalid commands are not told apart from unknown ones; - some V2 short IDs the reference implementation takes are refused; - `headers2` is capped at 2000 headers instead of 8000. This pull request aligns them. Where the behaviour has a library entry point in the reference implementation, the claim comes with a corpus section generated by running a program against it at the pin above. Each program is embedded in the section it generated. Where the behaviour lives only in the reference implementation's message handler, the claim is cited. It supersedes #59, #64 and #65, which are reworked here into one series. The `dash-primitives` transaction check changes that this pull request carried before, which superseded #63, now have their own pull request. ## Additional Information | Description | Notes | Reference implementation | Corpus | | ----------- | ----- | ------------------------ | ------ | | Read the protocol version as signed | A version with the top bit set reads below every minimum. | [read as `int`](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/net_processing.cpp#L3945-L3950), [minimum check](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/net_processing.cpp#L3970), [minimum](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/version.h#L20) | - | | Bound every command by the message limit | `P2pMsg::decode_payload` applies `MAX_P2P_PAYLOAD_SIZE` to unknown commands too, as the reference implementation bounds a message before it looks at its type. | [limit](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/net.h#L84) | - | | Bound V2 contents before reading the type | Contents above 1 + 12 bytes + `MAX_P2P_PAYLOAD_SIZE` disconnect whatever the type. A long-form type is printable ASCII (`0x20` to `0x7f`) up to the first NUL and then only NULs. Otherwise the message is rejected without disconnecting. | [bound](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/net.cpp#L1297-L1307), [type](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/net.cpp#L1496-L1534), [reject](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/net.cpp#L1549-L1559) | `transport.json5` `v2` | | Take V2 message types as the reference transport does | Contents that end before the type is complete are rejected. Short IDs 1 to 32 and 128 to 168 are taken whether a message is assigned to them or not. An unassigned one gives an empty type, which is ignored. Every other short ID is rejected. The table counts are derived from the messages and the new `unassigned` slots of `define_p2p!`. | [empty contents](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/net.cpp#L1498), [short long form](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/net.cpp#L1512-L1514), [short ID ranges](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/net.cpp#L1028-L1035), [tables](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/net.cpp#L892-L977) | `transport.json5` `v2`, `tests/decode_v2.rs` | | Refuse invalid commands | `P2pMsg::decode_payload` refuses a command that a V2 transport refuses as a long-form type. `p2p_core` only speaks V2, so V1 rules are not evaluated. | [type](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/net.cpp#L1496-L1534) | `tests/decode_payload.rs` | | Name every inventory type | - | [types](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/protocol.h#L495-L528) | - | | Map inventory types to commands, and tell transaction and block types apart | Asset unlocks are announced by their own type but carried in `tx` messages. | [commands](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/protocol.cpp#L281-L309), [predicates](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/protocol.h#L555-L562) | `inv.json5` `inv_type` | | Accept up to 8000 headers in `headers2` | `headers` stays at 2000. | [limits](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/validation.h#L82-L83), [count check](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/net_processing.cpp#L5323-L5327) | - | | Pick the `headers2` limit from the negotiated version | `Headers2::MAX_COUNT` only from protocol version 70235, otherwise `Headers::MAX_COUNT`. | [selection](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/net_processing.cpp#L1210-L1216), [version](https://github.com/dashpay/dash/blob/0364ed626ec2eaa518bf1ae4a0566ce638c7fe18/src/version.h#L50) | - | Header counts are also bounded by the remaining input before anything is allocated. A count the input cannot hold now reports `DecodeError::Eof`. The reference implementation fails the same read. The command in a `PayloadTooLarge` or `InvalidCommand` message is escaped, as it is for `UnknownCommand`, since it can come from any peer. The last six commits answer the review: the V2 type is read through `BaseCodec`, commands are checked against the V2 rule only, the V2 transport vectors are checked from `tests`, `MAX_P2P_PAYLOAD_SIZE` is exported so docs can link it instead of quoting its value, size constants carry a `_SIZE` suffix, and the short ID table counts are derived from the messages. The branch is rebased onto `develop` after #70 and #71. It needed no conflict resolution: #71 does not touch the code this pull request changes, and the header count reads keep their own bound, since each entry takes more than the one byte `CompactSize::decode_len` assumes. ## Breaking Changes Changes below for `dash-p2p-core`. - `ProtocolVersion` wraps an `i32` instead of a `u32`. - `P2pDecodeError::Consensus` carries the `DecodeError` instead of its message. - `P2pDecodeError::PayloadTooLarge::command` is a `CommandString`, and its message escapes the command. Every command, known or not, is bounded by `MAX_P2P_PAYLOAD_SIZE`. - `decode_v2` refuses contents above 1 + 12 + `MAX_P2P_PAYLOAD_SIZE` bytes with `PayloadTooLarge`. It reports contents that end before the type as the new `P2pDecodeError::MissingCommand`, and an invalid long-form type as the new `P2pDecodeError::InvalidCommand`. An unassigned short ID within the reference implementation's tables decodes as an empty command, giving `UnknownCommand`. - `P2pMsg::decode_payload` reports a command that fails `CommandString::is_valid_v2` as `InvalidCommand` instead of `UnknownCommand`. - `InvType` has a variant for every inventory type the reference implementation defines. - Decoding `Headers2` accepts up to 8000 headers. Decoding `Headers` and `Headers2` reports a count the remaining input cannot hold as `DecodeError::Eof`. - `MAX_P2P_PAYLOAD_SIZE` (now exported at the crate root), `CommandString::is_valid_v2`, `ShortId::is_in_table`, `InvType::command`, `InvType::is_tx`, `InvType::is_block`, `Headers::MAX_COUNT`, `Headers2::MAX_COUNT`, `Headers2::max_count` and `ProtocolVersion::INCREASE_MAX_HEADERS2` are new. ## How Has This Been Tested? ``` just comb ``` `just comb` (`just test lint fuzz::test`, CodeQL 2.27.2 included) passed on every commit, rebased on `develop` (f2fdb28), on x86_64 Linux. Since #66, `dash-pow` builds without warnings there, so no local patch was needed. The corpus programs were run against the reference implementation at the pin, and the sections hold their output. Each section says how to run its program. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional tests - [x] I have made corresponding changes to the documentation - [ ] I did not use agentic assistance (i.e. LLMs, AI) except as [disclosed](#agent-disclosure) and can answer questions, comments and concerns without said assistance. I assume authorship of the code submitted. - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ### Agent disclosure Code, tests, corpus programs, commit messages and this description were generated with Claude Code using Claude Opus 5.5, running autonomously as an agent on behalf of the author. The agent also ran the corpus programs against the reference implementation, answered the review in follow-up commits, and split and rebased the branch. The branch was reviewed by Claude Code agents, using Opus 5.5 for two review rounds and Sonnet 5.5 for a simplification pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) `````
3 of 5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
A blocker for adopting our hashing crate is the hard dependency in
core::simd, that has not stabilised yet and would regardless force an MSRV bump beyond that tolerated by downstream consumers. The non-viability of downstream consumers switching tonightlyfor the sake of one crate (more on this has been elaborated in base-sdk#60) has required us to commit to astable-first toolchain.On top of that, as a workaround for the
nightlyrestriction (and due to initial uncertainty surrounding the SIMD implementation), scalar implementations were also supplied as a baseline while iterating on various SIMD strategies. They have now outlived their purpose and can be retired in preparation for an initial release of the crate.In its place, we will instead run fuzzing against
pornin/sphlibto ensure that future iterations on the SIMD implementation do introduce behaviours that cause unexpected digest outputs, though this work has been reserved for a future pull request, ideally before future optimisation work is undertaken.Additional Information
portablefeature, define system requirements, decoupleserdefromcodec, usethiserror, tune build flags, addmutantconfig,gitmerge script #60Checklist
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 using Fable 5.1, xhigh effort.