Implementation Review PR

[flea89]

Created this PR to capture initial feedback on UCAN library implementation

[adamalton]

I've read through all of this, and it all looks great. I just have one suggestion (which may already be in the pipeline), which is that the logic in semantics.js could probably be made generic for any set of hierarchical actions. So it could be turned into something where maybe instead of a storageSemantics class, there could just be a generic semantics class. So you'd have:

const storageActions = {
  'upload/': {
    '*': {
      'IMPORT',
    }
  }
}

And then you could pass that into the generic semantics class like:

const storageSemantics = sematics(storageActions);

and the tryParsing and tryDelegating methods (and therefore that whole class) then become completely generic and can be used for other cases as well.

You've probably already thought of this, but it's my main suggestion for improvement at the mo.

[adamalton]

I've just read the documentation additions to the README, which look great. Reading that, it's given me a thought, which is that it would be nice to allow the CLI commands to accept the private key as a path to a file, so that (1) the user doesn't have to go through the step of copying the value out of their private key file, and (2) their private key isn't stored in their shell history.

So, e.g. ucan-storage keypair --from <private-key> could have an alternative usage of ucan-storage keypair --from-file <path-to-private-key-file>.
and similarly ucan-storage ucan --issuer <your-private-key> could have an alternative usage of ucan-storage ucan --issuer-file <path-to-your-private-key>.

The user could just pass the key file contents as an argument themselves using some shell trickery. And there's also the complication that if we implement it then we have to consider the possibility of the key file being encrypted. So maybe it's not worth it. But it might be nice to have this option as a convenience and to encourage a more secure usage.

[hugomrdias]

I've just read the documentation additions to the README, which look great. Reading that, it's given me a thought, which is that it would be nice to allow the CLI commands to accept the private key as a path to a file, so that (1) the user doesn't have to go through the step of copying the value out of their private key file, and (2) their private key isn't stored in their shell history.

So, e.g. ucan-storage keypair --from <private-key> could have an alternative usage of ucan-storage keypair --from-file <path-to-private-key-file>. and similarly ucan-storage ucan --issuer <your-private-key> could have an alternative usage of ucan-storage ucan --issuer-file <path-to-your-private-key>.

The user could just pass the key file contents as an argument themselves using some shell trickery. And there's also the complication that if we implement it then we have to consider the possibility of the key file being encrypted. So maybe it's not worth it. But it might be nice to have this option as a convenience and to encourage a more secure usage.

already made an issue about this please check it out and add feedback there #11