[Task] Implement key deletion in the `Account`

[PhilippGackstatter]

Description

The Storage::key_del function can be used to delete the key belonging to a verification method, but it is currently not used from the Account. The task is to decide when to delete the key material of a verification method, and implement it.

Motivation

The key material of deleted verification methods does not need to be retained, and so it should be deleted to keep the storage clean.

When to delete keys?

In theory, keys can be deleted as soon as the verification method has been deleted. One caveat comes for methods that are used to sign updates. Rotating DID document signing keys means deleting an old method from the document, but the old method is still required to sign the update that removes itself. Thus, deleting keys immediately when processing Update::DeleteMethod is not possible in general.

In theory, publishing waits for milestone confirmation, so it is safe to delete keys afterwards.

One approach (I believe suggested by @cycraig) was to compare (diff?) the old and new document during publishing, and if publishing is successful, attempt to delete all keys that were removed. "Attempt", because in a distributed setup, not all verification methods may actually have local key material. Considering distributed setups, keys also need to be removed during Account::fetch_document, because methods may have been removed from a different machine. A nice feature of this approach is that it should not require additional state to be kept.

This approach may or may not have to be discussed some more.

To-do list

Create a task-specific to-do list. Please link PRs that match the To-do list item behind the item after it has been submitted.

• Implement key deletion in the account.

Change checklist

Add an x to the boxes that are relevant to your changes, and delete any items that are not.

• The feature or fix is implemented in Rust and across all bindings whereas possible.
• The feature or fix has sufficient testing coverage
• All tests and examples build and run locally as expected
• Every piece of code has been document according to the documentation guidelines.
• If conceptual documentation (mdbook) and examples highlighting the feature exist, they are properly updated.
• If the feature is not currently documented, a documentation task Issue has been opened to address this.

[cycraig]

compare (diff?) the old and new document during publishing, and if publishing is successful, attempt to delete all keys that were removed.

Indeed that's one of the approaches we could use. I think I raised that we should only attempt that after resolving to ensure the latest state does not contain the keys (only resolving really confirms that a published update was successful), and resolving might be something we want to do for that reason anyway.

However I believe we should never automatically delete or remove private keys from storage. Users may be temporarily removing a verification method in order to re-add it with different verification method relationships---this is actually a use-case we do not explicitly support since the Account rejects adding verification method relationships to embedded methods, so we say to remove and re-add the method as a workaround which is difficult since the private key is not extractable/movable in storage by end-developers using the Account alone. That's a separate issue, however, and may work with the KeyLocation refactor incidentally anyway. The second reason is that those private keys may be important and removing a verification method could be done by mistake.

I think we should have a method similar to purge---purge_keys perhaps---that has to be manually called by a developer, and that deletes any keys in storage that are not linked to any verification methods in the current or previous state of the DID Document.

[PhilippGackstatter]

We discussed this more in yesterday's weekly.

Key Deletion

Publishing currently can wait for milestone confirmation or not, depending on user-supplied options, but only when using the Client directly. The Account uses the default, which is to wait for milestone confirmation. In my opinion, the Account should always wait for milestone confirmation. It's fine to give this control to lower-level uses, but the Account manages keys and so it needs to work with all the guarantees it can get. Since all the operations are async, users can easily poll it in the background if it takes too long.

There are at least two approaches for key deletion:

1. After a milestone-confirmed update (e.g. one that rotates signing keys) we could delete the key of the removed verification method automatically, since a milestone is a proper confirmation that the message was included. Unless the used node lies, this should be a pretty safe solution.

2. After a milestone-confirmed publication, we mark the key for deletion in some auxiliary data structure. Users can manually call Account::purge_unused_keys (or similar) to delete those marked keys for good. The Account can maintain that data structure without requiring every Storage implementation to take care of it. This is an even safer alternative than 1., that never automatically deletes keys and puts the responsibility (or burden) on the user. Depending on the value of the identity, users may choose to check with multiple nodes (quorum) to decide whether an update was published correctly and to increase confidence in that outcome. Note that the marked keys can continue to be used, because Storage is unaware of these markers. If a message was not published correctly, for whatever reason, users can attempt another publication because the key is still in storage (as long as they did not call purge_unused_keys).
   This solution should enable @cycraig's point about re-adding verification methods. Removing a method marks the key for deletion, but adding a method with an existing key location (not currently part of the API, but could be) could unmark the key for deletion.

Account DID Deactivation

Related, see #851.