docs: credential status convention + W3C DID resolution#58
Conversation
…ention The vc-vault tracks per-VC status on-chain (verify_vc) but a third-party verifier had no standard way to discover it. Add Annex B.4 defining a W3C VC Data Model 2.0 §4.10 credentialStatus method (StellarStatusRegistryEntry): the status entry points to the vault contract + vc_id, and the verifier resolves it via a read-only verify_vc RPC call. Custom status types are explicitly permitted by §4.10. The read-only ledger lookup never notifies the issuer, satisfying the §4.10 privacy MUST against holder tracking. No contract changes.
The spec defined DID Document construction (5) but not resolution per W3C DID Resolution v0.3. Add 5.7 covering the resolve() three-tuple result, the resolution algorithm, didResolutionMetadata (contentType + error), didDocumentMetadata (created/updated/versionId/deactivated mapped from DidRecord ledger fields), and the error model with HTTP bindings (invalidDid 400, notFound 404, deactivated 410, etc.). Add the DID Resolution reference. No contract changes.
|
Warning Review limit reached
More reviews will be available in 45 minutes and 43 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds two specification sections to the Stellar DID method specification: a complete DID Resolution algorithm defining how resolvers retrieve and construct DID documents from on-chain registry data, and a Credential Status integration pattern for verifying verifiable credential revocation status through on-chain vault contracts. ChangesStellar DID Resolution and Credential Status Specifications
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
aguilar1x
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/did-spec/did-stellar-v0.1.md (1)
387-389: 💤 Low valueConsider clarifying "successful resolution" vs HTTP 410 for deactivated DIDs.
The text states that a deactivated DID results in "a successful resolution (no
error)" but then notes "an HTTP binding MUST map it to410 Gone." This juxtaposition might confuse readers about whether the resolution succeeded or failed.Consider rephrasing to clarify that the DID Resolution protocol-level result is successful (document returned, no error in metadata) but the HTTP status reflects the deactivated state:
✏️ Optional clarification
- Document (Annex A.3) with `didDocumentMetadata.deactivated = true`. This is a - successful resolution (no `error`), but an HTTP binding MUST map it to `410 Gone`. + Document (Annex A.3) with `didDocumentMetadata.deactivated = true`. At the DID + Resolution protocol level this is successful (no `error` in `didResolutionMetadata`), + but HTTP bindings MUST return `410 Gone` to reflect the deactivated state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/did-spec/did-stellar-v0.1.md` around lines 387 - 389, Clarify the wording so readers understand protocol vs transport: state that when DidRecord.deactivated == true the DID Resolution operation itself is considered successful (it returns the tombstone DID Document with didDocumentMetadata.deactivated = true and no protocol-level `error`), but an HTTP binding MUST translate that protocol-level successful resolution into an HTTP 410 Gone response; update the sentence to explicitly mention "resolution result" vs "HTTP transport status" and include the symbols DidRecord.deactivated, didDocumentMetadata.deactivated, and HTTP 410 in the revised text.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/did-spec/did-stellar-v0.1.md`:
- Line 820: The table's `id` row incorrectly references this spec's §4.4; update
the text for the `id` field to point to the correct external authority (e.g.,
"MUST be a single URL (see W3C VC Data Model 2.0 §4.4)" or similar), or remove
the intra-spec section marker and replace it with a clear citation to the W3C VC
Data Model section about credential identifiers; ensure the change is applied to
the `id` table cell text so the `id` field reference is unambiguous.
- Around line 403-404: The spec's description for created/updated (fields
`created`/`updated` mapping
`DidRecord.created_ledger`/`DidRecord.updated_ledger`) is ambiguous about which
Stellar API and field to use and how to convert to ISO 8601 UTC; update the text
to explicitly state either (a) use Horizon's ledger endpoint and its `closed_at`
ISO 8601 field (no conversion required), or (b) if using the RPC `getLedgers`
method, read the `ledgerCloseTime` Unix timestamp and convert it to an ISO 8601
UTC string (e.g., use UTC epoch seconds -> ISO 8601), and include that clear
method/field and conversion rule for `created` and `updated`.
---
Nitpick comments:
In `@docs/did-spec/did-stellar-v0.1.md`:
- Around line 387-389: Clarify the wording so readers understand protocol vs
transport: state that when DidRecord.deactivated == true the DID Resolution
operation itself is considered successful (it returns the tombstone DID Document
with didDocumentMetadata.deactivated = true and no protocol-level `error`), but
an HTTP binding MUST translate that protocol-level successful resolution into an
HTTP 410 Gone response; update the sentence to explicitly mention "resolution
result" vs "HTTP transport status" and include the symbols
DidRecord.deactivated, didDocumentMetadata.deactivated, and HTTP 410 in the
revised text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 079def65-8354-434e-8511-26ae67940f76
📒 Files selected for processing (1)
docs/did-spec/did-stellar-v0.1.md
| | `created` | `DidRecord.created_ledger` | The resolver maps the ledger sequence to its close time (ISO 8601 UTC) via Stellar RPC. | | ||
| | `updated` | `DidRecord.updated_ledger` | Same mapping. Equals `created` until the first mutation. | |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify Stellar RPC provides ledger close time mapping
# Check if Stellar documentation describes ledger close time retrieval
rg -i "close.*time|ledger.*timestamp" --type md -C3Repository: ACTA-Team/contracts-acta
Length of output: 763
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="docs/did-spec/did-stellar-v0.1.md"
# Show the exact section around the cited lines
sed -n '380,430p' "$FILE" | nl -ba
echo "----"
# See if the spec already mentions any specific Stellar RPC/Horizon method names (e.g., getLedger)
rg -n --hidden --no-ignore-vcs "getLedger|ledger.*close|close.*time|horizon|rpc" "$FILE"Repository: ACTA-Team/contracts-acta
Length of output: 111
🌐 Web query:
Stellar Horizon API getLedger ledger close time field name
💡 Result:
In the Stellar Horizon API, the field name for the ledger close time is closed_at [1][2][3]. This field returns an ISO 8601 formatted string indicating the time when the specific ledger was closed and committed to the network [1][4][2]. Note: This field name is specific to the Horizon API [2]. If you are using the Stellar RPC API (often used for Soroban and other low-level network interactions), the equivalent field is typically named ledgerCloseTime [5].
Citations:
- 1: https://docs.stellarcn.org/horizon/reference/resources/ledger.html
- 2: https://developers.stellar.org/docs/data/apis/horizon/api-reference/resources/ledgers/object
- 3: https://github.com/stellar/go/blob/master/protocols/horizon/main.go
- 4: https://stellar-docs.overcat.me/horizon/reference/resources/ledger.html
- 5: https://developers.stellar.org/docs/data/apis/rpc/api-reference/methods/getLedgers
🌐 Web query:
Stellar RPC getLedger returns ledger closed time timestamp
💡 Result:
In the Stellar RPC API, specifically for the getLedgers method, the ledger closed time is returned as a Unix timestamp [1][2][3]. For an individual ledger object within the getLedgers response, this field is named ledgerCloseTime and is returned as a number (Unix timestamp) [1][4], though in some implementation documentation it may be noted as a string or a 64-bit integer [2]. Additionally, the top-level getLedgers response includes fields for the latest and oldest ledgers, which use the following timestamp formats: - latestLedgerCloseTime: A Unix timestamp representing the close time of the latest ledger [1][5]. - oldestLedgerCloseTime: A Unix timestamp representing the close time of the oldest ledger ingested by the RPC node [1][5]. Note that this differs from the Stellar Horizon API, which typically returns closed_at timestamps in ISO-8601 string format [6]. If you are working with other RPC methods like getEvents, you may encounter ledgerClosedAt as an ISO-8601 string [7], so it is recommended to verify the specific method documentation as formats can vary across the RPC interface [8].
Citations:
- 1: https://www.alchemy.com/docs/chains/stellar/stellar-api-endpoints/get-ledgers.md
- 2: Add
getLedgersimplementation stellar/stellar-rpc#303 - 3: https://developers.stellar.org/docs/data/apis/rpc/api-reference/methods/getLedgers
- 4: https://docs.validationcloud.io/v1/stellar/stellar-rpc-formerly-soroban-api/getledgers
- 5: https://docs.rs/stellar-rpc-client/latest/stellar_rpc_client/struct.GetLedgersResponse.html
- 6: https://stellar-docs.overcat.me/horizon/reference/resources/ledger.html
- 7: https://developers.stellar.org/docs/data/apis/rpc/api-reference/methods/getEvents
- 8: https://developers.stellar.org/docs/data/apis/rpc/api-reference/methods/getTransactions
Clarify ledger-to-timestamp mapping for created/updated.
The spec says created/updated map DidRecord.*_ledger to ISO 8601 UTC “via Stellar RPC” but doesn’t name the method/field or the required timestamp conversion. Stellar Horizon exposes ledger close time as closed_at (ISO 8601), while Stellar RPC (e.g., getLedgers) returns ledgerCloseTime as a Unix timestamp—so the spec should either (a) reference the Horizon endpoint/closed_at, or (b) name the exact RPC method/field and state the conversion to ISO 8601 UTC. (docs/did-spec/did-stellar-v0.1.md:403-404)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/did-spec/did-stellar-v0.1.md` around lines 403 - 404, The spec's
description for created/updated (fields `created`/`updated` mapping
`DidRecord.created_ledger`/`DidRecord.updated_ledger`) is ambiguous about which
Stellar API and field to use and how to convert to ISO 8601 UTC; update the text
to explicitly state either (a) use Horizon's ledger endpoint and its `closed_at`
ISO 8601 field (no conversion required), or (b) if using the RPC `getLedgers`
method, read the `ledgerCloseTime` Unix timestamp and convert it to an ISO 8601
UTC string (e.g., use UTC epoch seconds -> ISO 8601), and include that clear
method/field and conversion rule for `created` and `updated`.
| | Property | Requirement | Meaning | | ||
| |---|---|---| | ||
| | `type` | MUST | Fixed `"StellarStatusRegistryEntry"`. (Per §4.10, `type` is REQUIRED.) | | ||
| | `id` | MAY | If present, MUST be a single URL (§4.4). Optional human/resolver endpoint. | |
There was a problem hiding this comment.
Clarify the section reference for the id field.
The table states that the optional id field "MUST be a single URL (§4.4)," but §4.4 in this document refers to "Mutation Semantics" in the Contract Operations section, which is unrelated to credential status.
This reference likely refers to W3C VC Data Model 2.0 §4.4 (or a similar section about credential identifiers), not this specification's §4.4.
📝 Proposed clarification
-| `id` | MAY | If present, MUST be a single URL (§4.4). Optional human/resolver endpoint. |
+| `id` | MAY | If present, MUST be a single URL (W3C VC Data Model §4.4). Optional human/resolver endpoint. |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | `id` | MAY | If present, MUST be a single URL (§4.4). Optional human/resolver endpoint. | | |
| | `id` | MAY | If present, MUST be a single URL (W3C VC Data Model §4.4). Optional human/resolver endpoint. | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/did-spec/did-stellar-v0.1.md` at line 820, The table's `id` row
incorrectly references this spec's §4.4; update the text for the `id` field to
point to the correct external authority (e.g., "MUST be a single URL (see W3C VC
Data Model 2.0 §4.4)" or similar), or remove the intra-spec section marker and
replace it with a clear citation to the W3C VC Data Model section about
credential identifiers; ensure the change is applied to the `id` table cell text
so the `id` field reference is unambiguous.
- Pin challenge_jcs_hex (canonical RFC 8785 form) in vectors 5-6 so every implementation MUST canonicalize to exactly these bytes before signing. The Ed25519 signatures still await the off-chain reference impl. - Add vector 7 (multi-key: exercises #auth-2 index numbering). - Add vectors 8-9 (DID Resolution errors: notFound 404 / invalidDid 400) per the new 5.7 resolution section. Closes the vector coverage gaps from the audit (no contract changes).
DID Core 1.1 §7.3 requires documenting the RFC 3552 attack forms; §7.4 requires discussing each applicable RFC 6973 §5 privacy category. The spec previously covered only replay (security) and correlation/disclosure (privacy). - 7.9 Attack Surface: enumerate all 8 mandated attack forms (eavesdropping, replay, message insertion/deletion/modification, DoS, amplification, MITM) with method-specific treatment. - 7.10 Operation integrity, authentication, unique assignment (DidAlreadyExists + 128-bit id), and which data is cryptographically protected. - 7.11 Verifiable-data-registry / DLT trust assumptions and residual risk. - 8.6 RFC 6973 coverage: all 9 categories mapped (surveillance, stored-data compromise, unsolicited traffic, misattribution, correlation, identification, secondary use, disclosure, exclusion). - Fix the §7/§8 citations to the normative §7.3/§7.4 titles. No contract changes.
1 participant
Closes two W3C-conformance gaps in the
did:stellarspec found in the contract/spec audit. No contract changes — both are spec/convention only.1. Credential Status (Annex B.4) —
StellarStatusRegistryEntryThe
vc-vaulttracks per-VC status on-chain (verify_vc), but a third-party verifier had no standard way to discover it. Adds a W3C VC Data Model 2.0 §4.10credentialStatusmethod:The verifier resolves it via a read-only
verify_vc(vcId)RPC call (Valid/Revoked/Invalid). Custom status types are explicitly permitted by §4.10, and the read-only ledger lookup never notifies the issuer — satisfying the §4.10 privacy MUST against holder tracking.2. DID Resolution (§5.7)
The spec defined DID Document construction (§5) but not resolution per W3C DID Resolution v0.3. Adds §5.7:
resolve()three-tuple result + resolution algorithmdidResolutionMetadata(contentType,error)didDocumentMetadata(created/updated/versionId/deactivatedmapped fromDidRecordledger fields)invalidDid400,notFound404, deactivated 410,representationNotSupported406, …)Both close gaps documented in the audit; the resolver/verifier that consume these conventions remain off-chain work.
Summary by CodeRabbit
Release Notes