Skip to content
This repository has been archived by the owner on Jun 3, 2020. It is now read-only.

Implement signing and add back integration test #44

Closed
liamsi opened this issue Aug 28, 2018 · 3 comments · Fixed by #55
Closed

Implement signing and add back integration test #44

liamsi opened this issue Aug 28, 2018 · 3 comments · Fixed by #55

Comments

@liamsi
Copy link
Contributor

liamsi commented Aug 28, 2018

The existing integration test for signing was removed with #38. The sign requests used to look like:

/// Sign the given opaque message with the given public key
#[derive(Serialize, Deserialize, Debug)]
pub struct SignRequest {
    /// Public key to use to sign the request
    pub public_key: Vec<u8>,

    /// Message to be signed
    pub msg: Vec<u8>,
}

This doesn't match with the messages/requests we receive (and (de)serialize) from the tendermint side. E.g., SignBytes of:
https://github.com/tendermint/tendermint/blob/013b9cef642f875634c614019ab13b17570778ad/types/vote.go#L62-L72

Signing needs to be implemented and the integration test reintroduced. The current integration test only tests the secret connection by sending / handling a message (PoisonPillMsg).

These types and their encoding might change with (we need to track this): tendermint/tendermint#1622

We might need to change this trait (implemented by all types) as it currently can't know about which private key to use for the signing operation:

pub trait TendermintSign {
    fn cannonicalize(self, chain_id: &str) -> String;
    fn sign(&mut self);
}

We might just remove the sign method from that trait and only have a cannonicalize or SignBytes equivalent.

@jleni
Copy link
Contributor

jleni commented Sep 11, 2018

Do you have any progress here? I would like to start running integration tests for signatory

@liamsi
Copy link
Contributor Author

liamsi commented Sep 11, 2018

I haven't continued working on this yet. I hope to prioritize it this week though.

@tarcieri
Copy link
Contributor

I'll be trying to land a Signatory upgrade today, just FYI

alessio pushed a commit that referenced this issue May 15, 2020
Cargo.lock: update dependencies
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants