Feat/integrate comms

[elenaf9]

Description of change

Refactor communication actor and integrate it into the stronghold interface.

Type of change

• Enhancement (a non-breaking change which adds functionality)

How the change has been tested

Added interface tests to read/ write a remote stronghold store.

Change checklist

• I have followed the contribution guidelines for this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[nothingismagick]

@lucasfernog and @tensor-programming - would love to have you both do an in depth review of this PR, from the perspective of your respective sides of this actor stack.

[elenaf9]

I am right now doing some refactoring, but I will soon be done with that and then also mark the PR's as ready for review.

[lucasfernog]

Please tag me when this is ready :)

[elenaf9]

@lucasfernog @tensor-programming the PR is ready for review, but the following aspects are not implemented yet / need to be discussed afterwards:

• what methods should exists in the stronghold interface (currently I only added write_remote_vault, read_remote_store and write_remote_store)
• the firewall actor is at the moment just a dummy actor that accepts all requests, eventually it should be provided by the user
• my changes depends on this PR to riker, right now I've set my fork of riker as dependency for development purposes.

[lucasfernog]

can you rename these b variables to vault_exists and record_exists?

[lucasfernog]

also, should this whole logic be a core stronghold message? like a EnsureVaultExists and EnsureRecordExists - maybe this is useful for consumers too

[elenaf9]

I copied this logical sequence of the messages from the interface for targeting the local client, so this question also concerns those methods. But yes I can imagine that a user might need these dedicated message types when they don't want to automatically create a new vault if the vault path doesn't exist, especially with remote strongholds but also in a local system.
@tensor-programming what do you think about this?

[tensor-programming]

I have to wonder if we should even allow a user to create a remote vault. Or if there should be some kind of policy for restricting that kind of behavior. Ideally, we would want each user to have access to a select number of vaults based on which paths they know and haven't really considered how we would restrict users inside of a shared stronghold. Perhaps, its something to think about later.

For now it should be ok to just enable the ability to add/create a new vault remotely if the vault doesn't exist and we can restrict/change that behavior later using policies.

[elenaf9]

Well, i think this type of policy that you are talking about Tensor could maybe be decided by the Firewall, so the Firewall can be configured for each stronghold individually to allow or reject a SHRequest::CreateNewVault?

[tensor-programming]

Most of this looks good.

[elenaf9]

The tests currently fail until this PR is merged.

[tensor-programming]

Looks good. Just a couple of questions and minor things really.

One more thing, there are a ton of unwraps in the code that kind of make me uneasy. I know that its kind of hard to deal with these especially when you are implementing an interface or working in a closure. The problem is that theses unwraps will cause panics if the error happens (in most cases these are options). In production that's not necessarily what we want to happen, in other words, some of them should be recoverable errors if possible. I would recommend going back over each of the unwrap cases to make sure that they are absolutely necessary. If you can't remove the unwrap, it might be better to use expect because at least you give the user some information.