Refactor: simplify module call definitions (#24)

* Refactor: simplify module call definitions

* Fix compilation errors after merge

* Add missing comments and remove unused imports

* Now it compiles

src/codec.rs

@@ -1,8 +1,4 @@
-use parity_scale_codec::{
-    Encode,
-    EncodeAsRef,
-    HasCompact,
-};
+use parity_scale_codec::Encode;
 
 #[derive(Clone)]
 pub struct Encoded(pub Vec<u8>);
@@ -12,9 +8,3 @@ impl Encode for Encoded {
         self.0.to_owned()
     }
 }
-
-pub fn compact<T: HasCompact>(t: T) -> Encoded {
-    let encodable: <<T as HasCompact>::Type as EncodeAsRef<'_, T>>::RefType =
-        From::from(&t);
-    Encoded(encodable.encode())
-}

src/lib.rs

@@ -31,6 +31,7 @@ use metadata::Metadata;
 use parity_scale_codec::{
     Codec,
     Decode,
+    Encode,
 };
 use runtime_primitives::{
     generic::UncheckedExtrinsic,
@@ -61,15 +62,14 @@ use crate::{
         DefaultExtra,
         SignedExtra,
     },
-    metadata::MetadataError,
     palette::{
+        Call,
         balances::Balances,
         system::{
             System,
             SystemEvent,
             SystemStore,
         },
-        ModuleCalls,
     },
     rpc::{
         BlockNumber,
@@ -266,30 +266,21 @@ impl<T: System + Balances + 'static, S: 'static> Client<T, S> {
                 runtime_version,
                 genesis_hash,
                 signer,
-                call: None,
-                marker: PhantomData,
             }
         })
     }
 }
 
-/// The extrinsic builder is ready to finalize construction.
-pub enum Valid {}
-/// The extrinsic builder is not ready to finalize construction.
-pub enum Invalid {}
-
 /// Transaction builder.
-pub struct XtBuilder<T: System, P, S, V = Invalid> {
+pub struct XtBuilder<T: System, P, S> {
     client: Client<T, S>,
     nonce: T::Index,
     runtime_version: RuntimeVersion,
     genesis_hash: T::Hash,
     signer: P,
-    call: Option<Result<Encoded, MetadataError>>,
-    marker: PhantomData<fn() -> V>,
 }
 
-impl<T: System + Balances + 'static, P, S: 'static, V> XtBuilder<T, P, S, V>
+impl<T: System + Balances + 'static, P, S: 'static> XtBuilder<T, P, S>
 where
     P: Pair,
 {
@@ -304,42 +295,19 @@ where
     }
 
     /// Sets the nonce to a new value.
-    pub fn set_nonce(&mut self, nonce: T::Index) -> &mut XtBuilder<T, P, S, V> {
+    pub fn set_nonce(&mut self, nonce: T::Index) -> &mut XtBuilder<T, P, S> {
         self.nonce = nonce;
         self
     }
 
     /// Increment the nonce
-    pub fn increment_nonce(&mut self) -> &mut XtBuilder<T, P, S, V> {
+    pub fn increment_nonce(&mut self) -> &mut XtBuilder<T, P, S> {
         self.set_nonce(self.nonce() + 1.into());
         self
     }
-
-    /// Sets the module call to a new value
-    pub fn set_call<F>(&self, module: &'static str, f: F) -> XtBuilder<T, P, S, Valid>
-    where
-        F: FnOnce(ModuleCalls<T, P>) -> Result<Encoded, MetadataError>,
-    {
-        let call = self
-            .metadata()
-            .module(module)
-            .and_then(|module| f(ModuleCalls::new(module)))
-            .map_err(Into::into);
-
-        XtBuilder {
-            client: self.client.clone(),
-            nonce: self.nonce.clone(),
-            runtime_version: self.runtime_version.clone(),
-            genesis_hash: self.genesis_hash.clone(),
-            signer: self.signer.clone(),
-            call: Some(call),
-            marker: PhantomData,
-        }
-    }
 }
 
-impl<T: System + Balances + Send + Sync + 'static, P, S: 'static>
-    XtBuilder<T, P, S, Valid>
+impl<T: System + Balances + Send + Sync + 'static, P, S: 'static> XtBuilder<T, P, S>
 where
     P: Pair,
     S: Verify + Codec + From<P::Signature>,
@@ -347,8 +315,9 @@ where
     T::Address: From<T::AccountId>,
 {
     /// Creates and signs an Extrinsic for the supplied `Call`
-    pub fn create_and_sign(
+    pub fn create_and_sign<C>(
         &self,
+        call: Call<C>,
     ) -> Result<
         UncheckedExtrinsic<
             T::Address,
@@ -357,15 +326,18 @@ where
             <DefaultExtra<T> as SignedExtra<T>>::Extra,
         >,
         Error,
-    > {
+    >
+    where
+        C: parity_scale_codec::Encode,
+    {
         let signer = self.signer.clone();
         let account_nonce = self.nonce.clone();
         let version = self.runtime_version.spec_version;
         let genesis_hash = self.genesis_hash.clone();
         let call = self
-            .call
-            .clone()
-            .expect("A Valid extrinisic builder has a call; qed")?;
+            .metadata()
+            .module(&call.module)
+            .and_then(|module| module.call(&call.function, call.args))?;
 
         log::info!(
             "Creating Extrinsic with genesis hash {:?} and account nonce {:?}",
@@ -379,9 +351,9 @@ where
     }
 
     /// Submits a transaction to the chain.
-    pub fn submit(&self) -> impl Future<Item = T::Hash, Error = Error> {
+    pub fn submit<C: Encode>(&self, call: Call<C>) -> impl Future<Item = T::Hash, Error = Error> {
         let cli = self.client.connect();
-        self.create_and_sign()
+        self.create_and_sign(call)
             .into_future()
             .map_err(Into::into)
             .and_then(move |extrinsic| {
@@ -390,8 +362,9 @@ where
     }
 
     /// Submits transaction to the chain and watch for events.
-    pub fn submit_and_watch(
+    pub fn submit_and_watch<C: Encode>(
         &self,
+        call: Call<C>
     ) -> impl Future<Item = ExtrinsicSuccess<T>, Error = Error> {
         let cli = self.client.connect();
         let metadata = self.client.metadata().clone();
@@ -399,7 +372,7 @@ where
             .into_future()
             .map_err(Into::into);
 
-        self.create_and_sign()
+        self.create_and_sign(call)
             .into_future()
             .map_err(Into::into)
             .join(decoder)
@@ -419,9 +392,7 @@ mod tests {
             balances::{
                 Balances,
                 BalancesStore,
-                BalancesXt,
             },
-            contracts::ContractsXt,
         },
         DefaultNodeRuntime as Runtime,
     };
@@ -454,15 +425,14 @@ mod tests {
 
         let dest = AccountKeyring::Bob.to_account_id();
         let transfer = xt
-            .balances(|calls| calls.transfer(dest.clone().into(), 10_000))
-            .submit();
+            .submit(balances::transfer::<Runtime>(dest.clone().into(), 10_000));
         rt.block_on(transfer).unwrap();
 
         // check that nonce is handled correctly
         let transfer = xt
             .increment_nonce()
-            .balances(|calls| calls.transfer(dest.clone().into(), 10_000))
-            .submit();
+            .submit(balances::transfer::<Runtime>(dest.clone().into(), 10_000));
+
         rt.block_on(transfer).unwrap();
     }
 
@@ -483,8 +453,7 @@ mod tests {
         let wasm = wabt::wat2wasm(CONTRACT).expect("invalid wabt");
 
         let put_code = xt
-            .contracts(|call| call.put_code(500_000, wasm))
-            .submit_and_watch();
+            .submit_and_watch(contracts::put_code(500_000, wasm));
 
         let success = rt
             .block_on(put_code)
@@ -561,8 +530,9 @@ mod tests {
 
         let transfer = pallet_balances::Call::transfer(address.clone(), amount);
         let call = node_runtime::Call::Balances(transfer);
+        let subxt_transfer = crate::palette::balances::transfer::<Runtime>(address, amount);
         let call2 = balances
-            .call("transfer", (address, codec::compact(amount)))
+            .call("transfer", subxt_transfer.args)
             .unwrap();
         assert_eq!(call.encode().to_vec(), call2.0);
 

src/palette/balances.rs

@@ -1,34 +1,24 @@
 //! Implements support for the pallet_balances module.
 use crate::{
-    codec::{
-        compact,
-        Encoded,
-    },
     error::Error,
-    metadata::MetadataError,
     palette::{
+        Call,
         system::System,
-        ModuleCalls,
     },
     Client,
-    Valid,
-    XtBuilder,
 };
 use futures::future::{
     self,
     Future,
 };
-use parity_scale_codec::Codec;
+use parity_scale_codec::{Encode, Codec};
 use runtime_primitives::traits::{
-    IdentifyAccount,
     MaybeSerialize,
     Member,
     SimpleArithmetic,
-    Verify,
 };
 use runtime_support::Parameter;
 use std::fmt::Debug;
-use substrate_primitives::Pair;
 
 /// The subset of the `pallet_balances::Trait` that a client must implement.
 pub trait Balances: System {
@@ -101,61 +91,26 @@ impl<T: Balances + 'static, S: 'static> BalancesStore for Client<T, S> {
     }
 }
 
-/// The Balances extension trait for the XtBuilder.
-pub trait BalancesXt {
-    /// Balances type.
-    type Balances: Balances;
-    /// Keypair type
-    type Pair: Pair;
-    /// Signature type
-    type Signature: Verify;
+const MODULE: &str = "Balances";
+const TRANSFER: &str = "transfer";
 
-    /// Create a call for the pallet balances module
-    fn balances<F>(
-        &self,
-        f: F,
-    ) -> XtBuilder<Self::Balances, Self::Pair, Self::Signature, Valid>
-    where
-        F: FnOnce(
-            ModuleCalls<Self::Balances, Self::Pair>,
-        ) -> Result<Encoded, MetadataError>;
+/// Arguments for transferring a balance
+#[derive(Encode)]
+pub struct TransferArgs<T: Balances> {
+    to: <T as System>::Address,
+    #[codec(compact)]
+    amount: <T as Balances>::Balance
 }
 
-impl<T: Balances + 'static, P, S: 'static, V> BalancesXt for XtBuilder<T, P, S, V>
-where
-    P: Pair,
-    S: Verify,
-    S::Signer: From<P::Public> + IdentifyAccount<AccountId = T::AccountId>,
-{
-    type Balances = T;
-    type Pair = P;
-    type Signature = S;
-
-    fn balances<F>(&self, f: F) -> XtBuilder<T, P, S, Valid>
-    where
-        F: FnOnce(
-            ModuleCalls<Self::Balances, Self::Pair>,
-        ) -> Result<Encoded, MetadataError>,
-    {
-        self.set_call("Balances", f)
-    }
-}
-
-impl<T: Balances + 'static, P> ModuleCalls<T, P>
-where
-    P: Pair,
-{
-    /// Transfer some liquid free balance to another account.
-    ///
-    /// `transfer` will set the `FreeBalance` of the sender and receiver.
-    /// It will decrease the total issuance of the system by the `TransferFee`.
-    /// If the sender's account is below the existential deposit as a result
-    /// of the transfer, the account will be reaped.
-    pub fn transfer(
-        self,
-        to: <T as System>::Address,
-        amount: <T as Balances>::Balance,
-    ) -> Result<Encoded, MetadataError> {
-        self.module.call("transfer", (to, compact(amount)))
-    }
+/// Transfer some liquid free balance to another account.
+///
+/// `transfer` will set the `FreeBalance` of the sender and receiver.
+/// It will decrease the total issuance of the system by the `TransferFee`.
+/// If the sender's account is below the existential deposit as a result
+/// of the transfer, the account will be reaped.
+pub fn transfer<T: Balances>(
+    to: <T as System>::Address,
+    amount: <T as Balances>::Balance,
+) -> Call<TransferArgs<T>> {
+    Call::new(MODULE, TRANSFER, TransferArgs { to, amount })
 }

src/palette/contracts.rs

@@ -1,23 +1,17 @@
 //! Implements support for the pallet_contracts module.
 use crate::{
-    codec::{
-        compact,
-        Encoded,
-    },
-    metadata::MetadataError,
     palette::{
+        Call,
         balances::Balances,
         system::System,
-        ModuleCalls,
     },
-    Valid,
-    XtBuilder,
 };
-use runtime_primitives::traits::{
-    IdentifyAccount,
-    Verify,
-};
-use substrate_primitives::Pair;
+use parity_scale_codec::Encode;
+
+const MODULE: &str = "Contracts";
+const PUT_CODE: &str = "put_code";
+const CREATE: &str = "create";
+const CALL: &str = "call";
 
 /// Gas units are chosen to be represented by u64 so that gas metering
 /// instructions can operate on them efficiently.
@@ -26,122 +20,76 @@ pub type Gas = u64;
 /// The subset of the `pallet_contracts::Trait` that a client must implement.
 pub trait Contracts: System + Balances {}
 
-/// Blanket impl for using existing runtime types
-impl<
-        T: pallet_contracts::Trait
-            + palette_system::Trait
-            + pallet_balances::Trait
-            + std::fmt::Debug,
-    > Contracts for T
-where
-    <T as palette_system::Trait>::Header: serde::de::DeserializeOwned,
-{
+/// Arguments for uploading contract code to the chain
+#[derive(Encode)]
+pub struct PutCodeArgs {
+    #[codec(compact)]
+    gas_limit: Gas,
+    code: Vec<u8>,
 }
 
-/// The Contracts extension trait for the XtBuilder.
-pub trait ContractsXt {
-    /// Contracts type.
-    type Contracts: Contracts;
-    /// Key Pair Type
-    type Pair: Pair;
-    /// Signature type
-    type Signature: Verify;
-
-    /// Create a call for the pallet contracts module
-    fn contracts<F>(
-        &self,
-        f: F,
-    ) -> XtBuilder<Self::Contracts, Self::Pair, Self::Signature, Valid>
-    where
-        F: FnOnce(
-            ModuleCalls<Self::Contracts, Self::Pair>,
-        ) -> Result<Encoded, MetadataError>;
+/// Arguments for creating an instance of a contract
+#[derive(Encode)]
+pub struct CreateArgs<T: Contracts> {
+    endowment: <T as Balances>::Balance,
+    #[codec(compact)]
+    gas_limit: Gas,
+    code_hash: <T as System>::Hash,
+    data: Vec<u8>,
 }
 
-impl<T: Contracts + 'static, P, S: 'static, V> ContractsXt for XtBuilder<T, P, S, V>
-where
-    P: Pair,
-    S: Verify,
-    S::Signer: From<P::Public> + IdentifyAccount<AccountId = T::AccountId>,
-{
-    type Contracts = T;
-    type Pair = P;
-    type Signature = S;
-
-    fn contracts<F>(&self, f: F) -> XtBuilder<T, P, S, Valid>
-    where
-        F: FnOnce(
-            ModuleCalls<Self::Contracts, Self::Pair>,
-        ) -> Result<Encoded, MetadataError>,
-    {
-        self.set_call("Contracts", f)
-    }
+/// Arguments for calling a contract
+#[derive(Encode)]
+pub struct CallArgs<T: Contracts> {
+    dest: <T as System>::Address,
+    value: <T as Balances>::Balance,
+    #[codec(compact)]
+    gas_limit: Gas,
+    data: Vec<u8>,
 }
 
-impl<T: Contracts + 'static, P> ModuleCalls<T, P>
-where
-    P: Pair,
-{
-    /// Stores the given binary Wasm code into the chain's storage and returns
-    /// its `codehash`.
-    /// You can instantiate contracts only with stored code.
-    pub fn put_code(
-        &self,
-        gas_limit: Gas,
-        code: Vec<u8>,
-    ) -> Result<Encoded, MetadataError> {
-        self.module.call("put_code", (compact(gas_limit), code))
-    }
-
-    /// Creates a new contract from the `codehash` generated by `put_code`,
-    /// optionally transferring some balance.
-    ///
-    /// Creation is executed as follows:
-    ///
-    /// - The destination address is computed based on the sender and hash of
-    /// the code.
-    /// - The smart-contract account is created at the computed address.
-    /// - The `ctor_code` is executed in the context of the newly-created
-    /// account. Buffer returned after the execution is saved as the `code`
-    /// of the account. That code will be invoked upon any call received by
-    /// this account.
-    /// - The contract is initialized.
-    pub fn create(
-        &self,
-        endowment: <T as Balances>::Balance,
-        gas_limit: Gas,
-        code_hash: <T as System>::Hash,
-        data: Vec<u8>,
-    ) -> Result<Encoded, MetadataError> {
-        self.module.call(
-            "create",
-            (compact(endowment), compact(gas_limit), code_hash, data),
-        )
-    }
-
-    /// Makes a call to an account, optionally transferring some balance.
-    ///
-    /// * If the account is a smart-contract account, the associated code will
-    ///  be executed and any value will be transferred.
-    /// * If the account is a regular account, any value will be transferred.
-    /// * If no account exists and the call value is not less than
-    /// `existential_deposit`, a regular account will be created and any value
-    ///  will be transferred.
-    pub fn call(
-        &self,
-        dest: <T as System>::Address,
-        value: <T as Balances>::Balance,
-        gas_limit: Gas,
-        data: Vec<u8>,
-    ) -> Result<Encoded, MetadataError> {
-        self.module
-            .call("call", (dest, compact(value), compact(gas_limit), data))
-    }
+/// Stores the given binary Wasm code into the chain's storage and returns
+/// its `codehash`.
+/// You can instantiate contracts only with stored code.
+pub fn put_code(gas_limit: Gas, code: Vec<u8>) -> Call<PutCodeArgs> {
+    Call::new(MODULE, PUT_CODE, PutCodeArgs { gas_limit, code })
 }
 
-/// Contracts Events
-#[derive(parity_scale_codec::Decode)]
-pub enum Event<T: System> {
-    /// Contract code stored
-    CodeStored(T::Hash),
+/// Creates a new contract from the `codehash` generated by `put_code`,
+/// optionally transferring some balance.
+///
+/// Creation is executed as follows:
+///
+/// - The destination address is computed based on the sender and hash of
+/// the code.
+/// - The smart-contract account is created at the computed address.
+/// - The `ctor_code` is executed in the context of the newly-created
+/// account. Buffer returned after the execution is saved as the `code`https://www.bbc.co.uk/
+/// of the account. That code will be invoked upon any call received by
+/// this account.
+/// - The contract is initialized.
+pub fn create<T: Contracts>(
+    endowment: <T as Balances>::Balance,
+    gas_limit: Gas,
+    code_hash: <T as System>::Hash,
+    data: Vec<u8>,
+) -> Call<CreateArgs<T>> {
+    Call::new(MODULE, CREATE, CreateArgs { endowment, gas_limit, code_hash, data })
+}
+
+/// Makes a call to an account, optionally transferring some balance.
+///
+/// * If the account is a smart-contract account, the associated code will
+///  be executed and any value will be transferred.
+/// * If the account is a regular account, any value will be transferred.
+/// * If no account exists and the call value is not less than
+/// `existential_deposit`, a regular account will be created and any value
+///  will be transferred.
+pub fn call<T: Contracts>(
+    dest: <T as System>::Address,
+    value: <T as Balances>::Balance,
+    gas_limit: Gas,
+    data: Vec<u8>,
+) -> Call<CallArgs<T>> {
+    Call::new(MODULE, CALL, CallArgs { dest, value, gas_limit, data })
 }

src/palette/mod.rs

@@ -1,24 +1,20 @@
 //! Implements support for built-in runtime modules.
 
-use crate::metadata::ModuleMetadata;
-use std::marker::PhantomData;
+use parity_scale_codec::Encode;
 
 pub mod balances;
 pub mod contracts;
 pub mod system;
 
 /// Creates module calls
-pub struct ModuleCalls<T, P> {
-    module: ModuleMetadata,
-    marker: PhantomData<fn() -> (T, P)>,
+pub struct Call<T: Encode> {
+    pub module: &'static str,
+    pub function: &'static str,
+    pub args: T,
 }
 
-impl<T, P> ModuleCalls<T, P> {
-    /// Create new module calls
-    pub fn new(module: &ModuleMetadata) -> Self {
-        ModuleCalls::<T, P> {
-            module: module.clone(),
-            marker: PhantomData,
-        }
+impl<T: Encode> Call<T> {
+    pub fn new(module: &'static str, function: &'static str, args: T) -> Self {
+        Call { module, function, args }
     }
 }

src/palette/system.rs

@@ -1,15 +1,11 @@
 //! Implements support for the palette_system module.
 use crate::{
-    codec::Encoded,
     error::Error,
-    metadata::MetadataError,
     palette::{
+        Call,
         balances::Balances,
-        ModuleCalls,
     },
     Client,
-    Valid,
-    XtBuilder,
 };
 use futures::future::{
     self,
@@ -21,7 +17,6 @@ use runtime_primitives::traits::{
     CheckEqual,
     Hash,
     Header,
-    IdentifyAccount,
     MaybeDisplay,
     MaybeSerialize,
     MaybeSerializeDeserialize,
@@ -29,12 +24,10 @@ use runtime_primitives::traits::{
     SimpleArithmetic,
     SimpleBitOps,
     StaticLookup,
-    Verify,
 };
 use runtime_support::Parameter;
 use serde::de::DeserializeOwned;
 use std::fmt::Debug;
-use substrate_primitives::Pair;
 
 /// The subset of the `palette::Trait` that a client must implement.
 pub trait System: 'static + Eq + Clone + Debug {
@@ -145,54 +138,15 @@ impl<T: System + Balances + 'static, S: 'static> SystemStore for Client<T, S> {
     }
 }
 
-/// The System extension trait for the XtBuilder.
-pub trait SystemXt {
-    /// System type.
-    type System: System;
-    /// Keypair type
-    type Pair: Pair;
-    /// Signature type
-    type Signature: Verify;
+const MODULE: &str = "System";
+const SET_CODE: &str = "set_code";
 
-    /// Create a call for the pallet system module
-    fn system<F>(
-        &self,
-        f: F,
-    ) -> XtBuilder<Self::System, Self::Pair, Self::Signature, Valid>
-    where
-        F: FnOnce(
-            ModuleCalls<Self::System, Self::Pair>,
-        ) -> Result<Encoded, MetadataError>;
-}
+/// Arguments for updating the runtime code
+pub type SetCode = Vec<u8>;
 
-impl<T: System + Balances + 'static, P, S: 'static, V> SystemXt for XtBuilder<T, P, S, V>
-where
-    P: Pair,
-    S: Verify,
-    S::Signer: From<P::Public> + IdentifyAccount<AccountId = T::AccountId>,
-{
-    type System = T;
-    type Pair = P;
-    type Signature = S;
-
-    fn system<F>(&self, f: F) -> XtBuilder<T, P, S, Valid>
-    where
-        F: FnOnce(
-            ModuleCalls<Self::System, Self::Pair>,
-        ) -> Result<Encoded, MetadataError>,
-    {
-        self.set_call("System", f)
-    }
-}
-
-impl<T: System + 'static, P> ModuleCalls<T, P>
-where
-    P: Pair,
-{
-    /// Sets the new code.
-    pub fn set_code(&self, code: Vec<u8>) -> Result<Encoded, MetadataError> {
-        self.module.call("set_code", code)
-    }
+/// Sets the new code.
+pub fn set_code(code: Vec<u8>) -> Call<SetCode> {
+    Call::new(MODULE, SET_CODE, code)
 }
 
 /// Event for the System module.