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

runtime API: Substitute UncheckedExtrinsic with custom encoding #1076

Merged
merged 20 commits into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion codegen/src/types/substitutes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,14 @@ impl TypeSubstitutes {
parse_quote!(#crate_path::utils::KeyedVec),
),
(path_segments!(BTreeSet), parse_quote!(::std::vec::Vec)),
// The `UncheckedExtrinsic(pub Vec<u8>)` is part of the runtime API calls.
// The inner bytes represent the encoded extrinsic, however when deriving the
// `EncodeAsType` the bytes would be re-encoded. This leads to the bytes
// being altered by adding the length prefix in front of them.
(
path_segments!(sp_runtime::generic::unchecked_extrinsic::UncheckedExtrinsic),
parse_quote!(#crate_path::utils::UncheckedExtrinsic),
),
];

let default_substitutes = defaults
Expand Down Expand Up @@ -339,7 +347,7 @@ impl<T: scale_info::form::Form> From<&scale_info::Path<T>> for PathSegments {
/// to = ::subxt::utils::Static<::sp_runtime::MultiAddress<A, B>>
/// ```
///
/// And we encounter a `sp_runtime::MultiAddress<Foo, bar>`, then we will pass the `::sp_runtime::MultiAddress<A, B>`
/// And we encounter a `sp_runtime::MultiAddress<Foo, Bar>`, then we will pass the `::sp_runtime::MultiAddress<A, B>`
/// type param value into this call to turn it into `::sp_runtime::MultiAddress<Foo, Bar>`.
fn replace_path_params_recursively<I: Borrow<syn::Ident>, P: Borrow<TypePath>>(
path: &mut syn::Path,
Expand Down
2 changes: 2 additions & 0 deletions subxt/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub mod bits;
mod multi_address;
mod multi_signature;
mod static_type;
mod unchecked_extrinsic;
mod wrapper_opaque;

use codec::{Compact, Decode, Encode};
Expand All @@ -18,6 +19,7 @@ pub use account_id::AccountId32;
pub use multi_address::MultiAddress;
pub use multi_signature::MultiSignature;
pub use static_type::Static;
pub use unchecked_extrinsic::UncheckedExtrinsic;
pub use wrapper_opaque::WrapperKeepOpaque;

// Used in codegen
Expand Down
91 changes: 91 additions & 0 deletions subxt/src/utils/unchecked_extrinsic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// Copyright 2019-2023 Parity Technologies (UK) Ltd.
// This file is dual-licensed as Apache-2.0 or GPL-3.0.
// see LICENSE for license details.

//! The "default" Substrate/Polkadot UncheckedExtrinsic.
//! This is used in codegen for runtime API calls.
//!
//! The inner bytes represent the encoded extrinsic expected by the
//! runtime APIs. Deriving `EncodeAsType` would lead to the inner
//! bytes to be re-encoded (length prefixed).

use std::marker::PhantomData;

use codec::Decode;
use scale_decode::{visitor::DecodeAsTypeResult, IntoVisitor, Visitor};

/// The unchecked extrinsic from substrate.
#[derive(Clone, Debug, Eq, PartialEq, Decode)]
pub struct UncheckedExtrinsic<Address, Call, Signature, Extra>(
pub Vec<u8>,
#[codec(skip)] pub PhantomData<(Address, Call, Signature, Extra)>,
);
Copy link
Collaborator

@jsdw jsdw Jul 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially we could reuse some other code and avoid some duplicated logic by having this be:

pub struct UncheckedExtrinsic<Address, Call, Signature, Extra>{
    encoded: Static<Encoded>,
    #[codec(skip)]  _marker: PhantomData<(Address, Call, Signature, Extra)>,
};

Then, all of the impls could delegate to the Static<Encoded> type, which I think already does what we want it to.

In any case though, let's give this a nicer interface since users need to call it and shouldn't care about PhantomData etc, something like:

// make a new one:
UncheckedExtrinsic::new(bytes)
// can convert bytes into this type easily:
impl From<Vec<u8>> for UncheckedExtrinsic
// get the bytes:
unchecked_ext.bytes() -> &[u8]
// maybe convert back into bytes?:
impl From<UncheckedExtrinsic> for Vec<u8>

Hopefully type inference will sort out the generic params when it's used, ie

let runtime_api_call = node_runtime::apis()
        .transaction_payment_api()
        .query_fee_details(tx_bytes.into(), len);

Will hopefully Just Work, and infer all of the types properly!


impl<Address, Call, Signature, Extra> codec::Encode
for UncheckedExtrinsic<Address, Call, Signature, Extra>
{
fn encode(&self) -> Vec<u8> {
self.0.to_owned()
}
lexnv marked this conversation as resolved.
Show resolved Hide resolved
}

impl<Address, Call, Signature, Extra> scale_encode::EncodeAsType
for UncheckedExtrinsic<Address, Call, Signature, Extra>
{
fn encode_as_type_to(
&self,
_type_id: u32,
_types: &scale_info::PortableRegistry,
out: &mut Vec<u8>,
) -> Result<(), scale_encode::Error> {
out.extend_from_slice(&self.0);
Ok(())
}
}

impl<Address, Call, Signature, Extra> scale_encode::EncodeAsFields
for UncheckedExtrinsic<Address, Call, Signature, Extra>
{
fn encode_as_fields_to(
&self,
_fields: &mut dyn scale_encode::FieldIter<'_>,
_types: &scale_info::PortableRegistry,
out: &mut Vec<u8>,
) -> Result<(), scale_encode::Error> {
out.extend_from_slice(&self.0);
Ok(())
}
}
lexnv marked this conversation as resolved.
Show resolved Hide resolved

pub struct UncheckedExtrinsicDecodeAsTypeVisitor<Address, Call, Signature, Extra>(
PhantomData<(Address, Call, Signature, Extra)>,
);

impl<Address, Call, Signature, Extra> Visitor
for UncheckedExtrinsicDecodeAsTypeVisitor<Address, Call, Signature, Extra>
{
type Value<'scale, 'info> = UncheckedExtrinsic<Address, Call, Signature, Extra>;
type Error = scale_decode::Error;

fn unchecked_decode_as_type<'scale, 'info>(
self,
input: &mut &'scale [u8],
_type_id: scale_decode::visitor::TypeId,
_types: &'info scale_info::PortableRegistry,
) -> DecodeAsTypeResult<Self, Result<Self::Value<'scale, 'info>, Self::Error>> {
use scale_decode::{visitor::DecodeError, Error};
let decoded = UncheckedExtrinsic::decode(input)
.map_err(|e| Error::new(DecodeError::CodecError(e).into()));
DecodeAsTypeResult::Decoded(decoded)
}
}
Copy link
Collaborator

@jsdw jsdw Jul 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aah, I was hoping that you could get rid of more of this by using Static<Encoded> instead! This visitor trait is quite verbose alas.

I wonder whether something like this would work at least, to lean on the inner impl better (while not making any assumptions about it):

impl<Address, Call, Signature, Extra> Visitor
    for UncheckedExtrinsicDecodeAsTypeVisitor<Address, Call, Signature, Extra>
{
    type Value<'scale, 'info> = UncheckedExtrinsic<Address, Call, Signature, Extra>;
    type Error = scale_decode::Error;

    fn unchecked_decode_as_type<'scale, 'info>(
        self,
        input: &mut &'scale [u8],
        type_id: scale_decode::visitor::TypeId,
        types: &'info scale_info::PortableRegistry,
    ) -> DecodeAsTypeResult<Self, Result<Self::Value<'scale, 'info>, Self::Error>> {
        DecodeAsTypeResult::Decoded(self.0.decode_as_type(input, type_id.0, types))
    }
}

Maybe this is a sign that we can do something better with the DecodeAsType macro to allow things to be decoded based on some inner type or something.


impl<Address, Call, Signature, Extra> IntoVisitor
for UncheckedExtrinsic<Address, Call, Signature, Extra>
{
type Visitor = UncheckedExtrinsicDecodeAsTypeVisitor<Address, Call, Signature, Extra>;

fn into_visitor() -> Self::Visitor {
UncheckedExtrinsicDecodeAsTypeVisitor(PhantomData)
}
}
56 changes: 56 additions & 0 deletions testing/integration-tests/src/full_client/runtime_api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// see LICENSE for license details.

use crate::{node_runtime, test_context};
use codec::Encode;
use core::marker::PhantomData;
use subxt::utils::AccountId32;
use subxt_signer::sr25519::dev;

Expand Down Expand Up @@ -47,3 +49,57 @@ async fn account_nonce() -> Result<(), subxt::Error> {

Ok(())
}

#[tokio::test]
async fn unchecked_extrinsic_encoding() -> Result<(), subxt::Error> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the test; great to check that this works :)

let ctx = test_context().await;
let api = ctx.client();

let alice = dev::alice();
let bob = dev::bob();
let bob_address = bob.public_key().to_address();

// Construct a tx from Alice to Bob.
let tx = node_runtime::tx().balances().transfer(bob_address, 10_000);

let signed_extrinsic = api
.tx()
.create_signed(&tx, &alice, Default::default())
.await
.unwrap();

let tx_bytes = signed_extrinsic.into_encoded();
let len = tx_bytes.len() as u32;

// Manually encode the runtime API call arguments to make a raw call.
let mut encoded = tx_bytes.clone();
encoded.extend(len.encode());

let expected_result: node_runtime::runtime_types::pallet_transaction_payment::types::FeeDetails<
::core::primitive::u128,
> = api
.runtime_api()
.at_latest()
.await?
.call_raw(
"TransactionPaymentApi_query_fee_details",
Some(encoded.as_ref()),
)
.await?;

// Use the generated API to confirm the result with the raw call.
let runtime_api_call = node_runtime::apis()
.transaction_payment_api()
.query_fee_details(subxt::utils::UncheckedExtrinsic(tx_bytes, PhantomData), len);

let result = api
.runtime_api()
.at_latest()
.await?
.call(runtime_api_call)
.await?;

assert_eq!(expected_result, result);

Ok(())
}