fix(auth): select signing algorithm explicitly for self-certifying DIDs#4
Merged
Merged
Conversation
Willow DIDs are now self-certifying
(did:willow:z<base58btc(SHA3-256(multicodec||pubkey))>) and no longer embed
the key algorithm in the string. detect_algorithm_from_did could therefore no
longer recover it and silently defaulted to Ed25519, so a secp256k1
(Ethereum/wallet) identity signed per-request auth and consensus transactions
with the wrong algorithm and was rejected by the server.
Thread an explicit `algorithm` argument (default "Ed25519", backward
compatible) through the signing paths:
- auth.sign_request
- WillowClient.set_identity -> stored and passed to its three
sign_request call sites
- ConsensusClient signing methods -> _sign_and_broadcast
detect_algorithm_from_did is retained (it still parses the retired
did:willow:<alg>:<pubkey> format) but is no longer used to choose a signing
algorithm and is documented as deprecated. The DID string is unchanged.
Add regression tests proving a secp256k1 identity signs with secp256k1 (and
would NOT verify under the old Ed25519 default) and that Ed25519 still
defaults correctly, at both the auth.sign_request and WillowClient levels.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
2 participants
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.
Problem
Willow DIDs are now self-certifying:
They no longer embed the key algorithm (the retired format was
did:willow:<alg>:<pubkey>, e.g.did:willow:ed25519:../did:willow:secp256k1:..).detect_algorithm_from_didparsed the algorithm out of the DID string and was used bysign_request(per-request auth) and the consensus client's transaction signing to choose Ed25519 vs secp256k1. With a self-certifying id that parse is impossible, so it silently defaults to Ed25519. A secp256k1 (Ethereum/wallet) identity then signs per-request auth and consensus transactions with the wrong algorithm and is rejected. Ed25519-first identities (the SDK default) were unaffected, which masked the bug.Reproduced before the fix: for a freshly generated secp256k1 identity,
detect_algorithm_from_did(did)returnsEd25519and the resulting per-request signature fails secp256k1 verification.Fix
Thread an explicit
algorithmargument (default"Ed25519", fully backward compatible) through the signing paths, instead of inferring it from the DID string:auth.sign_request(..., algorithm="Ed25519")WillowClient.set_identity(..., algorithm="Ed25519")— stored on the client and passed to its threesign_requestcall sites; reset byclear_identity.ConsensusClientsigning methods (register_did,register_subgrove,transfer,store_data,store_file_manifest,delete_file_manifest,deregister_subgrove) →_sign_and_broadcast(..., algorithm="Ed25519").The new parameter is appended (or placed after the existing trailing optional) in every signature, so existing positional and keyword calls are unaffected.
detect_algorithm_from_didis retained (it still parses the retireddid:willow:<alg>:<pubkey>format and remains exported) but is no longer used to choose a signing algorithm and is documented as deprecated. The DID string is unchanged — the algorithm is not reintroduced into it.Tests
tests/test_auth.py:sign_requestselects secp256k1 for a secp256k1 identity (signature verifies under secp256k1 and would NOT verify under the old Ed25519 default); Ed25519 still defaults; a self-certifying secp256k1 DID is documented as not algorithm-detectable.tests/test_client.py:set_identity(..., "secp256k1")is stored/reset; an end-to-end authenticated request (mocked transport) captures the signed headers and confirms the signature verifies as secp256k1, not Ed25519.Full suite: 295 passed, 13 skipped (the skips are pre-existing integration tests).
🤖 Generated with Claude Code