Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify Extrinsic Creation and Signing #477

Closed
jsdw opened this issue Mar 16, 2022 · 6 comments
Closed

Simplify Extrinsic Creation and Signing #477

jsdw opened this issue Mar 16, 2022 · 6 comments
Assignees

Comments

@jsdw
Copy link
Collaborator

jsdw commented Mar 16, 2022

Some goals I have in mind here:

  • Reduce our dependence on substrate types.
  • Instead, implement the encoding and signing logic ourselves, in one place, so it's easy to follow.
  • Simplify the process of creating and using custom additional/extra params, so that it can be customised with the minimum of fuss.
  • Don't sacrifice the existing flexibility re custom signer and custom extra/additional params.

How:

  • Have extrinsic construction and signing all happen in one place (eg client.create_signed or perhaps extrinsic::create_signed), so it's easy to see what's going on. Right now you have to dig through various traits and such to piece together how an extrinsic is encoded.
  • Additional and Extra params; have a single, simple trait to implement in order to customise these.
  • Maintain a way to pass in extra, custom params as we do in create_signed at the mo. Perhaps allow the nonce() to be optionally pulled from these too (in the future, it might be that we can opt to pull other params from here to override the defaults).
  • Simplify the Signer trait if useful/needed (we just want to be able to obtain the account ID and sign the relevant bytes, I think).
  • As a result of the above, remove reliance on substrate types and traits like SignedExtension and UncheckedExtrinsic; I don't think we need them if we handle encoding ourselves.

I thought about codegen to create the extra/additional types, and it may help a little going forwards, but for now I couldn't see a compelling enough win by doing this (I like the current approach of passing useful details from the client like nonce etc in when constructing them, and not sure how to reconcile this with codegen). Instead, I want to make it as easy as possible to create and use custom extra/additional params for your chain.

These thoughts expressed in code:

// SubmittableExtrinsic::create_signed: we could move more of the extrinsic
// construction logic into this function. If we did, it would look more like
// the following. Hopefullt it's much easier to see from this how extrinsics
// are constructed (and tweak things as needed going forwards).
pub async fn create_signed(
    &self,
    signer: &(dyn Signer<T, X> + Send + Sync),
    additional_params: X::Parameters,
) -> Result<Encoded, BasicError> {
    // 1. get nonce
    let account_nonce = if let Some(nonce) = signer.nonce() {
        nonce
    } else {
        self.client
            .rpc()
            .system_account_next_index(signer.account_id())
            .await?
    };

    // 2. SCALE encode call data to bytes (pallet u8, call u8, call params).
    let call_data: Vec<u8> = {
        let mut encoded = Vec::new();
        self
            .client
            .metadata()
            .pallet(C::PALLET)
            .and_then(|pallet| pallet.encode_call_to(&mut encoded))?;
        encoded
    };

    // 3. construct our custom additional/extra params.
    let additional_and_extra_params = X::new(
        self.client.runtime_version.spec_version,
        self.client.runtime_version.transaction_version,
        account_nonce,
        self.client.genesis_hash,
        additional_params,
    );

    // 4. construct payload to be signed
    let payload_to_be_signed = {
        let mut encoded = call_data.clone();
        additional_and_extra_params.encode_extra_to(&mut encoded);
        additional_and_extra_params.encode_additional_to(&mut encoded);
        encoded
    };

    // 5. Create signature from this.
    let signature_bytes: Vec<u8> = signer.sign(&payload_to_be_signed);

    // 6. Encode extrinsic, now that we have the parts we need.
    // (this may not be 100% correct but should be close enough for this example)
    let extrinsic = {
        let mut encoded_inner = Vec::new();
        // "is signed" + transaction protocol version (4)
        (b10000000 + 4u8).encode_to(&mut encoded_inner);
        // from address for signature
        signer.account_id().encode_to(&mut encoded_inner);
        // the signature bytes
        signature_bytes.encode_to(&mut encoded_inner);
        // attach custom extra params
        additional_and_extra_params.encode_extra_to(&mut encoded_inner);
        // and now, call data
        encoded_inner.extend(&mut call_data);
        // now, prefix byte length:
        let len = Compact(encoded_inner.len());
        let mut encoded = Vec::new();
        len.encode_to(&mut encoded);
        encoded.extend(&mut encoded_inner);
        encoded
    };

    // Wrap in Encoded to ensure that any more "encode" calls leave it in the right state.
    // maybe we can just return the raw bytes..
    Encoded(extrinsic)
}

// The above can guide how some of our other traits work. For example, `X` above would impl
// a trait like this, which is just a minimal way of allowing a user to supply some custom
// extra/additional params that we can SCALE encode as needed. Inspired by SignedExtra
trait ExtrinsicParams {
    type OtherParams: Default;

    fn new<T: Config>(
        spec_version: u32,
        tx_version: u32,
        nonce: T::Index,
        genesis_hash: T::Hash,
        other_params: Self::OtherParams,
    ) -> Self;

    fn encode_extra(&self, v: &mut Vec<u8>);
    fn encode_additional(&self, v: &mut Vec<u8>);
}

// We want a default type still, so that things "just work" if these extra/additional
// bits haven't been changed. This could be composed from smaller types, but it might be
// simpler just to embed the config we need here, so it's easy to manually construct when
// we want to do things like set the tip etc.
pub struct DefaultExtra<T> {
    era: Era,
    nonce: u32,
    tip: u128,
    spec_version: u32,
    transaction_version: u32,
    genesis_hash: Hash,
    mortality_checkpoint: Hash,
    marker: std::marker::PhantomData<T>
}

// Some sort of builder for the above to let us configure it
pub struct DefaultExtraBuilder {
    era: Era,
    era_checkpoint: Hash,
    tip: u128
}

impl <T: Config> ExtrinsicParams for DefaultExtra<T> {
    // This is how we can pass valeus at runtime:
    type OtherParams = DefaultExtraBuilder;

    fn new<T: Config>(
        // Provided from subxt client:
        spec_version: u32,
        tx_version: u32,
        nonce: T::Index,
        genesis_hash: T::Hash,
        // Provided externally:
        other_params: Self::OtherParams,
    ) -> Self {
        // use params from subxt client, as well as DefaultExtraBuilder,
        // to build our DefaultExtra type.
    }

    // And this is how we turn the params into our extra/additional SCALE
    // bytes as needed for extrinsic construction:
    fn encode_extra(&self, v: &mut Vec<u8>) {
        (self.era, Compact(self.nnonce), Compact(self.tip)).encode_to(v);
    }
    fn encode_additional(&self, v: &mut Vec<u8>) {
        (
            self.spec_version,
            self.transaction_version,
            self.genesis_hash,
            self.mortality_checkpoint
        ).encode_to(v);
    }
}

// We have the convenient `sign_and_submit_then_watch` and `sign_and_submit` methods which
// take default "other params", and force users to "do it themselves" a little more with
// "create_signed" if they want to supply parameters like a tip. We could also expose eg:
pub async fn sign_and_submit_then_watch_with_params(
    self,
    signer: &(dyn Signer<T, X> + Send + Sync),
    params: X::OtherParams
) -> Result<TransactionProgress<'client, T, E, Evs>, BasicError> { /*... */ }

pub async fn sign_and_submit_with_params(
    self,
    signer: &(dyn Signer<T, X> + Send + Sync),
) -> Result<T::Hash, BasicError> { /*... */ }

// Perhaps we swap the names around and make `sign_and_submit` the above, and `sign_and_submit_default`
// the version we currently expose using default params?

As much as anything, the code above serves as a reference to myself or anybody else who'd like to implement this as a starting point to work from!

Anyway, Any thoughts and such would be appreciated! :)

Supercedes #390 and #187.

Closed by #490.

@jsdw jsdw self-assigned this Mar 21, 2022
@amrbz
Copy link

amrbz commented Mar 24, 2022

@jsdw many thanks for this example. It really helped me to better understand how extrinsics work and rewrite my code which initially was based on this guide. I got some errors using subxt::PairSigner; and used sp_runtime::MultiSignature to sign call data. Unfortunately, I still get a "Transaction has a bad signature" error. Any thoughts on what I'm doing wrong? Thanks!

  let from = AccountKeyring::Alice.to_account_id();
  let alice_nonce = get_nonce(&from).await;
  let runtime_version = get_runtime_version().await.unwrap();
  let genesis_hash = get_genesis_hash().await;

  let pallet_index: u8 = 8;
  let method_index: u8 = 0;
  let call_index: [u8; 2] = [pallet_index, method_index];
  // extrinsic parameter
  let address: Vec<u8> = "4d4c14c40d1b7ecb942455794693fa68".as_bytes().to_vec().encode();
  // extrinsic parameter
  let owner: [u8; 1] = [0];

  let call: Vec<u8>  = [call_index.to_vec(), address, owner.to_vec()].concat();

  let extra = (
      Era::Immortal,
      Compact(alice_nonce),
      Compact(0u128)
  );

  let additional = (
      runtime_version.spec_version,
      runtime_version.transaction_version,
      genesis_hash,
      genesis_hash,
  );

  let payload_to_be_signed: Vec<u8> = {
      let mut encoded = call.clone();
      extra.encode_to(&mut encoded);
      additional.encode_to(&mut encoded);
      encoded
  };

  let signature= {
      if payload_to_be_signed.len() > 256 {
          AccountKeyring::Alice.sign(&sp_core::blake2_256(&payload_to_be_signed)[..])
      } else {
          AccountKeyring::Alice.sign(&payload_to_be_signed)
      }
  };

  let extrinsic = {
      let mut encoded_inner = Vec::new();
      (0b1000_0000 + 4u8).encode_to(&mut encoded_inner);
      MultiAddress::Id::<_, u32>(from).encode_to(&mut encoded_inner);
      MultiSignature::Sr25519(signature).encode_to(&mut encoded_inner);
      extra.encode_to(&mut encoded_inner);
      encoded_inner.extend(&call);
      let len = Compact(encoded_inner.len() as u32);
      let mut encoded = Vec::new();
      len.encode_to(&mut encoded);
      encoded.extend(&encoded_inner);
      encoded
  };

  let extrinsic_hex = format!("0x{}", hex::encode(&extrinsic));
  println!("RESULT HEX {:?}", extrinsic_hex);

@jsdw
Copy link
Collaborator Author

jsdw commented Mar 24, 2022

@amrbz My example code above was very quickly scrabbled together and was probably a little off; have a look at

pub async fn create_signed(
for the "final" version which has been tested to work.

Offhand what you've done looks good for a current Polkadot node though, so you may need to dig a little deeper. But, for a substrate node (ie substrate --dev --tmp) the tip payment (the Compact(0u128) in your extra) needs replacing with (Compact(0u128), None as Option<u32>) where the option can represent the ID of the asset that you'll be giving as a tip (None meaning.. pay using the native node currency).

@amrbz
Copy link

amrbz commented Mar 24, 2022

@jsdw Thanks. May I ask on the error with subxt::PairSigner. I use the code from the example

use subxt::PairSigner;

let signer = PairSigner::new(AccountKeyring::Alice.pair());

But I got this error:

❯ cargo run                                                                                                                                                                                                                                                        rpc-substrate -> main ?
   Compiling rpc-substrate v0.1.0 (/Users/amrbz/Dev/rust/rpc-substrate)
error[E0284]: type annotations needed for `PairSigner<T, E, sp_core::sr25519::Pair>`
   --> src/main.rs:363:18
    |
363 |     let signer = PairSigner::new(AccountKeyring::Alice.pair());
    |         ------   ^^^^^^^^^^^^^^^ cannot infer type for type parameter `T`
    |         |
    |         consider giving `signer` the explicit type `PairSigner<T, E, _>`, where the type parameter `T` is specified
    |
    = note: cannot satisfy `<_ as Config>::Signature == _`

For more information about this error, try `rustc --explain E0284`.
error: could not compile `rpc-substrate` due to previous error

So I can't try out subxt lib for signing data. Maybe that would help.

@jsdw
Copy link
Collaborator Author

jsdw commented Mar 25, 2022

T is the config type that's used. If you actually pass that signer into a subxt function like sign_and_submit_default() (eg like here) Rust will be able to infer the correct type for you.

In isolation, you'll need to tell it manually what typw T is; the default is subxt::DefaultConfig (which works fine with Substrate/Polkadot-like chains), so something like this probably will suffice:

let signer = PairSigner::<subxt::DefaultConfig, _, _>::new(AccountKeyring::Alice.pair());

@amrbz
Copy link

amrbz commented Mar 25, 2022

@jsdw I've figured out how to generate an extrinsic offline. The working code is available here. Thank you very much for your help!

@jsdw
Copy link
Collaborator Author

jsdw commented Mar 31, 2022

Closed by #490

@jsdw jsdw closed this as completed Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants