[DONE] How can we access advanced functionality in the library?

[eike-hass]

Currently the IOTA Identity framework has two different public API's: low-level-api and the account. The low-level-api is designed to give developers complete freedom at the cost of complexity. It is completely stateless, meaning that developers have a large responsibility to keep track of the state of an identity and its storage. The account is the opposite and a perfect entry point for new developers. The API protects developers from making common mistakes and provides a much easier API, at the cost of flexibility. It is stateful, therefore the developer has to take care of much less problems. The goal was that 90%+ of the use cases that have simple Identity requirements would do fine with just the account.

This discussion is about a trapdoor design (coined by Dave de Fijter), where low-level-api capabilities are exposed in the account API through a certain mechanism (a hidden door to a larger API). It would allow more flexibility to the account, but we are looking for a way to introduce the flexibility, but not at the cost of complexity. The team currently favours the idea of hiding the more advanced API behind some sort of mechanism. This discussion is about how a developer could access the more flexible yet dangerous functionalities, but new developers will understand that these functionalities are dangerous and not intended for them.

This might even allow the low-level-api to be retired as a public API and moved to an internal API, reducing the complexity of the framework on first glance. That is being discussed in #398.

Motivation

We want to enable users of the library to access advanced functionality, that only a small percentage of users would need, without complicating the API for the remaining users.

Proposal

1. Naming convention

Have the methods for the advanced functionality co-located with the other methods. The nature of the advanced functions can described by using keywords like unchecked or dangerous or characters like _ in the method name. Those "markers" can be adapted to different language targets in the bindings, e.g. the method uncheckedSetIssuer in Rust could be converted to dangerouslySetIssuer in WASM/JS.

For example:

//API description: Advanced functionality, only use if you know what you are doing. Sets the previous message id... blablabla
account.update_identity()
 .previous_message_id_unchecked(id);
 .apply()
 .await?;

2. Mutable DID-Document

Supply one method to return a DID or VC/VP document each and let advanced users directly mutate the document and commit the changes. This opens all possibilities of the documents, but might allow advanced users to bring the documents in a state, that is not supported by the more user friendly API methods.

let mut document = account.document_mut();
document.prevMsgId = id; 
client.publish_json(document);

3. Advanced / Elevated API

In the chain of a builder pattern have a command like .advanced() or .elevate(), that returns a new builder that contains advanced methods like mentioned in proposal 1, but with clean names.

Option 3.1

account.update_identity()
 .advanced()
 .previous_message_id(id)
 .apply()
 .await?;

Option 3.2

A similar approach, including a naming convention, would be:

account.update_identity_unchecked()
 .previous_message_id(id)
 .apply()
 .await?;

Edit: @JelleMillenaar edited the Discussion with intro and examples.
Edit: @PhilippGackstatter made the Rust examples more idiomatic and added 3.2.

[PhilippGackstatter]

Option 1 is fine, but I don't like that it pollutes the API for regular/most users. Also, we will have to decide what dangerous functionality to expose, which might not allow for as much flexibility as being able to freely mutate a document.

The second option basically means we have to make all fields public (or add setters and getters). That's pretty straightforward while allowing for maximum flexibility. To avoid API pollution, I would hide all setters and getters behind a different type, i.e. UncheckedIotaDocument. This will also easily translate to any language, since setters/getters are idiomatic everywhere (afaik). The downside here might be that we're really not giving the users anything. Maybe it's useful to have certain functionality be unchecked but still provide more than just setting or getting values.

I also like the third option, but we will again have to decide what to expose.

[cycraig]

Option 1

This requires that we have functions for all such operations, which is not always the case. While some structs such as IotaDocument already have methods like from_authentication_unchecked(), the Account in particular only exposes a subset of safe functionality with the update commands. I would not be in favour of adding unchecked/dangerous functions to it just for the sake of exposing such functionality unless we want to refactor the entire Account API to use them internally instead (which is a possibility).

Option 2

This would be my preference as it exposes the full functionality that the low-level API allows (arbitrary document mutations via as_document_mut()) but the exact implementation is unclear. If the low-level client API has to be used to publish the document, the Account needs to be able to synchronise with the updated document from the Tangle. The Account could instead be refactored to store an IotaDocument internally instead (or inside) of an IdentityState, so any mutations could still be published using the Account API, or the IdentityState itself could be made unsafely mutable. The latter only allows unsafe mutation of the fields an IdentityState tracks already, which doesn't seem to encompass everything an IotaDocument could do?

Option 3

This is similar to Option 1 in my opinion in that we essentially need to create duplicate functions to expose the full suite of unsafe functionality. I personally haven't seen this pattern in the wild. I think the one advantage it has over Option 1 is that it clearly delineates the separation between the safe and unsafe API: a developer has to call a single function with intention to even see the unsafe functionality, so API pollution is less of an issue.

tl;dr I like Option 2 the most currently; Option 1 and Option 3 are very similar but the clear API separation may make Option 3 slightly more preferable.

[cycraig]

I guess that's part of the decision too: do we want to expose a fully unsafe/unchecked API where there are no guarantees or is it really an "advanced/extended" API where you can do things that are only slightly unsupported but still feasible within the confines of the framework (like messing with previousMessageId).

The Account could always validate that the DID document would still pass verification before publishing it after performing "unchecked" updates locally, and we could expose a publish_unchecked() method to bypass that. I feel like this affects all three of the options depending on what the unchecked/advanced API allows.

It's worth considering just keeping the low-level API as-is and only allowing supported operations using the Account. The Client could then remain public to allow any "dangerous" operations, we would just drop the examples to discourage usage. This would only really work if the Account has sufficient functionality to support more than "90%+" of use-cases as stated.

[JelleMillenaar]

This should provide freedom, with the added risk, so something like publish_unchecked is probably a good idea. I wouldn't do checks as that again restricts.

[PhilippGackstatter]

I like the second option you presented. It seems like a good way to implement that would be to actually keep the low-level API, but make the Account more compatible with it, including tings like the IdentityState -> IotaDocument refactor. If possible, it'd be great to be able to mutate an account-managed document freely, but have it continue to be managed by the account using a verification + publication mechanism. Something like:

let mut document: IotaDocument = account.as_document_mut();
document.set_previous_message_id("xyz");
// Verifies the document + publishes the update to the tangle (if autopublish == true)
// This mirrors the update_identity functionality, but with an arbitrary document
account.update_document(document).await?;
// Now we can continue to update it with the account itself
account.update_identity().create_method().fragment("key-1").apply().await?;
// I'm also allowed to make stupid updates
document.set_previous_message_id("000000000000");
// This would probably fail verification, so we need to use the unchecked method
// I'm not 100% sure if this would work in the account or if it would break it
// but it should basically act like publishing an arbitrary document (Client::publish_document)
account.update_document_unchecked(document).await?;

Regardless of if the unchecked method is doable in that way, if the only reason we need to keep the Client around is for publish_unchecked and a few other methods, we can consider adding those to the account directly and make the Client private.

[JelleMillenaar]

I'll be honest, I didn't expect that option to gain the most support, but that is looking good. Is document the low-level-api or is it just a JSON object OR object instance that can be manipulated in any way?

Perhaps to extend the discussion, could we somehow add support to decide if the document update is a diff or integration update? If we do that, we are already pretty close towards having all low-level-api functionality available in account.

[PhilippGackstatter]

I would not just expose a json object and offer a publish_json method, as that provides no help to the developer. With the low-level API we already have, the common operations are possible, even if dangerous, but it's still a better interface than mutating a json value directly. Also, if we take an arbitrary json object, every time we call update_document we need to parse and validate it, which seems like an unnecessary overhead given the available alternatives.

I think whether a document update is a diff or integration message can just be an additional argument in update_document or a separate method - can't think of a good name right now, though.

[JelleMillenaar]

Issue opened to implement Option 2: #433