From 193104c5e95efe78ef1b4c44fdd082f7404b79db Mon Sep 17 00:00:00 2001 From: "Demi M. Obenour" Date: Wed, 27 May 2020 21:15:56 -0400 Subject: [PATCH 1/4] Make signing fallable and asynchronous This is needed for hardware wallets, which require human confirmation to sign transactions. Blocking on a human to sign transactions is not a good idea, and the signing might fail for many reasons (device unplugged, authorization not granted, etc). --- Cargo.toml | 2 +- proc-macro/src/call.rs | 2 ++ src/frame/balances.rs | 21 +++----------- src/lib.rs | 30 +++++++++++++------ src/signer.rs | 66 ++++++++++++++++++++++++++---------------- 5 files changed, 69 insertions(+), 52 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9b98fb18e10..254d303767e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ include = ["Cargo.toml", "src/**/*.rs", "README.md", "LICENSE"] [dependencies] log = "0.4" thiserror = "1.0" -futures = "0.3" +futures = "0.3.5" jsonrpsee = { version = "0.1", features = ["ws"] } num-traits = { version = "0.2", default-features = false } serde = { version = "1.0", features = ["derive"] } diff --git a/proc-macro/src/call.rs b/proc-macro/src/call.rs index 234edbd8dde..d1eab818997 100644 --- a/proc-macro/src/call.rs +++ b/proc-macro/src/call.rs @@ -82,6 +82,7 @@ pub fn call(s: Structure) -> TokenStream { T: #module + #subxt::system::System + Send + Sync + 'static, S: #codec::Encode + Send + Sync + 'static, E: #subxt::SignedExtra + #subxt::sp_runtime::traits::SignedExtension + Send + Sync + 'static, + <>::Extra as #subxt::sp_runtime::traits::SignedExtension>::AdditionalSigned: Send + Sync, { fn #call<'a>( &'a self, @@ -154,6 +155,7 @@ mod tests { T: Balances + substrate_subxt::system::System + Send + Sync + 'static, S: codec::Encode + Send + Sync + 'static, E: substrate_subxt::SignedExtra + substrate_subxt::sp_runtime::traits::SignedExtension + Send + Sync + 'static, + <>::Extra as substrate_subxt::sp_runtime::traits::SignedExtension>::AdditionalSigned: Send + Sync, { fn transfer<'a>( &'a self, diff --git a/src/frame/balances.rs b/src/frame/balances.rs index 02e41d9342a..cbbca1fbcea 100644 --- a/src/frame/balances.rs +++ b/src/frame/balances.rs @@ -16,21 +16,11 @@ //! Implements support for the pallet_balances module. -use crate::frame::system::{ - System, - SystemEventsDecoder, -}; -use codec::{ - Decode, - Encode, -}; +use crate::frame::system::{System, SystemEventsDecoder}; +use codec::{Decode, Encode}; use core::marker::PhantomData; use frame_support::Parameter; -use sp_runtime::traits::{ - AtLeast32Bit, - MaybeSerialize, - Member, -}; +use sp_runtime::traits::{AtLeast32Bit, MaybeSerialize, Member}; use std::fmt::Debug; /// The subset of the `pallet_balances::Trait` that a client must implement. @@ -110,10 +100,7 @@ pub struct TransferEvent { mod tests { use super::*; use crate::{ - system::{ - AccountStore, - AccountStoreExt, - }, + system::{AccountStore, AccountStoreExt}, tests::test_client, }; use sp_keyring::AccountKeyring; diff --git a/src/lib.rs b/src/lib.rs index 9d38535627a..76d03cd0b80 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -46,7 +46,10 @@ pub use sp_core; pub use sp_runtime; use codec::Encode; -use futures::future; +use futures::{ + future, + future::TryFutureExt as _, +}; use jsonrpsee::client::Subscription; use sc_rpc_api::state::ReadProof; use sp_core::storage::{ @@ -118,7 +121,7 @@ pub struct ClientBuilder> { client: Option, } -impl ClientBuilder { +impl ClientBuilder { /// Creates a new ClientBuilder. pub fn new() -> Self { Self { @@ -327,18 +330,21 @@ where } /// Creates a signed extrinsic. - pub async fn create_signed>( + pub async fn create_signed + Send + Sync>( &self, call: C, signer: &(dyn Signer + Send + Sync), ) -> Result< UncheckedExtrinsic>::Extra>, Error, - > { + > + where + <>::Extra as SignedExtension>::AdditionalSigned: Send + Sync, + { let unsigned = self .create_unsigned(call, signer.account_id(), signer.nonce()) .await?; - Ok(signer.sign(unsigned)) + signer.sign(unsigned).map_err(From::from).await } /// Returns an events decoder for a call. @@ -379,21 +385,27 @@ where } /// Submits a transaction to the chain. - pub async fn submit>( + pub async fn submit + Send + Sync>( &self, call: C, signer: &(dyn Signer + Send + Sync), - ) -> Result { + ) -> Result + where + <>::Extra as SignedExtension>::AdditionalSigned: Send + Sync, + { let extrinsic = self.create_signed(call, signer).await?; self.submit_extrinsic(extrinsic).await } /// Submits transaction to the chain and watch for events. - pub async fn watch>( + pub async fn watch + Send + Sync>( &self, call: C, signer: &(dyn Signer + Send + Sync), - ) -> Result, Error> { + ) -> Result, Error> + where + <>::Extra as SignedExtension>::AdditionalSigned: Send + Sync, + { let extrinsic = self.create_signed(call, signer).await?; let decoder = self.events_decoder::()?; self.submit_and_watch_extrinsic(extrinsic, decoder).await diff --git a/src/signer.rs b/src/signer.rs index b33b3fdd339..0ab07b2e1da 100644 --- a/src/signer.rs +++ b/src/signer.rs @@ -17,27 +17,19 @@ //! A library to **sub**mit e**xt**rinsics to a //! [substrate](https://github.com/paritytech/substrate) node via RPC. -use crate::{ - extra::SignedExtra, - frame::system::System, - Encoded, -}; +use crate::{extra::SignedExtra, frame::system::System, Encoded}; use codec::Encode; use sp_core::Pair; use sp_runtime::{ - generic::{ - SignedPayload, - UncheckedExtrinsic, - }, - traits::{ - IdentifyAccount, - Verify, - }, + generic::{SignedPayload, UncheckedExtrinsic}, + traits::{IdentifyAccount, Verify, SignedExtension}, }; -use std::marker::PhantomData; +use std::{future::Future, marker::PhantomData, pin::Pin}; /// Extrinsic signer. -pub trait Signer> { +pub trait Signer> + where <>::Extra as SignedExtension>::AdditionalSigned: Send + Sync +{ /// Returns the account id. fn account_id(&self) -> &T::AccountId; @@ -45,10 +37,23 @@ pub trait Signer> { fn nonce(&self) -> Option; /// Takes an unsigned extrinsic and returns a signed extrinsic. + /// + /// Some signers may fail, for instance because the hardware on which the keys are located has + /// refused the operation. fn sign( &self, extrinsic: SignedPayload, - ) -> UncheckedExtrinsic; + ) -> Pin< + Box< + dyn Future< + Output = Result< + UncheckedExtrinsic, + String, + >, + > + Send + + Sync, + >, + >; } /// Extrinsic signer using a private key. @@ -91,12 +96,13 @@ where impl Signer for PairSigner where - T: System, - T::AccountId: Into, - S: Encode, - E: SignedExtra, - P: Pair, - P::Signature: Into, + T: System + 'static, + T::AccountId: Into + 'static, + S: Encode + 'static + Send + Sync, + E: SignedExtra + 'static, + P: Pair + 'static, + P::Signature: Into + 'static, + <>::Extra as SignedExtension>::AdditionalSigned: Send + Sync, { fn account_id(&self) -> &T::AccountId { &self.account_id @@ -109,14 +115,24 @@ where fn sign( &self, extrinsic: SignedPayload, - ) -> UncheckedExtrinsic { + ) -> Pin< + Box< + dyn Future< + Output = Result< + UncheckedExtrinsic, + String, + >, + > + Send + + Sync, + >, + > { let signature = extrinsic.using_encoded(|payload| self.signer.sign(payload)); let (call, extra, _) = extrinsic.deconstruct(); - UncheckedExtrinsic::new_signed( + Box::pin(futures::future::ready(Ok(UncheckedExtrinsic::new_signed( call, self.account_id.clone().into(), signature.into(), extra, - ) + )))) } } From 36b124d1d5c080d9b28f319260c3205bc2a50975 Mon Sep 17 00:00:00 2001 From: "Demi M. Obenour" Date: Thu, 28 May 2020 12:34:08 -0400 Subject: [PATCH 2/4] Reformat --- src/frame/balances.rs | 21 +++++++++++++++++---- src/signer.rs | 26 +++++++++++++++++++++----- 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/frame/balances.rs b/src/frame/balances.rs index cbbca1fbcea..02e41d9342a 100644 --- a/src/frame/balances.rs +++ b/src/frame/balances.rs @@ -16,11 +16,21 @@ //! Implements support for the pallet_balances module. -use crate::frame::system::{System, SystemEventsDecoder}; -use codec::{Decode, Encode}; +use crate::frame::system::{ + System, + SystemEventsDecoder, +}; +use codec::{ + Decode, + Encode, +}; use core::marker::PhantomData; use frame_support::Parameter; -use sp_runtime::traits::{AtLeast32Bit, MaybeSerialize, Member}; +use sp_runtime::traits::{ + AtLeast32Bit, + MaybeSerialize, + Member, +}; use std::fmt::Debug; /// The subset of the `pallet_balances::Trait` that a client must implement. @@ -100,7 +110,10 @@ pub struct TransferEvent { mod tests { use super::*; use crate::{ - system::{AccountStore, AccountStoreExt}, + system::{ + AccountStore, + AccountStoreExt, + }, tests::test_client, }; use sp_keyring::AccountKeyring; diff --git a/src/signer.rs b/src/signer.rs index 0ab07b2e1da..68657f04e13 100644 --- a/src/signer.rs +++ b/src/signer.rs @@ -17,18 +17,34 @@ //! A library to **sub**mit e**xt**rinsics to a //! [substrate](https://github.com/paritytech/substrate) node via RPC. -use crate::{extra::SignedExtra, frame::system::System, Encoded}; +use crate::{ + extra::SignedExtra, + frame::system::System, + Encoded, +}; use codec::Encode; use sp_core::Pair; use sp_runtime::{ - generic::{SignedPayload, UncheckedExtrinsic}, - traits::{IdentifyAccount, Verify, SignedExtension}, + generic::{ + SignedPayload, + UncheckedExtrinsic, + }, + traits::{ + IdentifyAccount, + SignedExtension, + Verify, + }, +}; +use std::{ + future::Future, + marker::PhantomData, + pin::Pin, }; -use std::{future::Future, marker::PhantomData, pin::Pin}; /// Extrinsic signer. pub trait Signer> - where <>::Extra as SignedExtension>::AdditionalSigned: Send + Sync +where + <>::Extra as SignedExtension>::AdditionalSigned: Send + Sync, { /// Returns the account id. fn account_id(&self) -> &T::AccountId; From bc19991b7a2584fbde7a026d85395a8c28f6a53f Mon Sep 17 00:00:00 2001 From: "Demi M. Obenour" Date: Thu, 28 May 2020 12:36:38 -0400 Subject: [PATCH 3/4] Refactor as suggested by Andrew Jones (@ascjones). --- src/lib.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 76d03cd0b80..5c287060569 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -46,10 +46,7 @@ pub use sp_core; pub use sp_runtime; use codec::Encode; -use futures::{ - future, - future::TryFutureExt as _, -}; +use futures::future; use jsonrpsee::client::Subscription; use sc_rpc_api::state::ReadProof; use sp_core::storage::{ @@ -344,7 +341,8 @@ where let unsigned = self .create_unsigned(call, signer.account_id(), signer.nonce()) .await?; - signer.sign(unsigned).map_err(From::from).await + let signed = signer.sign(unsigned).await?; + Ok(signed) } /// Returns an events decoder for a call. From e196153f784540bbbebedda1e512a6e270db067e Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Fri, 29 May 2020 08:30:04 +0100 Subject: [PATCH 4/4] Use future::ok --- src/signer.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/signer.rs b/src/signer.rs index 68657f04e13..ec60b195e47 100644 --- a/src/signer.rs +++ b/src/signer.rs @@ -144,11 +144,11 @@ where > { let signature = extrinsic.using_encoded(|payload| self.signer.sign(payload)); let (call, extra, _) = extrinsic.deconstruct(); - Box::pin(futures::future::ready(Ok(UncheckedExtrinsic::new_signed( + Box::pin(futures::future::ok(UncheckedExtrinsic::new_signed( call, self.account_id.clone().into(), signature.into(), extra, - )))) + ))) } }