perf(crypto): vectorized BLAKE3 / LtHash and cached did:key parsing #150

Grandiras · 2026-09-24T18:14:58Z

Grandiras commented

2026-09-24 18:14:58 +00:00

Closes #112.

Speeds up the CPU-bound crypto that permissioned-space syncing and signature verification spend their time in. The public API does not change, and every result is bit-identical to before.

What changed

BLAKE3 (`Crypto/Blake3.cs`, internal)

XOF output blocks differ only in their counter, so the output stage compresses 8 blocks at once with Vector256<uint>, or 4 with Vector128<uint> (for example on ARM64/NEON or x64 without AVX2), with a scalar path otherwise. The compression function is written once, generic over the lane type (ILanes<T> with static abstract members), so all three paths run the same rounds.
The state and message are held in locals, and the rounds are the spec's "round, then permute" loop. There are no tables and no Unsafe. Fixed-size buffers are [InlineArray] structs, so every index is bounds-checked. Zero heap allocations (the old code allocated about 800 B per hash).
All byte/word conversion is explicitly little-endian (BinaryPrimitives), so big-endian hosts give identical output without a special case. Multi-chunk inputs keep the reference chunk-tree construction, now allocation-free with a stack-held chaining-value stack.
Removed the unused Blake3.KeyLen.
An internal overload HashExtended(input, output, parallelism) pins the batch width (1/4/8) so tests can exercise every path on any host.

LtHash (`Spaces/LtHash.cs`)

BLAKE3 writes the XOF output straight into the lane span (MemoryMarshal.AsBytes), with a byte swap only on big-endian hosts. Lanes are added and subtracted with Vector<ushort> over MemoryMarshal.Cast, with a scalar tail and a scalar fallback. WriteState and the state constructor copy in bulk instead of per lane.

Signature verification (`Crypto/AtProtoCrypto.cs`, new `Crypto/DidKeyCache.cs`)

AtProtoCrypto.VerifySignature(didKey, …) goes through DidKeyCache.Shared, a bounded LRU (1,024 keys) keyed by the did:key string. A did:key is its own content, so entries never go stale. .NET does not document ECDsa instances as thread-safe, so each entry keeps a small pool of imported keys (at most ProcessorCount idle). A verification rents one key, and eviction disposes idle keys immediately and rented ones on return. Parsing happens outside the lock, and a malformed key throws without being cached.
Curve constants (p, a, b, n, n/2, and the square-root exponent (p+1)/4) are computed once in a private CurveInfo instead of re-allocating 5 arrays per Sign/Verify. IsLowS no longer allocates.
Base58 decodes into a stack span through a lookup table (it previously used List<byte> and IndexOf). The work is bounded by the destination size, so an arbitrarily long did:key costs no more than a short one.
AtProtoKey.Verify rejects anything but a 64-byte r || s signature explicitly (DER included). The result is the same as before, but it no longer relies on the platform to reject those.

Internal duplication

AtProtoKey.GetCompressedPublicKey and CompressPublicKey share CompressPoint. DecompressPoint takes the curve directly instead of re-deriving it from OIDs. The OID and multicodec literals live in one place. FromMultikey reuses the multibase decoding instead of repeating it.
The curve→JWS alg mapping is one internal extension, KeyCurve.JwsAlgorithm(), used by ServiceAuthGenerator and SpaceTokens.

Behaviour notes (no public API change)

A did:key/multikey whose key is not 33 bytes now throws FormatException, as the XML docs already said, instead of ArgumentException. The in-repo callers (SpaceTokens, SpaceCommit, ISpaceServiceAuthVerifier, FirehoseVerifier) catch both.
An undefined KeyCurve value throws ArgumentOutOfRangeException instead of silently being treated as K-256.
Both are listed under Fixed in the CHANGELOG. Neither is binary- or source-breaking.

Verification

Unit tests: 1781 passed, 0 failed, 5 skipped (the environment-gated tests). The full solution builds with 0 warnings, 0 errors.
The 27 official BLAKE3 vectors and all pinned LtHash vectors pass unchanged. Those LtHash vectors are 2048-byte XOF output from @noble/hashes, so they already cover the 8-way path.
New pinned vectors:
- atproto-interop-tests crypto/ (signature-fixtures.json, w3c_didkey_P256.json, w3c_didkey_K256.json), vendored unchanged from bluesky-social/atproto-interop-tests@056e574 (CC0; origin and license in tests/ATProtoNet.Tests/Crypto/TestData/README.md). The tests cover valid, high-S and DER-encoded signatures for both curves, through the cached VerifySignature, FromDidKey, and the legacy publicKeyMultibase form. The did:key vectors derive each published did:key from its private key, parse it back, and sign and verify with it.
- Long-output BLAKE3 (sha256 of 1000–4099-byte output for 0 B to 100 KiB inputs) from @noble/hashes 1.8.0, an independent implementation, checked on the 1-, 4- and 8-lane paths. The official vectors stop at 131 bytes, which is too short to reach the batched path.
Batched vs scalar: every input length 0…1024 with 2048 bytes of output (what LtHash asks for) on 4-way and 8-way against scalar, and every output length 0…1280.
LtHash Add/Remove checked lane for lane against the scalar construction, including elements past the 512-byte stack buffer and past one BLAKE3 chunk.
DidKeyCache: results match AtProtoKey.Verify (valid, tampered, wrong message), LRU capacity, malformed and Ed25519 did:keys throw without caching, and 8 threads verifying through a one-entry cache (constant eviction while keys are rented) all get correct answers.
K256_GenerateAndSign_IfSupported (renamed GenerateK256Key_SignAndVerify_RoundTripsThroughDidKey) now calls Assert.Skip where secp256k1 is missing instead of passing silently. It also checks low-S and the cached verify.
Also covered: off-curve x for both curves, x ≥ p, bad prefixes, wrong multikey lengths, both Y parities round-tripping through did:key, wrong-length signatures, known base58 values including leading zeros, and span-overflow.

Benchmarks

BenchmarkDotNet 0.14, ShortRun (3 warmup, 10 iterations, in-process), .NET 10.0.12, AMD Ryzen 5 7600X. "Before" is main at 7f672c0 and "after" is this branch, with the same harness.

Operation	Before	After	Speed-up
BLAKE3 XOF, 90 B input → 2048 B	12,109 ns / 800 B	762 ns / 0 B	15.9×
BLAKE3 32 B digest, 90 B input	1,025 ns / 800 B	264 ns / 0 B	3.9×
BLAKE3 32 B digest, 8 KiB input	47,930 ns / 4,048 B	14,273 ns / 0 B	3.4×
`LtHash.Add`	13,210 ns / 800 B	814 ns / 0 B	16.2×
`LtHash.Remove`	13,237 ns / 800 B	812 ns / 0 B	16.3×
`LtHash.Digest`	1,724 ns / 56 B	1,105 ns / 56 B	1.6×
`VerifySignature(did:key)` P-256	94,990 ns / 4,176 B	41,260 ns / 1,416 B	2.3×
`VerifySignature(did:key)` K-256	294,760 ns / 3,874 B	210,700 ns / 1,417 B	1.4×
`FromDidKey` P-256 (uncached by design)	42,330 ns / 2,480 B	41,990 ns / 1,336 B	–
`AtProtoKey.Sign` P-256	14,220 ns / 724 B	14,050 ns / 417 B	alloc −42 %

SpaceRepoCommit.FromIndex, 10,000 records (Stopwatch, mean of 20 runs, including building the element strings): 133 ms → 9–12 ms (runs of 8.6, 10.8 and 12.0 ms).
With AVX2 disabled (DOTNET_EnableAVX2=0, so the 4-way Vector128 path), the 2048-byte XOF takes 2,738 ns, 4.4× faster than before.
For reference, the performance audit's prototype measured 721 ns for the XOF and 772 ns for LtHash.Add. This production version is within about 5 % of it without Unsafe or a length-limited fast path.

Edits outside `Crypto/` and `Spaces/LtHash.cs`

src/ATProtoNet/Auth/ServiceAuthGenerator.cs and src/ATProtoNet/Spaces/SpaceTokens.cs: one line each, to use KeyCurve.JwsAlgorithm().
tests/ATProtoNet.Tests/ATProtoNet.Tests.csproj: one <None Include="Crypto/TestData/*.json" CopyToOutputDirectory="PreserveNewest" /> line, so the vendored fixtures reach the test output. This is the only way to ship them as JSON files.
docs/crypto.md: two sentences on the verification cache and the rejected signature forms.

Deferred

src/ATProtoNet.Server/Spaces/JsonWebKeyVerifier.cs still has the reverse mapping (JWK crv → curve and alg) and a K-256 OID literal. That file belongs to a parallel PR. It could use the same curve metadata once that lands.
FromDidKey/FromMultikey stay uncached, because they return a key the caller owns and disposes. Their cost is mostly the BigInteger square root and the platform import.
The per-lane carry into the high counter word is kept for correctness, but a span-sized output cannot reach it (at most 2^25 blocks).

🤖 Generated with Claude Code

Closes #112. Speeds up the CPU-bound crypto that permissioned-space syncing and signature verification spend their time in. The public API does not change, and every result is bit-identical to before. ## What changed ### BLAKE3 (`Crypto/Blake3.cs`, internal) - XOF output blocks differ only in their counter, so the output stage compresses **8 blocks at once with `Vector256<uint>`**, or 4 with `Vector128<uint>` (for example on ARM64/NEON or x64 without AVX2), with a scalar path otherwise. The compression function is written once, generic over the lane type (`ILanes<T>` with static abstract members), so all three paths run the same rounds. - The state and message are held in locals, and the rounds are the spec's "round, then permute" loop. There are no tables and no `Unsafe`. Fixed-size buffers are `[InlineArray]` structs, so every index is bounds-checked. **Zero heap allocations** (the old code allocated about 800 B per hash). - All byte/word conversion is explicitly little-endian (`BinaryPrimitives`), so big-endian hosts give identical output without a special case. Multi-chunk inputs keep the reference chunk-tree construction, now allocation-free with a stack-held chaining-value stack. - Removed the unused `Blake3.KeyLen`. - An internal overload `HashExtended(input, output, parallelism)` pins the batch width (1/4/8) so tests can exercise every path on any host. ### LtHash (`Spaces/LtHash.cs`) - BLAKE3 writes the XOF output straight into the lane span (`MemoryMarshal.AsBytes`), with a byte swap only on big-endian hosts. Lanes are added and subtracted with `Vector<ushort>` over `MemoryMarshal.Cast`, with a scalar tail and a scalar fallback. `WriteState` and the state constructor copy in bulk instead of per lane. ### Signature verification (`Crypto/AtProtoCrypto.cs`, new `Crypto/DidKeyCache.cs`) - `AtProtoCrypto.VerifySignature(didKey, …)` goes through `DidKeyCache.Shared`, a **bounded LRU (1,024 keys) keyed by the did:key string**. A did:key is its own content, so entries never go stale. `.NET` does not document `ECDsa` instances as thread-safe, so each entry keeps a small pool of imported keys (at most `ProcessorCount` idle). A verification rents one key, and eviction disposes idle keys immediately and rented ones on return. Parsing happens outside the lock, and a malformed key throws without being cached. - Curve constants (p, a, b, n, n/2, and the square-root exponent (p+1)/4) are computed once in a private `CurveInfo` instead of re-allocating 5 arrays per `Sign`/`Verify`. `IsLowS` no longer allocates. - Base58 decodes into a stack span through a lookup table (it previously used `List<byte>` and `IndexOf`). The work is bounded by the destination size, so an arbitrarily long did:key costs no more than a short one. - `AtProtoKey.Verify` rejects anything but a 64-byte `r || s` signature explicitly (DER included). The result is the same as before, but it no longer relies on the platform to reject those. ### Internal duplication - `AtProtoKey.GetCompressedPublicKey` and `CompressPublicKey` share `CompressPoint`. `DecompressPoint` takes the curve directly instead of re-deriving it from OIDs. The OID and multicodec literals live in one place. `FromMultikey` reuses the multibase decoding instead of repeating it. - The curve→JWS `alg` mapping is one internal extension, `KeyCurve.JwsAlgorithm()`, used by `ServiceAuthGenerator` and `SpaceTokens`. ## Behaviour notes (no public API change) - A `did:key`/multikey whose key is not 33 bytes now throws **`FormatException`**, as the XML docs already said, instead of `ArgumentException`. The in-repo callers (`SpaceTokens`, `SpaceCommit`, `ISpaceServiceAuthVerifier`, `FirehoseVerifier`) catch both. - An undefined `KeyCurve` value throws `ArgumentOutOfRangeException` instead of silently being treated as K-256. - Both are listed under `Fixed` in the CHANGELOG. Neither is binary- or source-breaking. ## Verification - Unit tests: **1781 passed, 0 failed, 5 skipped** (the environment-gated tests). The full solution builds with **0 warnings, 0 errors**. - **The 27 official BLAKE3 vectors and all pinned LtHash vectors pass unchanged.** Those LtHash vectors are 2048-byte XOF output from `@noble/hashes`, so they already cover the 8-way path. - New pinned vectors: - **atproto-interop-tests `crypto/`** (`signature-fixtures.json`, `w3c_didkey_P256.json`, `w3c_didkey_K256.json`), vendored unchanged from `bluesky-social/atproto-interop-tests@056e574` (CC0; origin and license in `tests/ATProtoNet.Tests/Crypto/TestData/README.md`). The tests cover valid, **high-S** and **DER-encoded** signatures for both curves, through the cached `VerifySignature`, `FromDidKey`, and the legacy `publicKeyMultibase` form. The did:key vectors derive each published did:key from its private key, parse it back, and sign and verify with it. - **Long-output BLAKE3** (sha256 of 1000–4099-byte output for 0 B to 100 KiB inputs) from `@noble/hashes` 1.8.0, an independent implementation, checked on the 1-, 4- and 8-lane paths. The official vectors stop at 131 bytes, which is too short to reach the batched path. - **Batched vs scalar:** every input length 0…1024 with 2048 bytes of output (what LtHash asks for) on 4-way and 8-way against scalar, and every output length 0…1280. - LtHash `Add`/`Remove` checked lane for lane against the scalar construction, including elements past the 512-byte stack buffer and past one BLAKE3 chunk. - `DidKeyCache`: results match `AtProtoKey.Verify` (valid, tampered, wrong message), LRU capacity, malformed and Ed25519 did:keys throw without caching, and 8 threads verifying through a one-entry cache (constant eviction while keys are rented) all get correct answers. - `K256_GenerateAndSign_IfSupported` (renamed `GenerateK256Key_SignAndVerify_RoundTripsThroughDidKey`) now calls `Assert.Skip` where secp256k1 is missing instead of passing silently. It also checks low-S and the cached verify. - Also covered: off-curve x for both curves, x ≥ p, bad prefixes, wrong multikey lengths, both Y parities round-tripping through did:key, wrong-length signatures, known base58 values including leading zeros, and span-overflow. ### Benchmarks BenchmarkDotNet 0.14, ShortRun (3 warmup, 10 iterations, in-process), .NET 10.0.12, AMD Ryzen 5 7600X. "Before" is `main` at 7f672c0 and "after" is this branch, with the same harness. | Operation | Before | After | Speed-up | |---|---:|---:|---:| | BLAKE3 XOF, 90 B input → 2048 B | 12,109 ns / 800 B | **762 ns / 0 B** | 15.9× | | BLAKE3 32 B digest, 90 B input | 1,025 ns / 800 B | 264 ns / 0 B | 3.9× | | BLAKE3 32 B digest, 8 KiB input | 47,930 ns / 4,048 B | 14,273 ns / 0 B | 3.4× | | `LtHash.Add` | 13,210 ns / 800 B | **814 ns / 0 B** | 16.2× | | `LtHash.Remove` | 13,237 ns / 800 B | 812 ns / 0 B | 16.3× | | `LtHash.Digest` | 1,724 ns / 56 B | 1,105 ns / 56 B | 1.6× | | `VerifySignature(did:key)` P-256 | 94,990 ns / 4,176 B | **41,260 ns / 1,416 B** | 2.3× | | `VerifySignature(did:key)` K-256 | 294,760 ns / 3,874 B | 210,700 ns / 1,417 B | 1.4× | | `FromDidKey` P-256 (uncached by design) | 42,330 ns / 2,480 B | 41,990 ns / 1,336 B | – | | `AtProtoKey.Sign` P-256 | 14,220 ns / 724 B | 14,050 ns / 417 B | alloc −42 % | - `SpaceRepoCommit.FromIndex`, 10,000 records (Stopwatch, mean of 20 runs, including building the element strings): **133 ms → 9–12 ms** (runs of 8.6, 10.8 and 12.0 ms). - With AVX2 disabled (`DOTNET_EnableAVX2=0`, so the 4-way `Vector128` path), the 2048-byte XOF takes 2,738 ns, 4.4× faster than before. - For reference, the performance audit's prototype measured 721 ns for the XOF and 772 ns for `LtHash.Add`. This production version is within about 5 % of it without `Unsafe` or a length-limited fast path. ## Edits outside `Crypto/` and `Spaces/LtHash.cs` - `src/ATProtoNet/Auth/ServiceAuthGenerator.cs` and `src/ATProtoNet/Spaces/SpaceTokens.cs`: one line each, to use `KeyCurve.JwsAlgorithm()`. - `tests/ATProtoNet.Tests/ATProtoNet.Tests.csproj`: one `<None Include="Crypto/TestData/*.json" CopyToOutputDirectory="PreserveNewest" />` line, so the vendored fixtures reach the test output. This is the only way to ship them as JSON files. - `docs/crypto.md`: two sentences on the verification cache and the rejected signature forms. ## Deferred - `src/ATProtoNet.Server/Spaces/JsonWebKeyVerifier.cs` still has the reverse mapping (JWK `crv` → curve and `alg`) and a K-256 OID literal. That file belongs to a parallel PR. It could use the same curve metadata once that lands. - `FromDidKey`/`FromMultikey` stay uncached, because they return a key the caller owns and disposes. Their cost is mostly the BigInteger square root and the platform import. - The per-lane carry into the high counter word is kept for correctness, but a span-sized output cannot reach it (at most 2^25 blocks). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Grandiras added 1 commit

2026-09-24 18:14:58 +00:00

perf(crypto): vectorized BLAKE3 / LtHash and cached did:key parsing (closes #112 )

CI / pds-integration (pull_request) Successful in 28s

Details

CI / build-and-test (pull_request) Successful in 1m1s

Details

b226fea721

LtHash expands every record path to 32 BLAKE3 output blocks, and each
space commit, sync pass and getRepo verify pays that per record. XOF
output blocks differ only in their counter, so BLAKE3 now compresses
8 (Vector256) or 4 (Vector128) of them at once. The compression
function is written once over the lane type, and the state lives in
locals. LtHash adds and subtracts its lanes with Vector<ushort> straight
over the XOF output. There is a scalar path for hosts without vectors
and a byte swap for big-endian hosts. Nothing is heap-allocated.

Signature verification against a did:key re-parsed the key every time,
and the parse costs as much as the verification. VerifySignature now
goes through a bounded LRU cache of imported keys. Each entry pools its
ECDsa instances, because .NET does not document them as safe for
concurrent use. Curve constants are computed once, base58 decodes into a
stack buffer with bounded work, IsLowS no longer allocates, and the
curve-to-JWS-alg mapping is one internal helper.

Tests add the atproto-interop-tests crypto fixtures (vendored, CC0),
long-output BLAKE3 vectors from an independent implementation, every
single-chunk input length on each batch width, lane-for-lane LtHash
checks, and cache behaviour under concurrent eviction. The K-256 round
trip now reports a skip instead of passing where secp256k1 is missing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Grandiras commented

2026-09-24 18:15:08 +00:00

@claude /review

claude-bot commented

2026-09-24 18:15:12 +00:00

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/585); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-150/run-585.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-09-24 18:17:39 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

This is a large, carefully constructed perf PR, and the implementation holds up under close reading.

What I checked in detail:

BLAKE3 vectorized compression (Blake3.cs): the generic ILanes<T> abstraction produces identical rounds/permutation across scalar, Vector128, Vector256 paths. The per-lane counter carry (Vector128.LessThan(low, first) mask subtraction) correctly propagates overflow from the low 32 bits into the high word. Squeeze cascades 8 → 4 → 1 → partial-block correctly and the tree construction (ChunkNode/TreeRoot/ParentNode) mirrors the reference chunk-tree algorithm faithfully, including the trailing-zero merge pattern. Extensive scalar-vs-vectorized differential tests plus independent-implementation vectors back this up.
LtHash (LtHash.cs): the direct-into-lane-span BLAKE3 write with a big-endian swap guard is correct, and the Vector<ushort> add/subtract with a scalar tail correctly matches the unchecked-wraparound scalar semantics.
DidKeyCache: the double-checked insert (parse outside the lock, re-check under lock, discard the loser) is correct and avoids leaking AtProtoKey instances; eviction disposes idle keys immediately and defers to Return() for rented ones via the _evicted flag. The concurrent eviction test is a good stress case for this.
Base58Decode into a Span<byte>: the leading-zero counting, capacity bound, and final reverse-then-copy-into-offset (verified safe under span overlap since Span<T>.CopyTo uses Memmove semantics) are correct, and the length-bounded decode meaningfully closes a DoS vector (previously unbounded List<byte> growth for arbitrarily long/malicious base58 input).
CurveInfo/DecompressPoint: computing P/A/B/order once from hex literals instead of re-deriving 5 byte arrays per call is straightforward and preserves behavior; the sqrt-and-verify step (y*y % p != ySquared) still guards against off-curve X values.
Signature-length hardening (HasSignatureLength) and the FormatException/ArgumentOutOfRangeException behavior changes are explicitly called out in the CHANGELOG under Fixed, consistent with CLAUDE.md's changelog requirement, and both are documented as source/binary compatible.

I didn't find correctness bugs. The test suite described (official BLAKE3 vectors, independent long-output vectors, exhaustive batched-vs-scalar sweeps, vendored atproto-interop-tests fixtures, and the cache concurrency test) covers the areas that would be easiest to get subtly wrong in this kind of change.

⏱ 2m 14s · 1 turns · tokens: 63.9k in / 13.4k out (+23.9k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. This is a large, carefully constructed perf PR, and the implementation holds up under close reading. What I checked in detail: - **BLAKE3 vectorized compression** (`Blake3.cs`): the generic `ILanes<T>` abstraction produces identical rounds/permutation across scalar, `Vector128`, `Vector256` paths. The per-lane counter carry (`Vector128.LessThan(low, first)` mask subtraction) correctly propagates overflow from the low 32 bits into the high word. `Squeeze` cascades 8 → 4 → 1 → partial-block correctly and the tree construction (`ChunkNode`/`TreeRoot`/`ParentNode`) mirrors the reference chunk-tree algorithm faithfully, including the trailing-zero merge pattern. Extensive scalar-vs-vectorized differential tests plus independent-implementation vectors back this up. - **LtHash** (`LtHash.cs`): the direct-into-lane-span BLAKE3 write with a big-endian swap guard is correct, and the `Vector<ushort>` add/subtract with a scalar tail correctly matches the unchecked-wraparound scalar semantics. - **`DidKeyCache`**: the double-checked insert (parse outside the lock, re-check under lock, discard the loser) is correct and avoids leaking `AtProtoKey` instances; eviction disposes idle keys immediately and defers to `Return()` for rented ones via the `_evicted` flag. The concurrent eviction test is a good stress case for this. - **`Base58Decode` into a `Span<byte>`**: the leading-zero counting, capacity bound, and final reverse-then-copy-into-offset (verified safe under span overlap since `Span<T>.CopyTo` uses `Memmove` semantics) are correct, and the length-bounded decode meaningfully closes a DoS vector (previously unbounded `List<byte>` growth for arbitrarily long/malicious base58 input). - **`CurveInfo`/`DecompressPoint`**: computing P/A/B/order once from hex literals instead of re-deriving 5 byte arrays per call is straightforward and preserves behavior; the sqrt-and-verify step (`y*y % p != ySquared`) still guards against off-curve X values. - **Signature-length hardening** (`HasSignatureLength`) and the `FormatException`/`ArgumentOutOfRangeException` behavior changes are explicitly called out in the CHANGELOG under `Fixed`, consistent with `CLAUDE.md`'s changelog requirement, and both are documented as source/binary compatible. I didn't find correctness bugs. The test suite described (official BLAKE3 vectors, independent long-output vectors, exhaustive batched-vs-scalar sweeps, vendored atproto-interop-tests fixtures, and the cache concurrency test) covers the areas that would be easiest to get subtly wrong in this kind of change. --- ⏱ 2m 14s · 1 turns · tokens: 63.9k in / 13.4k out (+23.9k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-150/run-585.md)

Grandiras force-pushed issue-112-crypto-perf from b226fea721

CI / pds-integration (pull_request) Successful in 28s

Details

CI / build-and-test (pull_request) Successful in 1m1s

Details

to 446067915f

CI / pds-integration (pull_request) Successful in 25s

Details

CI / build-and-test (pull_request) Successful in 55s

Details

Sync Closures to GitHub / sync-closure (pull_request) Successful in 7s

Details

2026-09-24 18:18:22 +00:00

Compare

opencode-bot commented

2026-09-24 18:19:19 +00:00

🤖 Reviewing this PR — the review will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the review runs.

🤖 Reviewing this PR — the review will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-150/run-2026-09-24T18-19-18-941Z.md) — updating every ~5 minutes while the review runs.

Grandiras commented

2026-09-24 18:19:36 +00:00

Rebased onto main to resolve a CHANGELOG-only conflict (#149 merged). verify-rebase: the PR's patch outside CHANGELOG.md is byte-identical between the approved head b226fea7 and the new head 44606791 (2058 changed lines). CI is green on the new head (and 2546 unit tests pass locally), so the approval carries over under the maintainer-approved policy for CHANGELOG-only rebases.

Rebased onto main to resolve a CHANGELOG-only conflict (#149 merged). `verify-rebase`: the PR's patch outside `CHANGELOG.md` is byte-identical between the approved head `b226fea7` and the new head `44606791` (2058 changed lines). CI is green on the new head (and 2546 unit tests pass locally), so the approval carries over under the maintainer-approved policy for CHANGELOG-only rebases.

Grandiras merged commit e261596308 into main

2026-09-24 18:19:38 +00:00

Grandiras deleted branch issue-112-crypto-perf

2026-09-24 18:19:38 +00:00

Grandiras referenced this pull request from a commit

2026-09-24 18:19:39 +00:00

perf(crypto): vectorized BLAKE3 / LtHash and cached did:key parsing (#150)

opencode-reviewer approved these changes

2026-09-24 18:37:44 +00:00

opencode-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Independent read of the whole diff finds no correctness or security blockers. What I verified in detail:

BLAKE3: the generic ILanes<T> compression reproduces the spec's quarter-rounds, column/diagonal schedule and MSG_PERMUTATION tuple swap; the batched Squeeze cascades 8 → 4 → 1 → partial with strictly sequential output-block counters, and the LessThan(low, first) mask subtraction correctly carries low-word wrap into the high counter word. ChunkNode/TreeRoot reproduce the reference chunk-tree (CHUNK_START on the first block, CHUNK_END on the last, trailing-zero merges, right-to-left stack fold, root counter preserved), and Store's lane-major transpose puts block i (counter i) at the right offset. All word/byte conversion is explicit little-endian. The official vectors, the noble long-output pins, and the exhaustive scalar-vs-batched sweeps give strong evidence all three paths agree.
LtHash: writing the XOF straight into the lane bytes is equivalent to the previous ReadUInt16LittleEndian walk on little-endian hosts, and the in-place ReverseEndianness swap covers big-endian; Vector<ushort> add/sub wraps identically to the unchecked scalar ops, with a correct scalar tail; WriteState/state-ctor bulk copies touch only StateBytes.
Crypto refactor: CurveInfo constants (p, a, b, order, n/2, (p+1)/4) match the known values for both curves; DecompressPoint keeps the range check, the √-verification and parity selection, now writing fixed 32-byte ECPoint fields. The explicit 64-byte r || s check makes the DER/high-S rejection deterministic — the vendored interop fixtures cover exactly those cases.
DidKeyCache: the parse-outside-lock + double-checked insert is correct (a racing loser's entry is evicted, a capacity evict always removes the LRU tail, never the just-added entry); evicted entries dispose idle keys immediately and flag rented ones for dispose-on-return, so nothing leaks or gets used after dispose; pools are bounded; malformed keys throw before anything is cached; no nested locks.
Base58: the digit-table decode matches known values, the destination-bound overflow check is right, and the reverse/copy sequence is memmove-safe with overlapping spans.
Housekeeping: the interop fixtures are byte-identical to bluesky-social/atproto-interop-tests@056e5741 (checked contents; LICENSE-CC0 exists upstream), the xunit-v3 TheoryData<T>(IEnumerable<T>) constructor used by the new tests exists, Assert.Skip is a v3 API, the csproj copy mirrors the existing Streaming/TestData pattern, and all in-repo callers of VerifySignature/FromDidKey catch FormatException/ArgumentException, so the documented exception-type changes are safe.

Two minor, non-blocking notes below (post-merge review — the PR merged a few seconds after this run started, so treat these as follow-ups if convenient).

⏱ 18m 25s · 30 steps · $0.1958 · tokens: 304.5k in / 35.2k out (+3772.0k cache-read) · Go: 5h 3% · wk 97% · mo 81%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Independent read of the whole diff finds no correctness or security blockers. What I verified in detail: - **BLAKE3**: the generic `ILanes<T>` compression reproduces the spec's quarter-rounds, column/diagonal schedule and MSG_PERMUTATION tuple swap; the batched `Squeeze` cascades 8 → 4 → 1 → partial with strictly sequential output-block counters, and the `LessThan(low, first)` mask subtraction correctly carries low-word wrap into the high counter word. `ChunkNode`/`TreeRoot` reproduce the reference chunk-tree (CHUNK_START on the first block, CHUNK_END on the last, trailing-zero merges, right-to-left stack fold, root counter preserved), and `Store`'s lane-major transpose puts block *i* (counter *i*) at the right offset. All word/byte conversion is explicit little-endian. The official vectors, the noble long-output pins, and the exhaustive scalar-vs-batched sweeps give strong evidence all three paths agree. - **LtHash**: writing the XOF straight into the lane bytes is equivalent to the previous `ReadUInt16LittleEndian` walk on little-endian hosts, and the in-place `ReverseEndianness` swap covers big-endian; `Vector<ushort>` add/sub wraps identically to the `unchecked` scalar ops, with a correct scalar tail; `WriteState`/state-ctor bulk copies touch only `StateBytes`. - **Crypto refactor**: `CurveInfo` constants (p, a, b, order, n/2, (p+1)/4) match the known values for both curves; `DecompressPoint` keeps the range check, the √-verification and parity selection, now writing fixed 32-byte `ECPoint` fields. The explicit 64-byte `r || s` check makes the DER/high-S rejection deterministic — the vendored interop fixtures cover exactly those cases. - **`DidKeyCache`**: the parse-outside-lock + double-checked insert is correct (a racing loser's entry is evicted, a capacity evict always removes the LRU tail, never the just-added entry); evicted entries dispose idle keys immediately and flag rented ones for dispose-on-return, so nothing leaks or gets used after dispose; pools are bounded; malformed keys throw before anything is cached; no nested locks. - **Base58**: the digit-table decode matches known values, the destination-bound overflow check is right, and the reverse/copy sequence is memmove-safe with overlapping spans. - **Housekeeping**: the interop fixtures are byte-identical to `bluesky-social/atproto-interop-tests@056e5741` (checked contents; `LICENSE-CC0` exists upstream), the xunit-v3 `TheoryData<T>(IEnumerable<T>)` constructor used by the new tests exists, `Assert.Skip` is a v3 API, the csproj copy mirrors the existing `Streaming/TestData` pattern, and all in-repo callers of `VerifySignature`/`FromDidKey` catch `FormatException`/`ArgumentException`, so the documented exception-type changes are safe. Two minor, non-blocking notes below (post-merge review — the PR merged a few seconds after this run started, so treat these as follow-ups if convenient). --- ⏱ 18m 25s · 30 steps · $0.1958 · tokens: 304.5k in / 35.2k out (+3772.0k cache-read) · Go: 5h 3% · wk 97% · mo 81% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-150/run-2026-09-24T18-19-18-941Z.md)

src/ATProtoNet/Crypto/Blake3.cs

					
				@ -177,0 +256,4 @@

				        if (bytes.Length < BlockLen)

				        {

				            Span<byte> padded = stackalloc byte[BlockLen];

The zero-padding the doc comment promises is implicit: padded is a plain stackalloc, and only bytes.Length bytes are copied. This works today because the C# compiler emits localsinit by default (which zero-initializes stackalloc data — confirmed via the SkipLocalsInit proposal/API docs), so the official vectors pass on every supported runtime as configured. But for a hash primitive this is a fragile invisible dependency: if anyone later adds [Module: SkipLocalsInit] — a documented perf optimization aimed precisely at stackalloc-heavy code like this — every input whose length isn't a multiple of 64 would hash leftover stack bytes into the block, producing garbage-dependent, test-flaky output. One padded.Fill(0) (or padded[bytes.Length..].Clear()) before the copy makes the contract explicit at effectively zero cost.

The zero-padding the doc comment promises is implicit: `padded` is a plain `stackalloc`, and only `bytes.Length` bytes are copied. This works today because the C# compiler emits `localsinit` by default (which zero-initializes stackalloc data — confirmed via the `SkipLocalsInit` proposal/API docs), so the official vectors pass on every supported runtime as configured. But for a hash primitive this is a fragile invisible dependency: if anyone later adds `[Module: SkipLocalsInit]` — a documented perf optimization aimed precisely at stackalloc-heavy code like this — every input whose length isn't a multiple of 64 would hash leftover stack bytes into the block, producing garbage-dependent, test-flaky output. One `padded.Fill(0)` (or `padded[bytes.Length..].Clear()`) before the copy makes the contract explicit at effectively zero cost.

tests/ATProtoNet.Tests/Crypto/AtProtoInteropCryptoTests.cs

					
				@ -0,0 +47,4 @@

				    [Theory]

				    [MemberData(nameof(SignatureFixtures))]

				    public void VerifySignature_InteropFixture_MatchesExpectedValidity(string comment)

These two fixture theories (and the one at line 67) iterate the K-256 fixtures without an environment gate: on a host without secp256k1 (macOS/Windows), AtProtoCrypto.VerifySignature/ImportCompressedPublicKey throw PlatformNotSupportedException from ECDsa.Create, which would fail rather than skip — unlike AssertDidKeyFixture and GenerateK256KeyOrSkip, which gate correctly. Consider skipping the K-256 fixtures when fixture.Algorithm == "ES256K" on a platform that lacks the curve, e.g. by probing support once in a static helper.

These two fixture theories (and the one at line 67) iterate the K-256 fixtures without an environment gate: on a host without secp256k1 (macOS/Windows), `AtProtoCrypto.VerifySignature`/`ImportCompressedPublicKey` throw `PlatformNotSupportedException` from `ECDsa.Create`, which would fail rather than skip — unlike `AssertDidKeyFixture` and `GenerateK256KeyOrSkip`, which gate correctly. Consider skipping the K-256 fixtures when `fixture.Algorithm == "ES256K"` on a platform that lacks the curve, e.g. by probing support once in a static helper.

opencode-bot commented

2026-09-24 18:37:44 +00:00

🔧 Let opencode-bot fix this — tick the box and opencode-bot picks up the review above, on this PR branch. Nothing runs until you do.

@opencode-bot Address the findings of the latest review on this pull request.

🔧 **Let opencode-bot fix this** — tick the box and opencode-bot picks up the review above, on this PR branch. Nothing runs until you do. - [ ] @opencode-bot Address the findings of the latest review on this pull request.

Grandiras referenced this pull request

2026-09-24 18:53:33 +00:00

refactor(auth): shared JWT, DPoP and base64url primitives #151

Grandiras referenced this pull request

2026-09-24 19:05:04 +00:00

refactor(http)!: one error model and a robust XRPC transport #152

Sign in to join this conversation.

No reviewers