Encryption, decryption and public key RPCs. #1946
Conversation
gavofyork
added
the
A3-inprogress ⏳
gavofyork
added
A0-pleasereview 🤓
| self.sender.set(Some(s)); | ||
| Ok(s) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Returns the public key of the sender. | ||
| pub fn public_key(&self) -> Result<Public, Error> { | ||
| Ok(try!(recover(&self.signature(), &self.unsigned.hash()))) |
There was a problem hiding this comment.
Ok(try!(...)) can just be .... If the error type is wrong then a .map(Into::into) might be more idiomatic
There was a problem hiding this comment.
Also may want to move the caching of self.sender into this function, so that all uses of public_key can benefit from the cache.
There was a problem hiding this comment.
doesn't really work for further calls to public_key, but would indeed mean sender were cached on a call to public_key.
| try!(self.active()); | ||
| from_params::<(RpcH160, RpcBytes)>(params).and_then(|(address, ciphertext)| { | ||
| let s = try!(take_weak!(self.accounts).decrypt(address.into(), &[0; 0], &ciphertext.0).map_err(|_| Error::internal_error())); | ||
| Ok(to_value(&RpcBytes::from(s))) |
There was a problem hiding this comment.
to_value can now take arguments by value.
| @@ -75,6 +78,7 @@ impl From<LocalizedTransaction> for Transaction { | |||
| Action::Call(_) => None, | |||
| }, | |||
| raw: ::rlp::encode(&t.signed).to_vec().into(), | |||
| public_key: t.public_key().ok().map(Into::into), | |||
There was a problem hiding this comment.
for example, using the existing cache would prevent an extra ec_recover here.
There was a problem hiding this comment.
would need to cache both public_key and sender's address to retain present cache performance on sender since you can't derive public_key from sender.
| @@ -51,6 +51,9 @@ pub struct Transaction { | |||
| pub creates: Option<H160>, | |||
| /// Raw transaction data | |||
| pub raw: Bytes, | |||
| /// Public key of the signer. | |||
| #[serde(rename="publicKey")] | |||
| pub public_key: Option<H512>, | |||
There was a problem hiding this comment.
This transaction structure diverges from the spec: https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_gettransactionbyhash
Affected: all getTransactionBy* and getBlockBy* functions
There was a problem hiding this comment.
it's additive functionality and therefore of no great harm. in any case, we're doing it already with additional APIs and providing a separate API for it would be substantially more code/complexity with relatively little gain.
There was a problem hiding this comment.
other thing to note is that we already provide a non-standard block structure (with the key sealFields). it's not especially controversial.
| @@ -235,6 +239,7 @@ pub trait EthSigning: Sized + Send + Sync + 'static { | |||
| delegate.add_method("eth_postSign", EthSigning::post_sign); | |||
| delegate.add_method("eth_postTransaction", EthSigning::post_transaction); | |||
| delegate.add_method("eth_checkRequest", EthSigning::check_request); | |||
| delegate.add_method("ethcore_decryptMessage", EthSigning::decrypt_message); | |||
There was a problem hiding this comment.
Mixing of module names here? Strange that EthSigning would bridge both eth and ethcore.
There was a problem hiding this comment.
EthSigning is a bit of an odd one as it pollutes into eth_ namespace. this just makes it pollute into ethcore_ namespace, too.
Still need to add support for "signer" to decrypt payloads; this can be separate PR though.