feat(identity): mark sign as infaillible #5728
Conversation
| @@ -176,12 +178,12 @@ impl Keypair { | |||
| /// Sign a message using the private key of this keypair, producing | |||
| /// a signature that can be verified using the corresponding public key. | |||
| #[allow(unused_variables)] | |||
| pub fn sign(&self, msg: &[u8]) -> Result<Vec<u8>, SigningError> { | |||
| pub fn sign(&self, msg: &[u8]) -> Result<Vec<u8>, Infallible> { | |||
There was a problem hiding this comment.
Since we are returning Ok throughout the inner functions, should we go on and remove the Result here?
There was a problem hiding this comment.
We can. But what about upstream? there are a couple of functions that return a Result only because sign does so. Should we address them here? The biggest change will be noise security upgrade become infallible but the trait bound in swarm builder expect an associated Error type.
There was a problem hiding this comment.
We could change it so it is no longer expecting an associated error type
There was a problem hiding this comment.
Im not against making the change here. Thoughts on that @jxs ?
There was a problem hiding this comment.
yeah I think if it's api that doesn't make sense we should address it and take the next release cycle to release a new libp2p-identity release wdyt?
| @@ -90,7 +90,7 @@ impl Keypair { | |||
| self, | |||
| id_keys: &identity::Keypair, | |||
| ) -> Result<AuthenticKeypair, Error> { | |||
| let sig = id_keys.sign(&[STATIC_KEY_DOMAIN.as_bytes(), self.public.as_ref()].concat())?; | |||
| let sig = id_keys.sign(&[STATIC_KEY_DOMAIN.as_bytes(), self.public.as_ref()].concat()).unwrap(); | |||
There was a problem hiding this comment.
ditto; we should use expect here to provide context if it ever does fails and panics. Unlikely though but its more of a just-in-case.
Description
A minimal working example of making
Keypair::signinfaillible.Refer to #4650 for more detail.
Notes & open questions
Change checklist