search

decentralized-identity / sidetree

Sidetree Specification and Reference Implementation
https://identity.foundation/sidetree/spec
Apache License 2.0
437 stars 112 forks source link

Support ietf-json-patch #1197

Closed tonyimpervious closed 2 years ago

tonyimpervious commented 2 years ago

The spec says ietf-json-patch is supported.

https://github.com/decentralized-identity/sidetree/blob/bcc0cdb0f9263d15a95825faf94a7cb2240a05ba/docs/spec/patches.md#ietf-json-patch

However it is not.

https://github.com/decentralized-identity/sidetree/blob/151be6f44435c139906bf68bd02b60c89e734e6a/lib/core/versions/latest/DocumentComposer.ts#L183-L204

I'm trying to do this with ION right now but it seems the error I'm getting is from the above. I'd rather not have to write a custom implementation around adding/removing services when ietf-json-patch was supposed to solve this problem in the DID world.

csuwildcat commented 2 years ago

The Patch section lays out optional patch types that an implementer may incorporate. ION, being one implementation of Sidetree, does not implement that optional patch type for reasons related to verification and performance and complexity reasons. If you implement the JSON Patch option for deltas, you need to validate every transformation, and that's very hard to do statically. You'd effectively need to apply a patch then inspect the changed values to see if the output document is valid.

tonyimpervious commented 2 years ago

Gotcha. It's not documented anywhere what ION implements or not, and besides that, there's ION and then there's this Sidetree implementation that also doesn't support it, so I'll close this as won't-fix I guess. I don't know what the point is of listing something in the spec that someone may do but nobody wants to support for complexity reasons.

csuwildcat commented 2 years ago

@tonyimpervious I would have left that patch type out, but there was an Ethereum-based method that implemented it and participated in advocating for its inclusion.