Feat: did:ethr resolver#70
Open
mirceanis wants to merge 29 commits into
Open
Conversation
…DefaultEthereumRpcClient, models)
Also adds AdditionalProperties to VerificationMethod for publicKeyHex support.
- NetDidBuilder.AddDidEthr(networks) - NetDid.Samples.DidEthr - CHANGELOG [Unreleased] section with full feature summary - tasks/todo20260522-didethr.md all items checked + review section
PR moisesja#36 (upstream) made SupportedKeyTypes abstract on DidMethodBase. did:ethr only accepts Secp256k1 — declare that explicitly.
1. EthereumNetworkConfig default RegistryAddress was 0xdCa7EF... (mainnet). Sepolia uses 0x03d5003bf0e79c5f5223588f347eba39afbc3818. Updated sample to use the correct address. 2. EthrDocumentBuilder.BuildContext was emitting raw Dictionary<> objects for inline @context entries. WriteContextArray only handles string and JsonElement, so they were serialized as type names. Fixed by using JsonSerializer.SerializeToElement() to produce JsonElement values. 3. VerificationMethodJsonConverter.Write did not flush AdditionalProperties (added for publicKeyHex support). Fixed by iterating and writing them before WriteEndObject(), matching the pattern already used by ServiceJsonConverter.
The JS ethr-did-resolver keys entries by (eventName, name/delegateType, value/delegate) and always increments delegateCount even for revocation events. A revocation event DELETES any previously-added entry for the same logical key, meaning a revocation removes the earlier valid entry and a re-registration gets a new ID. Our implementation was using the event counter as the sole key, so every event got an independent slot — a revocation was ignored, leaving the original valid entry alive alongside the re-registration. Fix: rewrite the event replay loop to match spec semantics exactly: - entries are Dictionary<eventIndex, (counter, entry)> - delegateCount/serviceCount always increment (valid or expired) - valid events add/overwrite the entry at eventIndex - expired events remove the entry at eventIndex (if present)
Both #controller and delegate VMs now emit checksummed addresses (e.g. 0xdbF03B407c01E7cD3CBea99509d93f8DDDC8C6FB) rather than the lowercase form that comes off the wire from event decoding. Tests updated to assert the checksummed form.
EncodeVarint(0xed01) was encoding the integer 60673, producing a 3-byte varint [0x81, 0xDA, 0x03] instead of the correct 2-byte multicodec prefix [0xed, 0x01]. The same error applied to X25519 (0xec01 → [0x81, 0xD8, 0x03]). Fix: EncodeMultibase now takes a KeyType and calls GetMulticodec() which returns the correct code point (237 for Ed25519, 236 for X25519). EncodeVarint of those values produces the correct [0xed, 0x01] / [0xec, 0x01] prefixes, matching z6Mk... / z6Ls... output from the JS ethr-did-resolver.
…ionMethod
The JS resolver uses a fall-through switch for delegates and explicit dual
assignment for attributes:
sigAuth delegate: auth[eventIndex] = id (falls through to veriKey)
signingRefs[eventIndex] = id
sigAuth attribute: auth[eventIndex] = id
signingRefs[eventIndex] = id <- was missing
Our code used else-if, making the two relationships mutually exclusive.
Fixed both the delegate and attribute paths to add sigAuth refs to both
auths and asserts. Updated the test name and assertion to match.
… present DidDocumentSerializer.ComputeContext was unconditionally adding secp256k1-2019/v1 for any EcdsaSecp256k1* VM type. For did:ethr, EthrDocumentBuilder already puts security/v2 in doc.Context (matching the spec), so both ended up in the output. Fix: skip secp256k1-2019/v1 when the document already declares security/v2. did:key and did:peer are unaffected — they don't set doc.Context themselves so the existing behaviour is preserved.
Mirrors decentralized-identity/ethr-did-resolver src/config/deployments.ts.
- KnownNetworks static class: 12 active networks (mainnet, sepolia, holesky,
gnosis, polygon, aurora, ewc, volta, artis:sigma1/tau1, polygon:test,
linea:goerli) with correct registry addresses and legacyNonce flags.
- EthereumNetworkConfig: RpcUrl now required (no silent empty default);
RegistryAddress now required; adds LegacyNonce (Phase 2 readiness).
- NetDidBuilder.AddDidEthr(IReadOnlyDictionary<string,string>) overload:
caller supplies only network name → RpcUrl pairs; registry/chainId
resolved from KnownNetworks automatically.
- Sample updated to demonstrate KnownNetworks.Sepolia with { RpcUrl = ... }
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR introduces a Phase 1 implementation of the did:ethr method for NetDid, including Create/Resolve support, ERC-1056 event-chain resolution, and supporting infrastructure (RPC client, ABI codec, document builder), plus DI wiring, a sample, and extensive tests.
Changes:
- Added
NetDid.Method.Ethrpackage with ERC-1056 ABI/event parsing, RPC client, DID resolution, and DID Document construction logic. - Added new test project covering address derivation, ABI decoding, event parsing, document building, and method-level Create/Resolve scenarios.
- Updated core model/serializer to support extension verification method properties, and added DI + sample app for
did:ethr.
Reviewed changes
Copilot reviewed 38 out of 39 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| w3c-conformance-report.md | Regenerated conformance report timestamp. |
| tests/NetDid.Method.Ethr.Tests/NetDid.Method.Ethr.Tests.csproj | New test project for did:ethr. |
| tests/NetDid.Method.Ethr.Tests/EthrDocumentBuilderTests.cs | Tests for DID Document building from ERC-1056 events. |
| tests/NetDid.Method.Ethr.Tests/EthereumAddressTests.cs | Tests for Keccak-256, address derivation, EIP-55 checksum vectors. |
| tests/NetDid.Method.Ethr.Tests/Erc1056EventParserTests.cs | Tests for log → typed-event parsing. |
| tests/NetDid.Method.Ethr.Tests/DidEthrMethodTests.cs | Integration-style tests for Create/Resolve with mocked RPC. |
| tests/NetDid.Method.Ethr.Tests/AbiDecoderTests.cs | Tests for ABI encoding/decoding helpers and event layouts. |
| tasks/todo20260522-didethr.md | Design/plan document capturing intended architecture and behavior. |
| src/NetDid.Method.Ethr/Rpc/KnownNetworks.cs | Added known ERC-1056 deployment metadata (registry addresses, chain IDs). |
| src/NetDid.Method.Ethr/Rpc/IEthereumRpcClient.cs | New RPC interface for JSON-RPC calls needed by resolver (plus Phase 2 stubs). |
| src/NetDid.Method.Ethr/Rpc/EthereumNetworkConfig.cs | New network config model (RPC URL, chain ID, registry address, legacy nonce). |
| src/NetDid.Method.Ethr/Rpc/EthereumLogFilter.cs | New eth_getLogs filter model. |
| src/NetDid.Method.Ethr/Rpc/EthereumLogEntry.cs | New eth_getLogs entry model. |
| src/NetDid.Method.Ethr/Rpc/DefaultEthereumRpcClient.cs | Default JSON-RPC client implementation over HttpClient. |
| src/NetDid.Method.Ethr/Resolution/EthrDocumentBuilder.cs | Core DID Document construction logic from ERC-1056 events. |
| src/NetDid.Method.Ethr/NetDid.Method.Ethr.csproj | New method package project file + dependencies. |
| src/NetDid.Method.Ethr/Erc1056/Erc1056Topics.cs | Keccak-derived event topics for ERC-1056 events. |
| src/NetDid.Method.Ethr/Erc1056/Erc1056Events.cs | Typed event record definitions. |
| src/NetDid.Method.Ethr/Erc1056/Erc1056EventParser.cs | Parser from raw logs to typed events using ABI decoder. |
| src/NetDid.Method.Ethr/Erc1056/Erc1056Calls.cs | Helpers for ERC-1056 read-only calldata (changed, identityOwner). |
| src/NetDid.Method.Ethr/DidEthrUpdateOptions.cs | Update options scaffold (Phase 2 shape). |
| src/NetDid.Method.Ethr/DidEthrResolveOptions.cs | Resolve options type (inherits common versioning fields). |
| src/NetDid.Method.Ethr/DidEthrMethod.cs | Main did:ethr method implementation (Create/Resolve) + event-chain walker. |
| src/NetDid.Method.Ethr/DidEthrDeactivateOptions.cs | Deactivate options scaffold (Phase 2 shape). |
| src/NetDid.Method.Ethr/DidEthrCreateOptions.cs | Create options for network selection + optional existing key. |
| src/NetDid.Method.Ethr/Crypto/EthereumIdentifier.cs | Parser for did:ethr method-specific IDs (network + addr/pubkey). |
| src/NetDid.Method.Ethr/Crypto/EthereumAddress.cs | Ethereum address derivation + EIP-55 checksumming. |
| src/NetDid.Method.Ethr/Abi/AbiEncoder.cs | Minimal ABI encoder for read-only ERC-1056 calls. |
| src/NetDid.Method.Ethr/Abi/AbiDecoder.cs | Minimal ABI decoder for event data and return values. |
| src/NetDid.Extensions.DependencyInjection/NetDidBuilder.cs | Added DI registration methods for did:ethr. |
| src/NetDid.Extensions.DependencyInjection/NetDid.Extensions.DependencyInjection.csproj | Added project reference to NetDid.Method.Ethr. |
| src/NetDid.Core/Serialization/DidDocumentSerializer.cs | Context computation tweak + serialize AdditionalProperties on VMs. |
| src/NetDid.Core/Model/VerificationMethod.cs | Added AdditionalProperties for non-standard VM fields (e.g., publicKeyHex). |
| samples/NetDid.Samples.DidEthr/Program.cs | New sample demonstrating resolve + dereference. |
| samples/NetDid.Samples.DidEthr/NetDid.Samples.DidEthr.csproj | New sample project file. |
| netdid.sln | Added new method, test, and sample projects to the solution. |
| Directory.Packages.props | Added acryptohashnet dependency version. |
| CHANGELOG.md | Documented new did:ethr method package and related additions. |
| .gitignore | Normalized comment indentation + added .idea/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Problem: AddDidEthr registered a single DefaultEthereumRpcClient with no
BaseAddress, so all networks shared one misconfigured HTTP client.
Fix:
- Add IEthereumRpcClientFactory with GetOrCreate(EthereumNetworkConfig)
- DefaultEthereumRpcClientFactory (DI path): resolves named HttpClient
'ethr-{name}' whose BaseAddress is set from network.RpcUrl in AddDidEthr
- DefaultEthereumRpcClientFactory.CreateDirect (non-DI path): each network
gets its own HttpClient built inline — used by samples / CLI tools
- DidEthrMethod constructor now takes IEthereumRpcClientFactory; resolves
the correct client per-network before every RPC call
- AddDidEthr registers one named client per network via
Services.AddHttpClient('ethr-{name}', c => c.BaseAddress = ...)
- New test: ResolveAsync_MultipleNetworks_RoutesRpcCallsToCorrectEndpoint
verifies mainnet calls never reach the sepolia RPC and vice-versa
815 tests, 0 failures, 0 warnings.
…ion tests Bug: WalkEventChainAsync took max(previousChange) across all events in a block. Later transactions in the same block emit previousChange == block.number (because changed[identity] was already updated by an earlier tx in the same block), so the walker revisited the same block indefinitely. Fix: only advance to a previousChange value that is STRICTLY less than currentBlock. This guarantees progress on every iteration. Tests added (EthrDocumentBuilderTests): - Build_DelegateAddedAndRevokedInSameBlock_NotInDocument - Build_AddTwoDelegatesRevokeOneInSameBlock_ThenAddThird_IndicesAreStable - Build_RevokeBeforeAddInSameBlock_KeyAppearsAtAddCounter Tests added (DidEthrMethodTests): - ResolveAsync_TwoEventsInSameBlock_WalkerTerminatesAndCollectsBothDelegates (uses call-count guard to detect re-entry into the same block)
Previously the filter only constrained topics[0] (event signatures), which
pulled every ERC-1056 event for ALL identities at each block visited by the
walker. On busy networks this bloats the RPC response with irrelevant logs and
shifts the server-side filtering work to the client.
Changes:
- EthereumLogFilter.Topics widened to IReadOnlyList<string[]?>? so null
entries correctly express 'match any topic at this position' per the
eth_getLogs spec.
- WalkEventChainAsync now sets topics[1] = [paddedIdentityAddress], giving
the node a precise AND-filter: event sig in {sig1,sig2,sig3} AND
indexed-identity == address.
- The identity filter guard (string.Equals check after parsing) is kept as a
defence-in-depth safeguard for nodes that do not respect topic filters.
Regression test added: ResolveAsync_EventChainWalking_FiltersLogsByIdentityAddressAtTopicsPosition1
… payloads Five explicit ArgumentException guards (PR review): 1. offsetInData out of bounds 2. pointer ulong > int.MaxValue (overflow guard) 3. pointer value exceeds data.Length 4. length ulong > int.MaxValue (overflow guard) 5. pointer + 32 + length exceeds data.Length (DoS / giant allocation guard) All five paths covered by new regression tests (AbiDecoderTests). No raw runtime exceptions can escape from untrusted event data.
…ory.CreateDirect DidEthrMethod constructor now takes IEthereumRpcClientFactory, not IEthereumRpcClient. The old snippet passed DefaultEthereumRpcClient directly which no longer compiles. Updated to match the sample project pattern.
…is:sigma1, a:b:c) Split on last ':0x' instead of first ':' so compound network names like 'artis:sigma1' are preserved correctly as the network segment. Adds EthereumIdentifierTests covering all network prefix formats.
… overflow PR feedback: silently truncating uint256 > ulong.MaxValue is a security risk — a malformed validTo could make an expired delegate appear valid. - DecodeUint256 now iterates bytes 0-23 and throws ArgumentException on any non-zero byte, with a message identifying the exact byte and value. - Class-level doc comment updated: 'throws if upper 24 bytes are non-zero'. - Two new tests: UpperBytesNonZero_ThrowsArgumentException and MaxUlong_ReturnsCorrectValue.
- Add MockEthereumRpcClientFactory + MockEthereumRpcClient to TestDidFactory - Wire did:ethr into CreateDid, CreateDidWithServices, GetMethod, CompositeResolver - Add CreateDidEthr + CreateDidEthrWithService factory helpers (service via DIDAttributeChanged event injected into mock) - AllMethods: +did:ethr in DidSubjectTests, VerificationMethodTests, VerificationRelationshipTests, DidSyntaxTests, ResolveTests, ResolutionMetadataTests, JsonProductionTests, JsonLdProductionTests - MethodsWithServices: +did:ethr in ServiceTests - NotFound switch: did:ethr uses unknown-network DID to trigger notFound - JsonLdProductionTests 6-9: generalised from Multikey-specific to 'context has >1 entry' (covers secp256k1recovery for did:ethr) - DidEthrMethod.ResolveCoreAsync: set ContentType=JsonLd + catch InvalidOperationException (unknown network) -> notFound - 217 conformance tests (+42 did:ethr rows), 0 failures
All remaining N/A entries for did:ethr are now PASS. Changes: [Fact] tests extended to record for all 4 methods instead of 2: - DidSyntaxTests: 3.1-4 (invalid DID syntax rejected) - DidUrlSyntaxTests: 3.1-9 (invalid DID URL rejected) - JsonLdProductionTests: 6-11, 6-12 (context rejection on consumption) - ResolveTests: 7.1-6 (methodNotSupported) - DereferenceTests: 7.2-8 (invalidDidUrl error) - DereferencingMetadataTests: 7.2-12 (error set on failure) AllMethods/MethodsWithServices extended with did:ethr: - DidUrlSyntaxTests: 3.1-5/6/7/8/10 (DID URL parsing) - DereferenceTests: 7.2-1/2/5/6 (fragment+bare-DID dereference) - DereferenceTests: 7.2-3/4/7/9/10 (service dereferencing) - DereferencingMetadataTests: 7.2-10/11 (dereference metadata) Correctly remaining N/A (intentional): - 4-12: JWK no-private-key (default ethr VM is blockchainAccountId) - 4-4/4-5: controller property (ethr uses blockchainAccountId, not controller) - did:key 4-20..23/7.2-3/4/7/9: did:key has no services
Two new method-level tests proving the pubkey-DID path works: - ResolveAsync_PubkeyDid_NoEvents_AddsControllerKeyVm changed()=0, expect #controller + #controllerKey (EcdsaSecp256k1VerificationKey2019 with PublicKeyJwk), both referenced in authentication and assertionMethod. - ResolveAsync_PubkeyDid_OwnerChanged_ControllerKeyAbsent Owner transfers to a different address; #controllerKey must not appear (pubkey no longer controls the DID), #controller reflects new owner.
…ance suite - Add TestDidFactory.CreateDidEthrWithPubkey(): generates secp256k1 key pair, constructs did:ethr:mainnet:0x<pubkey>, resolves it (changed=0) to get the document containing #controllerKey with publicKeyJwk. - Extend JwkDoesNotContainPrivateKeyMaterial [Fact] to also exercise did:ethr, flipping 4-12 from N/A to PASS. did:ethr conformance: 65 -> 66 statements. Only correctly-N/A entries remain: 4-4/4-5 (no top-level controller), did:peer/did:webvh 4-12 (no JWK fixtures)
moisesja
added a commit
that referenced
this pull request
Jun 13, 2026
NSec.Cryptography, NBitcoin.Secp256k1, and Nethermind.Crypto.Bls have no direct PackageReference anywhere on this branch after the NetCrypto migration — NetCrypto pulls them transitively. Removing the now-unused PackageVersion pins. did:ethr (#70) resolves NBitcoin.Secp256k1 transitively via NetDid.Core -> NetCrypto and is unaffected; if it later takes a direct reference it should re-add the pin. Co-Authored-By: Claude Opus 4.8 <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
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.
Implements the did:ethr DID method (Phase 1 - Create + Resolve), including historical resolution via versionId / versionTime. Update and Deactivate are stubbed with OperationNotSupportedException.
What's included
NetDid.Method.Ethr - new package
NetDid.Core
NetDidBuilder - two AddDidEthr overloads:
Usage