Both places the SDK pulled a signing key out of a DID document required
`type == "Multikey"`, so a document publishing the older
`EcdsaSecp256k1VerificationKey2019` / `EcdsaSecp256r1VerificationKey2019`
form resolved to no key at all: `SpaceSyncer.ResolveSigningKeyAsync` threw
`SpaceRepoVerificationException` and `FirehoseVerifier` treated every commit
from that account as unverifiable.

The two encodings differ by more than the type string — a Multikey's
`publicKeyMultibase` is base58btc over multicodec-tagged compressed key
bytes, the legacy form is base58btc over a bare uncompressed point
(0x04 || X || Y) with no prefix — so the point is compressed by the parity
of Y and re-tagged with the curve the type names, matching the reference
implementation's `getKey`.

The conversion lives in one place rather than at each call site:
`VerificationMethod.ToDidKey()`, `DidDocument.GetSigningKey()` and
`DidDocument.GetVerificationKey(fragment)`, over the new
`AtProtoCrypto.FormatDidKey` / `CompressPublicKey`. `SpaceAuthority` and
`FirehoseVerifier` both go through them, and `SpaceNetworkFixture` drops
the private resolver it needed to work around this.

closes #98

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

claude-bot referenced this pull request

2026-08-20 21:53:01 +00:00

DID document key extraction accepts only Multikey, not the legacy verification-method types #98

claude-bot commented

2026-08-20 21:53:05 +00:00

Author

Collaborator

🤖 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/334); 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-103/run-334.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-08-20 21:54:21 +00:00

Dismissed

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Correct, well-scoped fix. The core insight — Multikey's publicKeyMultibase is already multicodec-tagged compressed key bytes while the legacy EcdsaSecp256k1/r1VerificationKey2019 forms are a bare uncompressed point — is handled cleanly by putting the conversion in one place (VerificationMethod.ToDidKey()) and having DidDocument.GetSigningKey/GetVerificationKey, SpaceAuthority.GetSigningKey, and FirehoseVerifier all go through it instead of three near-duplicate hand-rolled lookups.

Verified the crypto details: CompressPublicKey sets the SEC1 prefix from the parity of the last byte of Y (big-endian, so that's the LSB) — standard convention, matches what AtProtoKey.ToMultikey() already does elsewhere in the file. FormatDidKey composes CompressPublicKey + the existing internal ToMultikey(bytes, curve) the same way AtProtoKey.ToDidKey() does, so there's no drift between the two call paths. Point-on-curve validation is correctly deferred to decompression (FromDidKey/VerifySignature), which is fine since that's where it's cryptographically meaningful — compressing an off-curve point just produces a value that later fails to decompress/verify rather than silently succeeding.

SpaceNetworkFixture.ResolveSigningKeyAsync now delegates straight to SpaceSyncer.ResolveSigningKeyAsync(DidResolver), which is a good simplification — the tests now exercise the actual production code path (SpaceAuthority.GetSigningKey → DidDocument.GetVerificationKey → VerificationMethod.ToDidKey) instead of a parallel hand-written implementation that could drift from it.

Test coverage is solid: both curves, both Y parities (looped until both are observed), a real sign/verify round trip on the recovered key (not just string equality against a re-derived value), unknown-type and missing-key-material null cases, malformed-material exception cases, and the SpaceAuthority space-key-preferred-over-account-key fallback. Docs (did-resolution.md, testing-spaces.md) and CHANGELOG were updated consistently with the code change, filed correctly under Fixed since nothing here is a breaking signature change.

⏱ 1m 0s · 7 turns · tokens: 34.6k in / 5.0k out (+384.1k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Correct, well-scoped fix. The core insight — Multikey's `publicKeyMultibase` is already multicodec-tagged compressed key bytes while the legacy `EcdsaSecp256k1/r1VerificationKey2019` forms are a bare uncompressed point — is handled cleanly by putting the conversion in one place (`VerificationMethod.ToDidKey()`) and having `DidDocument.GetSigningKey/GetVerificationKey`, `SpaceAuthority.GetSigningKey`, and `FirehoseVerifier` all go through it instead of three near-duplicate hand-rolled lookups. Verified the crypto details: `CompressPublicKey` sets the SEC1 prefix from the parity of the last byte of Y (big-endian, so that's the LSB) — standard convention, matches what `AtProtoKey.ToMultikey()` already does elsewhere in the file. `FormatDidKey` composes `CompressPublicKey` + the existing internal `ToMultikey(bytes, curve)` the same way `AtProtoKey.ToDidKey()` does, so there's no drift between the two call paths. Point-on-curve validation is correctly deferred to decompression (`FromDidKey`/`VerifySignature`), which is fine since that's where it's cryptographically meaningful — compressing an off-curve point just produces a value that later fails to decompress/verify rather than silently succeeding. `SpaceNetworkFixture.ResolveSigningKeyAsync` now delegates straight to `SpaceSyncer.ResolveSigningKeyAsync(DidResolver)`, which is a good simplification — the tests now exercise the actual production code path (`SpaceAuthority.GetSigningKey` → `DidDocument.GetVerificationKey` → `VerificationMethod.ToDidKey`) instead of a parallel hand-written implementation that could drift from it. Test coverage is solid: both curves, both Y parities (looped until both are observed), a real sign/verify round trip on the recovered key (not just string equality against a re-derived value), unknown-type and missing-key-material null cases, malformed-material exception cases, and the SpaceAuthority space-key-preferred-over-account-key fallback. Docs (`did-resolution.md`, `testing-spaces.md`) and CHANGELOG were updated consistently with the code change, filed correctly under Fixed since nothing here is a breaking signature change. --- ⏱ 1m 0s · 7 turns · tokens: 34.6k in / 5.0k out (+384.1k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-103/run-334.md)

Grandiras commented

2026-08-20 21:54:22 +00:00

Owner

@claude-bot Please fix the merge conflict

claude-bot commented

2026-08-20 21:54:25 +00:00

Author

Collaborator

🤖 On it — follow along in the action run; the reply will link the full session transcript.

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

🤖 On it — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/336); 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/issue-103/run-336.md) — updating every ~5 minutes while the task runs.

claude-bot added 1 commit

2026-08-20 21:56:00 +00:00

Merge remote-tracking branch 'origin/main' into claude/issue-98

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

Details

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

Details

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

Details

949a48f6a6

# Conflicts:
#	CHANGELOG.md

claude-bot commented

2026-08-20 21:56:22 +00:00

Author

Collaborator

Merged main into claude/issue-98 and pushed — the PR is conflict-free now.

The conflict was in CHANGELOG.md only: main picked up the Issue #99 bullet at the top of ### Fixed (via #100) while this branch added the Issue #98 bullet in the same spot. Both entries are real and both belong, so I kept both, ordered #99 then #98 to match the file's descending-issue-number convention. No text was dropped from either side.

Nothing else conflicted textually, but main's #99 change also touches SpaceSyncer.cs — the same file this branch modifies — so I checked the auto-merge rather than trusting it:

SpaceSyncer.ResolveSigningKeyAsync(DidResolver) (this branch's addition, which routes through SpaceAuthority.GetSigningKey → DidDocument.GetVerificationKey → VerificationMethod.ToDidKey) is intact alongside #99's new SpaceSyncOutcome.NoRepo path.
SpaceNetworkFixture.ResolveSigningKeyAsync still delegates to the production resolver, and SpaceRepoSyncTests picked up #99's renamed SyncRepoAsync_ForAnAccountThatHasWrittenNothing_ReportsNoRepo on top of it cleanly — the two changes are orthogonal.

Verified: dotnet build succeeds with 0 warnings / 0 errors (including the integration-test project, which is where the fixture change lives), and dotnet test tests/ATProtoNet.Tests/ passes — 1552 passed, 0 failed, 5 skipped (the usual Aspire-manifest gates). git merge-tree against origin/main now reports no conflicts.

Pushed as merge commit 949a48f on claude/issue-98. No new branch, no new PR.

⏱ 1m 39s · 14 turns · tokens: 36.4k in / 3.2k out (+574.8k cache-read)
📜 Full session transcript

Merged `main` into `claude/issue-98` and pushed — the PR is conflict-free now. **The conflict** was in `CHANGELOG.md` only: `main` picked up the Issue #99 bullet at the top of `### Fixed` (via #100) while this branch added the Issue #98 bullet in the same spot. Both entries are real and both belong, so I kept both, ordered #99 then #98 to match the file's descending-issue-number convention. No text was dropped from either side. Nothing else conflicted textually, but `main`'s #99 change also touches `SpaceSyncer.cs` — the same file this branch modifies — so I checked the auto-merge rather than trusting it: - `SpaceSyncer.ResolveSigningKeyAsync(DidResolver)` (this branch's addition, which routes through `SpaceAuthority.GetSigningKey` → `DidDocument.GetVerificationKey` → `VerificationMethod.ToDidKey`) is intact alongside #99's new `SpaceSyncOutcome.NoRepo` path. - `SpaceNetworkFixture.ResolveSigningKeyAsync` still delegates to the production resolver, and `SpaceRepoSyncTests` picked up #99's renamed `SyncRepoAsync_ForAnAccountThatHasWrittenNothing_ReportsNoRepo` on top of it cleanly — the two changes are orthogonal. **Verified:** `dotnet build` succeeds with 0 warnings / 0 errors (including the integration-test project, which is where the fixture change lives), and `dotnet test tests/ATProtoNet.Tests/` passes — 1552 passed, 0 failed, 5 skipped (the usual Aspire-manifest gates). `git merge-tree` against `origin/main` now reports no conflicts. Pushed as merge commit `949a48f` on `claude/issue-98`. No new branch, no new PR. --- ⏱ 1m 39s · 14 turns · tokens: 36.4k in / 3.2k out (+574.8k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-103/run-336.md)

Grandiras merged commit 5bd0062f77 into main

2026-08-20 21:56:59 +00:00

Grandiras deleted branch claude/issue-98

2026-08-20 21:56:59 +00:00

Grandiras referenced this pull request from a commit

2026-08-20 21:57:00 +00:00

Claude: changes for issue #98 (#103)

claude-bot commented

2026-08-20 21:57:02 +00:00

Author

Collaborator

🤖 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/338); 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-103/run-338.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-08-20 21:57:29 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

No new commits since my last pass — the diff is identical to what I already reviewed and approved. Reconfirming: the fix correctly centralizes did:key extraction (VerificationMethod.ToDidKey(), DidDocument.GetSigningKey()/GetVerificationKey(), AtProtoCrypto.FormatDidKey()/CompressPublicKey()) so Multikey and the legacy EcdsaSecp256k1/r1VerificationKey2019 forms both resolve correctly, removes the three near-duplicate hand-rolled lookups (SpaceAuthority.FindMultikey, FirehoseVerifier.GetSigningKey, the test fixture's private resolver), and is well covered by tests (both curves, both Y parities, real sign/verify round trips, null and malformed-input cases). Docs and CHANGELOG are updated consistently. Good to merge.

⏱ 0m 16s · 3 turns · tokens: 30.0k in / 1.0k out (+132.1k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. No new commits since my last pass — the diff is identical to what I already reviewed and approved. Reconfirming: the fix correctly centralizes did:key extraction (`VerificationMethod.ToDidKey()`, `DidDocument.GetSigningKey()`/`GetVerificationKey()`, `AtProtoCrypto.FormatDidKey()`/`CompressPublicKey()`) so `Multikey` and the legacy `EcdsaSecp256k1/r1VerificationKey2019` forms both resolve correctly, removes the three near-duplicate hand-rolled lookups (`SpaceAuthority.FindMultikey`, `FirehoseVerifier.GetSigningKey`, the test fixture's private resolver), and is well covered by tests (both curves, both Y parities, real sign/verify round trips, null and malformed-input cases). Docs and CHANGELOG are updated consistently. Good to merge. --- ⏱ 0m 16s · 3 turns · tokens: 30.0k in / 1.0k out (+132.1k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-103/run-338.md)