Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No longer preserve vault ID on cloneVault #253

Closed
joshuakarp opened this issue Oct 20, 2021 · 7 comments · Fixed by #266
Closed

No longer preserve vault ID on cloneVault #253

joshuakarp opened this issue Oct 20, 2021 · 7 comments · Fixed by #266
Assignees
Labels
development Standard development r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management

Comments

@joshuakarp
Copy link
Contributor

Specification

Before we had the concept of a remote field on a vault, we were intending to preserve the vault ID of a cloned vault, such that they retain their unique identity in the global Polykey network. However, now that we're using the remote field for a vault, we can remove this.

For example, if a node A has a vault with ID id1 and shares this vault with B, when cloning, the local, cloned version on B will have a completely different ID.

Recall that a cloned vault is intended to be immutable. We've had discussions on what the presence of a remote field actually indicates. If a remote field is set (i.e. not undefined or missing), then this will indicate that a vault is a cloned/synchronised vault (i.e. originally coming from some other node). Therefore, any vault with a remote field set should be immutable.

Additional context

Tasks

  1. Remove the preservation of vault ID on clone
  2. Set a new vault ID on clone
  3. Set the remote field to be nodeID + vaultID
  4. Make the vault immutable
@joshuakarp joshuakarp added the development Standard development label Oct 20, 2021
@scottmmorris
Copy link
Contributor

This is mostly already being done inside of 266, the remote node and vault are stored in the db as metadata to the vault.

However, the cloned vaults are not yet immutable. This should be a simple case of maybe just setting a property on each vault (if it is cloned) and then checking that if a commit is trying to be made. I'll make these changes on 266 as well unless anyone has any objections

@joshuakarp
Copy link
Contributor Author

@scottmmorris I remember we briefly mentioned whether we could make use of a remote field to determine whether the vault was immutable. i.e. if the remote field is empty, then it's not a cloned vault. Otherwise, if there is a value in the remote field, then we know it's been cloned from somewhere, and therefore it's immutable. Is this the process, or is there an easier solution?

@scottmmorris
Copy link
Contributor

Currently the remote field is purely stored in the db as metadata with no in memory state for the remote. So it would mean having to reading from the db every time you want to make a commit to check if the remote field is set. I can't see a better way around this then to just have a property that is set each time a vault is created just by reading the db then and so then it wont have to be done for the remainder of the lifetime of that vault. Is that what is meant by having a remote field?

class VaultInternal {
    protected remote: bool;
    constructor() {
        // if db entry exists set remote
    }
}

Or alternatively the remote could be stored like this to save having to read the db each time:

type Remote {
    node: NodeId;
    vault: VaultId;
}

class VaultInternal {
    protected remote: Remote;
    constructor() {
        // if db entry exists load node and vault into remote
    }
}

Also I have made it so the remote details are only mutatable by cloning and pulling, so no direct interfacing with the remote fields is done by the user. This wasn't explicitly mentioned in the spec but I think that makes sense considering the only reason for now for changing the remote of a vault would be to pull from there.

@joshuakarp
Copy link
Contributor Author

Is there any benefit gained from having an explicit Remote type that specifies the NodeId and VaultId that it's pulled from? Would this be used anywhere?

@scottmmorris
Copy link
Contributor

Yeah it just saves you having to read the db multiple times throughout the lifespan of a vault. Say you pulled twice to a vault and then tried to write a commit on the vault:

pull1: Loads in the remote from the db

pull2: Loads in remote from memory (would need to read from db otherwise)

commit: Checks the remote in memory and doesn't allow commit (would need to read from db otherwise)

Pull , clone and commit would use that remote property

@joshuakarp
Copy link
Contributor Author

That seems like the best solution to me then. By default, I'd expect the remote field to then be undefined, and then for now, an isMutable function could simply be added that returns true if the field is undefined.

@joshuakarp
Copy link
Contributor Author

Will be fixed by #266

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management
3 participants