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

Static Decoding of Signed Extensions: Simple Approach #1235

Merged
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
c0226dc
skeleton commit
tadeohepperle Oct 9, 2023
ffec0b1
signed extension decoding
tadeohepperle Oct 12, 2023
b0fb96a
fix some minor things
tadeohepperle Oct 16, 2023
c87f165
make api more similar to Extrinsics
tadeohepperle Oct 23, 2023
4ed554d
defer decoding of signed extensions
tadeohepperle Oct 23, 2023
460bfaf
fix byte slices
tadeohepperle Oct 23, 2023
ea59e6c
add test for nonce signed extension
tadeohepperle Oct 23, 2023
3dad997
adjust test and extend for tip
tadeohepperle Oct 24, 2023
fe708e7
Merge branch 'master' into tadeohepperle/support-decoding-signed-exte…
tadeohepperle Oct 24, 2023
0eabab9
clippy
tadeohepperle Oct 24, 2023
58fd654
support both ChargeTransactionPayment and ChargeAssetTxPayment
tadeohepperle Oct 24, 2023
1dd8f53
address PR comments
tadeohepperle Oct 25, 2023
64b5c90
Extend lifetimes, expose pub structs, remove as_type
jsdw Oct 27, 2023
7d8447e
Merge branch 'master' into tadeohepperle/support-decoding-signed-exte…
tadeohepperle Oct 27, 2023
fe1db8c
add signed extensions to block subscribing example
tadeohepperle Oct 30, 2023
8c93d5b
add Decoded type
tadeohepperle Oct 30, 2023
bba2c4b
Merge branch 'master' into tadeohepperle-static-decoding-of-signed-ex…
tadeohepperle Oct 31, 2023
2692882
fix merging bug and tests
tadeohepperle Oct 31, 2023
0d52317
add decoded type in CustomSignedExtension
tadeohepperle Oct 31, 2023
7ff7552
fix minor issues, extend test
tadeohepperle Oct 31, 2023
f200d0f
cargo fmt differences
tadeohepperle Nov 2, 2023
5afe150
remove the `decoded` function
tadeohepperle Nov 2, 2023
42a0423
new as_signed_extra fn, do not expose as_type anymore
tadeohepperle Nov 2, 2023
b7d9411
fix Result-Option order, simplify obtaining Nonce
tadeohepperle Nov 3, 2023
13e3f2d
tx: Remove `wait_for_in_block` helper method (#1237)
lexnv Nov 2, 2023
39f1a30
Update smoldot to 0.12 (#1212)
lexnv Nov 3, 2023
cb65208
ChargeAssetTxPayment: support providing u32 or MultiLocation in defau…
tadeohepperle Nov 3, 2023
d50519d
generic AssetId
tadeohepperle Nov 9, 2023
1dc9976
Merge branch 'master' into tadeohepperle-static-decoding-of-signed-ex…
tadeohepperle Nov 10, 2023
3a79e70
fix generics
tadeohepperle Nov 10, 2023
0876c28
fmt
tadeohepperle Nov 10, 2023
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
1 change: 0 additions & 1 deletion subxt/examples/blocks_subscribing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {

for evt in events.iter() {
let evt = evt?;

let pallet_name = evt.pallet_name();
let event_name = evt.variant_name();
let event_values = evt.field_values()?;
Expand Down
1 change: 1 addition & 0 deletions subxt/examples/setup_config_signed_extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ pub struct CustomSignedExtension;
// up in the chain metadata in order to know when and if to use it.
impl<T: Config> signed_extensions::SignedExtension<T> for CustomSignedExtension {
const NAME: &'static str = "CustomSignedExtension";
type Decoded = ();
}

// Gather together any params we need for our signed extension, here none.
Expand Down
68 changes: 49 additions & 19 deletions subxt/src/blocks/extrinsic_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ use crate::{
Metadata,
};

use crate::config::signed_extensions::{ChargeAssetTxPayment, ChargeTransactionPayment};
use crate::config::SignedExtension;
use crate::dynamic::DecodedValue;
use crate::utils::strip_compact_prefix;
use codec::{Compact, Decode};
Expand Down Expand Up @@ -366,12 +368,13 @@ where
}

/// Returns `None` if the extrinsic is not signed.
pub fn signed_extensions(&self) -> Option<ExtrinsicSignedExtensions<'_>> {
pub fn signed_extensions(&self) -> Option<ExtrinsicSignedExtensions<'_, T>> {
let signed = self.signed_details.as_ref()?;
let extra_bytes = &self.bytes[signed.signature_end_idx..signed.extra_end_idx];
Some(ExtrinsicSignedExtensions {
bytes: extra_bytes,
metadata: &self.metadata,
_marker: std::marker::PhantomData,
})
}

Expand Down Expand Up @@ -605,21 +608,22 @@ impl<T: Config> ExtrinsicEvents<T> {

/// The signed extensions of an extrinsic.
#[derive(Debug, Clone)]
pub struct ExtrinsicSignedExtensions<'a> {
pub struct ExtrinsicSignedExtensions<'a, T: Config> {
bytes: &'a [u8],
metadata: &'a Metadata,
_marker: std::marker::PhantomData<T>,
}

impl<'a> ExtrinsicSignedExtensions<'a> {
impl<'a, T: Config> ExtrinsicSignedExtensions<'a, T> {
/// Returns an iterator over each of the signed extension details of the extrinsic.
/// If the decoding of any signed extension fails, an error item is yielded and the iterator stops.
pub fn iter(&self) -> impl Iterator<Item = Result<ExtrinsicSignedExtension<'a>, Error>> {
pub fn iter(&self) -> impl Iterator<Item = Result<ExtrinsicSignedExtension<T>, Error>> {
let signed_extension_types = self.metadata.extrinsic().signed_extensions();
let num_signed_extensions = signed_extension_types.len();
let bytes = self.bytes;
let metadata = self.metadata;
let mut index = 0;
let mut byte_start_idx = 0;
let metadata = &self.metadata;

std::iter::from_fn(move || {
if index == num_signed_extensions {
Expand Down Expand Up @@ -649,25 +653,38 @@ impl<'a> ExtrinsicSignedExtensions<'a> {
ty_id,
identifier: extension.identifier(),
metadata,
_marker: std::marker::PhantomData,
}))
})
}

fn find_by_name(&self, name: &str) -> Option<ExtrinsicSignedExtension<'_, T>> {
let signed_extension = self
.iter()
.find_map(|e| e.ok().filter(|e| e.name() == name))?;
Some(signed_extension)
}

/// Searches through all signed extensions to find a specific one.
/// If the Signed Extension is found, but decoding failed, `Some(Err(err))` is returned.
pub fn find<S: SignedExtension<T>>(&self) -> Option<Result<S::Decoded, Error>> {
Copy link
Collaborator

@jsdw jsdw Nov 2, 2023

Choose a reason for hiding this comment

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

Nit: Maybe this should be Result<Option<..> instead, actually? I'm never sure; just looking at some of the other find methods we have around :)

let signed_extension = self.find_by_name(S::NAME)?;
signed_extension.as_signed_extra::<S>().transpose()
}

/// The tip of an extrinsic, extracted from the ChargeTransactionPayment or ChargeAssetTxPayment
/// signed extension, depending on which is present.
///
/// Returns `None` if `tip` was not found or decoding failed.
pub fn tip(&self) -> Option<u128> {
let tip = self.iter().find_map(|e| {
e.ok().filter(|e| {
e.name() == "ChargeTransactionPayment" || e.name() == "ChargeAssetTxPayment"
})
})?;

// Note: ChargeAssetTxPayment might have addition information in it (asset_id).
// But both should start with a compact encoded u128, so this decoding is fine.
let tip = Compact::<u128>::decode(&mut tip.bytes()).ok()?.0;
Some(tip)
// Note: the overhead of iterating twice should be negligible.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could probably modify the find_by_name function to take in a filter instead of the name, but yea we have a few extras anyway

self.find::<ChargeTransactionPayment>()
.map(|e| e.map(|e| e.tip()))
.or_else(|| {
self.find::<ChargeAssetTxPayment>()
.map(|e| e.map(|e| e.tip()))
})?
.ok()
}

/// The nonce of the account that submitted the extrinsic, extracted from the CheckNonce signed extension.
Expand All @@ -684,14 +701,15 @@ impl<'a> ExtrinsicSignedExtensions<'a> {

/// A single signed extension
#[derive(Debug, Clone)]
pub struct ExtrinsicSignedExtension<'a> {
pub struct ExtrinsicSignedExtension<'a, T: Config> {
bytes: &'a [u8],
ty_id: u32,
identifier: &'a str,
metadata: &'a Metadata,
_marker: std::marker::PhantomData<T>,
}

impl<'a> ExtrinsicSignedExtension<'a> {
impl<'a, T: Config> ExtrinsicSignedExtension<'a, T> {
/// The bytes representing this signed extension.
pub fn bytes(&self) -> &'a [u8] {
self.bytes
Expand All @@ -709,11 +727,23 @@ impl<'a> ExtrinsicSignedExtension<'a> {

/// Signed Extension as a [`scale_value::Value`]
pub fn value(&self) -> Result<DecodedValue, Error> {
let value =
DecodedValue::decode_as_type(&mut &self.bytes[..], self.ty_id, self.metadata.types())?;
self.as_type()
}

/// Decodes the `extra` bytes of this Signed Extension into a static type.
fn as_type<E: DecodeAsType>(&self) -> Result<E, Error> {
let value = E::decode_as_type(&mut &self.bytes[..], self.ty_id, self.metadata.types())?;
Ok(value)
}

/// Decodes the `extra` bytes of this Signed Extension into its associated `Decoded` type.
/// Returns `Ok(None)` if the identitfier of this Signed Extension object does not line up with the `NAME` constant of the provided Signed Extension type.
pub fn as_signed_extra<S: SignedExtension<T>>(&self) -> Result<Option<S::Decoded>, Error> {
if self.identifier != S::NAME {
return Ok(None);
}
self.as_type::<S::Decoded>().map(Some)
}
}

#[cfg(test)]
Expand Down
36 changes: 34 additions & 2 deletions subxt/src/config/signed_extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::utils::Era;
use crate::{client::OfflineClientT, Config};
use codec::{Compact, Encode};
use core::fmt::Debug;
use scale_decode::DecodeAsType;
use std::collections::HashMap;

/// A single [`SignedExtension`] has a unique name, but is otherwise the
Expand All @@ -21,6 +22,11 @@ pub trait SignedExtension<T: Config>: ExtrinsicParams<T> {
/// The name of the signed extension. This is used to associate it
/// with the signed extensions that the node is making use of.
const NAME: &'static str;

/// The type representing the `extra` bytes of a signed extension.
/// Decoding from this type should be symmetrical to the respective
/// `ExtrinsicParamsEncoder::encode_extra_to()` implementation of this signed extension.
type Decoded: DecodeAsType;
}

/// The [`CheckSpecVersion`] signed extension.
Expand Down Expand Up @@ -48,6 +54,7 @@ impl ExtrinsicParamsEncoder for CheckSpecVersion {

impl<T: Config> SignedExtension<T> for CheckSpecVersion {
const NAME: &'static str = "CheckSpecVersion";
type Decoded = ();
}

/// The [`CheckNonce`] signed extension.
Expand Down Expand Up @@ -75,6 +82,7 @@ impl ExtrinsicParamsEncoder for CheckNonce {

impl<T: Config> SignedExtension<T> for CheckNonce {
const NAME: &'static str = "CheckNonce";
type Decoded = Compact<u64>;
}

/// The [`CheckTxVersion`] signed extension.
Expand Down Expand Up @@ -102,6 +110,7 @@ impl ExtrinsicParamsEncoder for CheckTxVersion {

impl<T: Config> SignedExtension<T> for CheckTxVersion {
const NAME: &'static str = "CheckTxVersion";
type Decoded = ();
}

/// The [`CheckGenesis`] signed extension.
Expand Down Expand Up @@ -134,6 +143,7 @@ impl<T: Config> ExtrinsicParamsEncoder for CheckGenesis<T> {

impl<T: Config> SignedExtension<T> for CheckGenesis<T> {
const NAME: &'static str = "CheckGenesis";
type Decoded = ();
}

/// The [`CheckMortality`] signed extension.
Expand Down Expand Up @@ -213,15 +223,28 @@ impl<T: Config> ExtrinsicParamsEncoder for CheckMortality<T> {

impl<T: Config> SignedExtension<T> for CheckMortality<T> {
const NAME: &'static str = "CheckMortality";
type Decoded = Era;
}

/// The [`ChargeAssetTxPayment`] signed extension.
#[derive(Debug)]
#[derive(Debug, DecodeAsType)]
pub struct ChargeAssetTxPayment {
tip: Compact<u128>,
asset_id: Option<u32>,
}

impl ChargeAssetTxPayment {
/// Tip to the extrinsic author in the native chain token.
pub fn tip(&self) -> u128 {
self.tip.0
}

/// Tip to the extrinsic author using the asset ID given.
pub fn asset_id(&self) -> Option<u32> {
self.asset_id
}
}

/// Parameters to configure the [`ChargeAssetTxPayment`] signed extension.
#[derive(Default)]
pub struct ChargeAssetTxPaymentParams {
Expand Down Expand Up @@ -277,14 +300,22 @@ impl ExtrinsicParamsEncoder for ChargeAssetTxPayment {

impl<T: Config> SignedExtension<T> for ChargeAssetTxPayment {
const NAME: &'static str = "ChargeAssetTxPayment";
type Decoded = Self;
}

/// The [`ChargeTransactionPayment`] signed extension.
#[derive(Debug)]
#[derive(Debug, DecodeAsType)]
pub struct ChargeTransactionPayment {
tip: Compact<u128>,
}

impl ChargeTransactionPayment {
/// Tip to the extrinsic author in the native chain token.
pub fn tip(&self) -> u128 {
self.tip.0
}
}

/// Parameters to configure the [`ChargeTransactionPayment`] signed extension.
#[derive(Default)]
pub struct ChargeTransactionPaymentParams {
Expand Down Expand Up @@ -325,6 +356,7 @@ impl ExtrinsicParamsEncoder for ChargeTransactionPayment {

impl<T: Config> SignedExtension<T> for ChargeTransactionPayment {
const NAME: &'static str = "ChargeTransactionPayment";
type Decoded = Self;
}

/// This accepts a tuple of [`SignedExtension`]s, and will dynamically make use of whichever
Expand Down
6 changes: 5 additions & 1 deletion subxt/src/utils/era.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@
// This file is dual-licensed as Apache-2.0 or GPL-3.0.
// see LICENSE for license details.

use scale_decode::DecodeAsType;

// Dev note: This and related bits taken from `sp_runtime::generic::Era`
/// An era to describe the longevity of a transaction.
#[derive(PartialEq, Default, Eq, Clone, Copy, Debug, serde::Serialize, serde::Deserialize)]
#[derive(
PartialEq, Default, Eq, Clone, Copy, Debug, serde::Serialize, serde::Deserialize, DecodeAsType,
)]
pub enum Era {
/// The transaction is valid forever. The genesis hash must be present in the signed content.
#[default]
Expand Down
52 changes: 40 additions & 12 deletions testing/integration-tests/src/full_client/blocks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
use crate::{test_context, utils::node_runtime};
use codec::{Compact, Encode};
use futures::StreamExt;

use subxt::config::signed_extensions::{ChargeAssetTxPayment, CheckMortality, CheckNonce};
use subxt::config::DefaultExtrinsicParamsBuilder;
use subxt::config::SubstrateConfig;
use subxt::utils::Era;
use subxt_metadata::Metadata;
use subxt_signer::sr25519::dev;

Expand Down Expand Up @@ -272,38 +274,64 @@ async fn decode_signed_extensions_from_blocks() {
}};
}

let expected_signed_extensions = [
"CheckNonZeroSender",
"CheckSpecVersion",
"CheckTxVersion",
"CheckGenesis",
"CheckMortality",
"CheckNonce",
"CheckWeight",
"ChargeAssetTxPayment",
];

let transaction1 = submit_transfer_extrinsic_and_get_it_back!(1234);
let extensions1 = transaction1.signed_extensions().unwrap();
let nonce1 = extensions1.nonce().unwrap();
let nonce1_static = extensions1.find::<CheckNonce>().unwrap().unwrap().0;
let tip1 = extensions1.tip().unwrap();
let tip1_static: u128 = extensions1
.find::<ChargeAssetTxPayment>()
.unwrap()
.unwrap()
.tip();

let transaction2 = submit_transfer_extrinsic_and_get_it_back!(5678);
let extensions2 = transaction2.signed_extensions().unwrap();
let nonce2 = extensions2.nonce().unwrap();
let nonce2_static = extensions2.find::<CheckNonce>().unwrap().unwrap().0;
let tip2 = extensions2.tip().unwrap();
let tip2_static: u128 = extensions2
.find::<ChargeAssetTxPayment>()
.unwrap()
.unwrap()
.tip();

assert_eq!(nonce1, 0);
assert_eq!(nonce1, nonce1_static);
assert_eq!(tip1, 1234);
assert_eq!(tip1, tip1_static);
assert_eq!(nonce2, 1);
assert_eq!(nonce2, nonce2_static);
assert_eq!(tip2, 5678);
assert_eq!(tip2, tip2_static);
Copy link
Collaborator

Choose a reason for hiding this comment

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

it'd be really good I think to also test getting the mortality (just because that returns an enum and would be good to check that it decodes OK! I'm not sure off the top of my head whether the encoded impl is consistent with this or not))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a check at the end of the test to verify Mortality decodes and is Era::Immortal.

Copy link
Collaborator

@jsdw jsdw Nov 2, 2023

Choose a reason for hiding this comment

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

Nice, thank you! That's reassuring :)


let expected_signed_extensions = [
"CheckNonZeroSender",
"CheckSpecVersion",
"CheckTxVersion",
"CheckGenesis",
"CheckMortality",
"CheckNonce",
"CheckWeight",
"ChargeAssetTxPayment",
];

assert_eq!(extensions1.iter().count(), expected_signed_extensions.len());
for (e, expected_name) in extensions1.iter().zip(expected_signed_extensions.iter()) {
assert_eq!(e.unwrap().name(), *expected_name);
}

assert_eq!(extensions2.iter().count(), expected_signed_extensions.len());
for (e, expected_name) in extensions2.iter().zip(expected_signed_extensions.iter()) {
assert_eq!(e.unwrap().name(), *expected_name);
}

// check that era decodes:
for extensions in [&extensions1, &extensions2] {
let era: Era = extensions
.find::<CheckMortality<SubstrateConfig>>()
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's a shame to have to write the Config bound in like this, but unavoidable as it stands!

(We can probably come up with something to avoid this at some point, but all good for now!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree.

.unwrap()
.unwrap();
assert_eq!(era, Era::Immortal)
}
}
Loading