# Remote signing interface

- Define a tuple type `CryptoPublic` such as:

/// The name `Public` is already defined an interface
/// I have to find a better name for this.
/// Any suggestions?
struct CryptoPublic(CryptoTypeId, PublicKey)

- [ben] Do we want a new struct for that? Isn't a type-alias enough?

type CryptoPublic = (CryptoTypeId, PublicKey);

- Introduce `get_supported_keys`:

fn get_supported_keys(
    id: KeyTypeId, 
    keys: Vec<CryptoPublic>
) -> Result<Vec<CryptoPublic>>;

- `sign_with` will receive a public key and return the signature

/// Not the final type but just an illustration
type SignatureValue = Vec<u8>;
fn sign_with(
    &self, 
    id: KeyTypeId, 
    public: CryptoPublic, 
    msg: &[u8]
) -> Result<Signature, String>;

- Define three additional helper functions in `trait` that future http implementations can then provide optimized implementations for.

/// DO NOT IMPLEMENT. See `sign_with_any` suggested by ToDr
// fn sign_with_first(
//     &self, 
//     id: KeyTypeId, 
//     keys: Vec<CryptoPublic>, 
//     msg: &[u8]
// ) -> Result<SignatureVal, String> {
//     if pubs.len() == 1 {
//       self.sign_with(id, pubs.0, msg)
//     } else {
//         for k in self.get_supported_keys(id, pubs) {
//             if let Ok(sign) = self.sign_with(id, k, msg) {
//                 return Ok(sign);
//             }
//         }
//     }
//     return Err("Could not sign with any of the given keys")
// }
// [ToDr] ^^^ I'd rename to `sign_with_any` and I think returning the actual key that was used to produce signature would be a good idea (i.e `Result<(CryptoPublic, Sign), String>)
fn sign_with_any(
    &self, 
    id: KeyTypeId, 
    keys: Vec<CryptoPublic>, 
    msg: &[u8]
) -> Result<(Public, Signature), String> {
    if pubs.len() == 1 {
      self.sign_with(id, pubs.0, msg).map(|s| (pubs.0, s))
    } else {
        for k in self.get_supported_keys(id, pubs) {
            if let Ok(sign) = self.sign_with(id, k, msg) {
                return Ok((k, sign));
            }
        }
    }
    return Err("Could not sign with any of the given keys")
}

fn sign_with_all(
    &self, 
    id: KeyTypeId, 
    keys: Vec<CryptoPublic>, 
    msg: &[u8]
) -> Vec<Option<SignatureVal>> { // [ben] also minor change in the types here, because that is more useful.
    pubs.map(|p| self.sign_with(id, p, msg).ok()).collect()
}

[ToDr] I still feel that it might be worth to introduce another method, but put a lot of comments on why it's discouraged:

/// Retrieve a list of public keys that the signer supports.
/// 
/// Most of the time it's recommended to use `get_supported_keys`
/// and only retrieve the keys from a limited set you expect them to
/// be coming from.
/// Usually this means that we have some on-chain (or off-chain) registry
/// of keys that can be used for signing (that we validate against).
/// Such keys should also undergo a periodic rotation.
/// There is no guarantee that the keys inside the keystore,
/// are actaully useful at all, so please consider employing a better
/// scheme before using that method.
fn get_keys(
    id: KeyTypeId, 
) -> Result<Vec<CryptoPublic>, String> {
  Err("Not supported")
  /// [ToDr] ^^^ My point is that it should be supported (secure things simple, advanced things possible).
}

- Convert all `.sign` calls to go through the keystore for signing.
- Add a note to sign about the best practice of going through the store than signing directly.
- [ben] sign isn't a Result or Option ... if you have access to a pair, the current API suggests you can always sign with it. so, there is no way we can ensure consistency without breaking the APIs on either the Pair or the keystore to not return Pair anymore. I prefer the latter because I think the primary usage of Pair seems to be for the development and testing Keyring object. So we could continue to support that, but the barekeystore never retruns a pair – a function I don't see being used anyways (after moving our code to use the new sign functions).
- as in replace sr25519_key_pair and ed25519_key_pair with return None and deprecate their usage
- or remove them altogether

on trait BareCryptoStore that is.