ChargeAssetTxPayment: support providing u32 or MultiLocation in default impl

[jsdw]

ChargeAssetTxPayment is generic over the asset ID type. On the kitchensink runtime it's u32, and so that's what was implemented (actually this was done a while ago). On Asset Hub, it's MultiLocation.

Subxt decides which signed extensions from the provided ones to use based on their names. Since the name is the same here and yet the types/encoding is different, we can't just add some other signed extension which supports MultiLocations to the list.

Instead, we can use the metadata in the ChargeAssetTxPayment extension to see what the asset ID type is, and have special handling for u32 or MultiLocation. If other chains use different parameters for the extension, then those won't be supported and they will have to implement their own version.

Alternative

The alternative option is to add an AssetId or something to the Config, and then use this to determine the type in the ChargeAssetTxPayment signed extension impl. This would generally be cleaner, but has the downsides of:

• Making Config a little more complex; one more type to know how to configure
• Requiring a different Config impl to be used across these different chains, rather than one config that will work with both.
• More of a breaking change; Configs all need extending with a type that only has very niche usage.

Advantages though would be:

• We don't have to manually provide an API on the signed extension params to set a u32 and a MultiLocation (and then ignore one or the other. It's more "correct".
• The signed extension would support any valid type here and not just the two we hardcode support for.

There might be a middle ground option too wherein we provide only one API to add an Asset ID, and accept some type which impls EncodeAsType, and then use the signed extension metadata to try to encode it appropriately, or error otherwise. No extra Config type or extra API needed, but the downside that it's much easier to provide invalid types.

I'm going back and forth on which option is best here.

[tadeohepperle]

@jsdw I looked into this a bit, but cannot find the ChargeAssetTxPayment signed extension in the extrinsic extensions in the metadata returned for Polkadot and Assethub (wss://rpc.polkadot.io and wss://dot-rpc.stakeworld.io). Also retrieved the signed extensions for a build of polkadot from the current master branch and they also do not contain ChargeAssetTxPayment:

[
    SignedExtensionMetadata {
        identifier: "CheckNonZeroSender",
        extra_ty: 830,
        additional_ty: 34,
    },
    SignedExtensionMetadata {
        identifier: "CheckSpecVersion",
        extra_ty: 831,
        additional_ty: 4,
    },
    SignedExtensionMetadata {
        identifier: "CheckTxVersion",
        extra_ty: 832,
        additional_ty: 4,
    },
    SignedExtensionMetadata {
        identifier: "CheckGenesis",
        extra_ty: 833,
        additional_ty: 12,
    },
    SignedExtensionMetadata {
        identifier: "CheckMortality",
        extra_ty: 834,
        additional_ty: 12,
    },
    SignedExtensionMetadata {
        identifier: "CheckNonce",
        extra_ty: 836,
        additional_ty: 34,
    },
    SignedExtensionMetadata {
        identifier: "CheckWeight",
        extra_ty: 837,
        additional_ty: 34,
    },
    SignedExtensionMetadata {
        identifier: "ChargeTransactionPayment",
        extra_ty: 838,
        additional_ty: 34,
    },
    SignedExtensionMetadata {
        identifier: "PrevalidateAttests",
        extra_ty: 839,
        additional_ty: 34,
    },
]

Am I doing something wrong?

[jsdw]

Polkadot doesn't have it (you can't have different types of assets on Polkadot, only DOTs, so no need for it there); Polkadot instaed has ChargeTransactionPayment which is the simpler version of it.

The default substrate node (substrate --dev) has ChargeAssetTxPayment and Asset Hub does as well (eg see https://github.com/paritytech/polkadot-sdk/blob/7cbe0c76ef8fd2aabf9f07de0156941ce3ed44b0/cumulus/parachains/runtimes/assets/asset-hub-polkadot/src/lib.rs#L794); try eg wss://polkadot-asset-hub-rpc.polkadot.io for asset hub on polkadot?

[tadeohepperle]

Oh yeah, you are right, I can see it via wss://polkadot-asset-hub-rpc.polkadot.io. Hmm I wonder why the wss://dot-rpc.stakeworld.io url did not have it, both are listed under Asset Hub on polkadot.js

[jsdw]

Putting another thought on options out there:

We make ChargeAssetTxPayment generic over the type of asset (u32 or multilocation for instance), so eg ChargeAssetTxPayment<AssetId>.

Then, concrete instances of this can decide what the asset type is.

Then, for our DefaultExtrinsicParams, we either:

1. Set that type to be something like an enum AssetId { Id(u32), MultiLocation(MultiLocation) } which has From impls to convert from u32/MultiLocation into it, and then we will try to encode that type, failing if it's the wrong one for the chain (maybe need v2 and v3 MultiLocations). This works for common chains but less type safe (you can provide a u32 when you ened a MultiLocation and vice versa), and/or
2. Have a separate trait DefaultExtrinsicParamsConfig { type AssetId } trait (or just a generic param for now) that can be used to configure DefaultExtrinsicParams while not messing with the "top level" Config. This keeps extrinsic param specific stuff in its own space, but we then need defaults for that too etc. Maybe default the value to (1) for now or
3. Add AssetId to Config. The most type safety, and keeps just one Config thing (which maybe is nicer than having multiple floating about), but less good that we have to add another type to our Config.

Maybe we should do 1 and 2. Specifically I mean:

• Add generic param to ChargeAssetTxPayment to specify type of asset ID / location.
• Add AssetId type to Config trait. This ttype should trickly into DefaultExtrinsicParams/DefaultExtrinsicParamsBuilder to set that ChargeAssetTxPayment type.
• For PolkadotConfig set this AssetId type to u32.
• Maybe for SubstrateConfig set the type to enum AssetId { Id(u32), MultiLocation(MultiLocation) } (may want V2MultiLocation) for maximum flexibility when using this config (we sortof want it to be quite a generic one I think, so are willing to trade some type safety here).
• Maybe add AssetHubConfig with the type set to the relevant MultiLocation type?
• DefaultExtrinsicParams accepts impl Into<T::AssetId> so that one can transparently provide a u32 or a MultiLocation in the default case with Substrate config

I think this has a nice balance of:

• Best compile time safety when using a specific chain like Polkadot or AssetHub.
• Can alter the Config to work with custom chains fairly easily, or use SubstrateConfig for "Just Works" behaviour at a little type safety cost.
• Most people won't need to care; the Config they use will just keep working. (and, unless they want to set the asset ID for a tip other than the default, people won't actually even need to care if it's not configured properly).

I'm not a huge fan of adding a type to Config, but the alternative is just adding some extra generic param on DefaultExtrinsicParams (why, when Config is already provided), or just accepting that we'll not be as type safe anywhere by default (this might still be an OK trade off, but I see little reason not to improve the type safety for our default configs.

[jsdw]

See https://github.com/bee344/subxt/tree/anp-asset-conversion also; a custom impl that might shed some light on things too.

[tadeohepperle]

@jsdw I like the approach you proposed and implemented it so far, but I am not sure if we should provide a complete copy of the MultiLocation type in subxt. The type is pretty big and complicated (both v2 and v3) and I think people who want to build on top of it can pull it in from the staging_xcm crate or the generated static interface if they want to. I think it would be quite hard to keep such a MultiLocation type in sync with the staging_xcm crate. But we could pull in staging_xcm where the original MultiLocation type is defined behind the sp-compat feature flag maybe and then provide an AssetHubConfig if the feature flag is enabled.

[jsdw]

Pulling in the crate looks like it's fairly heavy in terms of deps unfortunately, eg:

 % cargo tree -e no-build,no-dev --no-default-features
staging-xcm v1.0.0 (/Users/james/Work/polkadot-sdk/polkadot/xcm)
├── bounded-collections v0.1.8
│   ├── log v0.4.20
│   ├── parity-scale-codec v3.6.4
│   │   ├── arrayvec v0.7.4
│   │   ├── byte-slice-cast v1.2.2
│   │   ├── bytes v1.4.0
│   │   ├── impl-trait-for-tuples v0.2.2 (proc-macro)
│   │   │   ├── proc-macro2 v1.0.67
│   │   │   │   └── unicode-ident v1.0.11
│   │   │   ├── quote v1.0.33
│   │   │   │   └── proc-macro2 v1.0.67 (*)
│   │   │   └── syn v1.0.109
│   │   │       ├── proc-macro2 v1.0.67 (*)
│   │   │       ├── quote v1.0.33 (*)
│   │   │       └── unicode-ident v1.0.11
│   │   └── parity-scale-codec-derive v3.6.4 (proc-macro)
│   │       ├── proc-macro-crate v1.3.1
│   │       │   ├── once_cell v1.18.0
│   │       │   └── toml_edit v0.19.14
│   │       │       ├── indexmap v2.0.0
│   │       │       │   ├── equivalent v1.0.1
│   │       │       │   └── hashbrown v0.14.0
│   │       │       ├── toml_datetime v0.6.3
│   │       │       └── winnow v0.5.15
│   │       ├── proc-macro2 v1.0.67 (*)
│   │       ├── quote v1.0.33 (*)
│   │       └── syn v1.0.109 (*)
│   ├── scale-info v2.9.0
│   │   ├── cfg-if v1.0.0
│   │   ├── derive_more v0.99.17 (proc-macro)
│   │   │   ├── proc-macro2 v1.0.67 (*)
│   │   │   ├── quote v1.0.33 (*)
│   │   │   └── syn v1.0.109 (*)
│   │   ├── parity-scale-codec v3.6.4 (*)
│   │   ├── scale-info-derive v2.9.0 (proc-macro)
│   │   │   ├── proc-macro-crate v1.3.1 (*)
│   │   │   ├── proc-macro2 v1.0.67 (*)
│   │   │   ├── quote v1.0.33 (*)
│   │   │   └── syn v1.0.109 (*)
│   │   └── serde v1.0.188
│   │       └── serde_derive v1.0.188 (proc-macro)
│   │           ├── proc-macro2 v1.0.67 (*)
│   │           ├── quote v1.0.33 (*)
│   │           └── syn v2.0.37
│   │               ├── proc-macro2 v1.0.67 (*)
│   │               ├── quote v1.0.33 (*)
│   │               └── unicode-ident v1.0.11
│   └── serde v1.0.188 (*)
├── derivative v2.2.0 (proc-macro)
│   ├── proc-macro2 v1.0.67 (*)
│   ├── quote v1.0.33 (*)
│   └── syn v1.0.109 (*)
├── environmental v1.1.4
├── impl-trait-for-tuples v0.2.2 (proc-macro) (*)
├── log v0.4.20
├── parity-scale-codec v3.6.4 (*)
├── scale-info v2.9.0 (*)
├── serde v1.0.188 (*)
├── sp-weights v20.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/weights)
│   ├── parity-scale-codec v3.6.4 (*)
│   ├── scale-info v2.9.0 (*)
│   ├── serde v1.0.188 (*)
│   ├── smallvec v1.11.0
│   ├── sp-arithmetic v16.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/arithmetic)
│   │   ├── integer-sqrt v0.1.5
│   │   │   └── num-traits v0.2.16
│   │   ├── num-traits v0.2.16
│   │   ├── parity-scale-codec v3.6.4 (*)
│   │   ├── scale-info v2.9.0 (*)
│   │   ├── serde v1.0.188 (*)
│   │   ├── sp-std v8.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/std)
│   │   └── static_assertions v1.1.0
│   ├── sp-core v21.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/core)
│   │   ├── array-bytes v6.1.0
│   │   ├── bitflags v1.3.2
│   │   ├── blake2 v0.10.6
│   │   │   └── digest v0.10.7
│   │   │       ├── block-buffer v0.10.4
│   │   │       │   └── generic-array v0.14.7
│   │   │       │       └── typenum v1.16.0
│   │   │       ├── crypto-common v0.1.6
│   │   │       │   ├── generic-array v0.14.7 (*)
│   │   │       │   └── typenum v1.16.0
│   │   │       └── subtle v2.4.1
│   │   ├── bounded-collections v0.1.8 (*)
│   │   ├── bs58 v0.5.0
│   │   ├── hash-db v0.16.0
│   │   ├── hash256-std-hasher v0.15.2
│   │   │   └── crunchy v0.2.2
│   │   ├── impl-serde v0.4.0
│   │   │   └── serde v1.0.188 (*)
│   │   ├── log v0.4.20
│   │   ├── merlin v2.0.1
│   │   │   ├── byteorder v1.4.3
│   │   │   ├── keccak v0.1.4
│   │   │   │   └── cpufeatures v0.2.9
│   │   │   │       └── libc v0.2.147
│   │   │   ├── rand_core v0.5.1
│   │   │   └── zeroize v1.6.0
│   │   │       └── zeroize_derive v1.4.2 (proc-macro)
│   │   │           ├── proc-macro2 v1.0.67 (*)
│   │   │           ├── quote v1.0.33 (*)
│   │   │           └── syn v2.0.37 (*)
│   │   ├── parity-scale-codec v3.6.4 (*)
│   │   ├── paste v1.0.14 (proc-macro)
│   │   ├── primitive-types v0.12.1
│   │   │   ├── fixed-hash v0.8.0
│   │   │   │   └── static_assertions v1.1.0
│   │   │   ├── impl-codec v0.6.0
│   │   │   │   └── parity-scale-codec v3.6.4 (*)
│   │   │   ├── impl-serde v0.4.0 (*)
│   │   │   ├── scale-info v2.9.0 (*)
│   │   │   └── uint v0.9.5
│   │   │       ├── byteorder v1.4.3
│   │   │       ├── crunchy v0.2.2
│   │   │       ├── hex v0.4.3
│   │   │       └── static_assertions v1.1.0
│   │   ├── scale-info v2.9.0 (*)
│   │   ├── schnorrkel v0.9.1
│   │   │   ├── arrayref v0.3.7
│   │   │   ├── arrayvec v0.5.2
│   │   │   ├── curve25519-dalek v2.1.3
│   │   │   │   ├── byteorder v1.4.3
│   │   │   │   ├── digest v0.8.1
│   │   │   │   │   └── generic-array v0.12.4
│   │   │   │   │       └── typenum v1.16.0
│   │   │   │   ├── rand_core v0.5.1
│   │   │   │   ├── subtle v2.4.1
│   │   │   │   └── zeroize v1.6.0 (*)
│   │   │   ├── merlin v2.0.1 (*)
│   │   │   ├── rand_core v0.5.1
│   │   │   ├── sha2 v0.8.2
│   │   │   │   ├── block-buffer v0.7.3
│   │   │   │   │   ├── block-padding v0.1.5
│   │   │   │   │   │   └── byte-tools v0.3.1
│   │   │   │   │   ├── byte-tools v0.3.1
│   │   │   │   │   ├── byteorder v1.4.3
│   │   │   │   │   └── generic-array v0.12.4 (*)
│   │   │   │   ├── digest v0.8.1 (*)
│   │   │   │   ├── fake-simd v0.1.2
│   │   │   │   └── opaque-debug v0.2.3
│   │   │   ├── subtle v2.4.1
│   │   │   └── zeroize v1.6.0 (*)
│   │   ├── secrecy v0.8.0
│   │   │   └── zeroize v1.6.0 (*)
│   │   ├── serde v1.0.188 (*)
│   │   ├── sp-core-hashing v9.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/core/hashing)
│   │   │   ├── blake2b_simd v1.0.1
│   │   │   │   ├── arrayref v0.3.7
│   │   │   │   ├── arrayvec v0.7.4
│   │   │   │   └── constant_time_eq v0.2.6
│   │   │   ├── byteorder v1.4.3
│   │   │   ├── digest v0.10.7 (*)
│   │   │   ├── sha2 v0.10.7
│   │   │   │   ├── cfg-if v1.0.0
│   │   │   │   ├── cpufeatures v0.2.9 (*)
│   │   │   │   └── digest v0.10.7 (*)
│   │   │   ├── sha3 v0.10.8
│   │   │   │   ├── digest v0.10.7 (*)
│   │   │   │   └── keccak v0.1.4 (*)
│   │   │   └── twox-hash v1.6.3
│   │   │       ├── cfg-if v1.0.0
│   │   │       ├── digest v0.10.7 (*)
│   │   │       └── static_assertions v1.1.0
│   │   ├── sp-debug-derive v8.0.0 (proc-macro) (/Users/james/Work/polkadot-sdk/substrate/primitives/debug-derive)
│   │   │   ├── proc-macro2 v1.0.67 (*)
│   │   │   ├── quote v1.0.33 (*)
│   │   │   └── syn v2.0.37 (*)
│   │   ├── sp-runtime-interface v17.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/runtime-interface)
│   │   │   ├── bytes v1.4.0
│   │   │   ├── impl-trait-for-tuples v0.2.2 (proc-macro) (*)
│   │   │   ├── parity-scale-codec v3.6.4 (*)
│   │   │   ├── primitive-types v0.12.1 (*)
│   │   │   ├── sp-externalities v0.19.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/externalities)
│   │   │   │   ├── environmental v1.1.4
│   │   │   │   ├── parity-scale-codec v3.6.4 (*)
│   │   │   │   ├── sp-std v8.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/std)
│   │   │   │   └── sp-storage v13.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/storage)
│   │   │   │       ├── impl-serde v0.4.0 (*)
│   │   │   │       ├── parity-scale-codec v3.6.4 (*)
│   │   │   │       ├── ref-cast v1.0.20
│   │   │   │       │   └── ref-cast-impl v1.0.20 (proc-macro)
│   │   │   │       │       ├── proc-macro2 v1.0.67 (*)
│   │   │   │       │       ├── quote v1.0.33 (*)
│   │   │   │       │       └── syn v2.0.37 (*)
│   │   │   │       ├── serde v1.0.188 (*)
│   │   │   │       ├── sp-debug-derive v8.0.0 (proc-macro) (/Users/james/Work/polkadot-sdk/substrate/primitives/debug-derive) (*)
│   │   │   │       └── sp-std v8.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/std)
│   │   │   ├── sp-runtime-interface-proc-macro v11.0.0 (proc-macro) (/Users/james/Work/polkadot-sdk/substrate/primitives/runtime-interface/proc-macro)
│   │   │   │   ├── Inflector v0.11.4
│   │   │   │   │   ├── lazy_static v1.4.0
│   │   │   │   │   └── regex v1.9.3
│   │   │   │   │       ├── aho-corasick v1.0.4
│   │   │   │   │       │   └── memchr v2.5.0
│   │   │   │   │       ├── memchr v2.5.0
│   │   │   │   │       ├── regex-automata v0.3.6
│   │   │   │   │       │   ├── aho-corasick v1.0.4 (*)
│   │   │   │   │       │   ├── memchr v2.5.0
│   │   │   │   │       │   └── regex-syntax v0.7.4
│   │   │   │   │       └── regex-syntax v0.7.4
│   │   │   │   ├── proc-macro-crate v1.3.1 (*)
│   │   │   │   ├── proc-macro2 v1.0.67 (*)
│   │   │   │   ├── quote v1.0.33 (*)
│   │   │   │   └── syn v2.0.37 (*)
│   │   │   ├── sp-std v8.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/std)
│   │   │   ├── sp-storage v13.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/storage) (*)
│   │   │   ├── sp-tracing v10.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/tracing)
│   │   │   │   ├── parity-scale-codec v3.6.4 (*)
│   │   │   │   ├── sp-std v8.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/std)
│   │   │   │   ├── tracing v0.1.37
│   │   │   │   │   ├── cfg-if v1.0.0
│   │   │   │   │   ├── pin-project-lite v0.2.13
│   │   │   │   │   └── tracing-core v0.1.31
│   │   │   │   └── tracing-core v0.1.31
│   │   │   ├── sp-wasm-interface v14.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/wasm-interface)
│   │   │   │   ├── impl-trait-for-tuples v0.2.2 (proc-macro) (*)
│   │   │   │   ├── parity-scale-codec v3.6.4 (*)
│   │   │   │   └── sp-std v8.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/std)
│   │   │   └── static_assertions v1.1.0
│   │   ├── sp-std v8.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/std)
│   │   ├── sp-storage v13.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/storage) (*)
│   │   ├── ss58-registry v1.43.0
│   │   └── zeroize v1.6.0 (*)
│   ├── sp-debug-derive v8.0.0 (proc-macro) (/Users/james/Work/polkadot-sdk/substrate/primitives/debug-derive) (*)
│   └── sp-std v8.0.0 (/Users/james/Work/polkadot-sdk/substrate/primitives/std)
└── xcm-procedural v1.0.0 (proc-macro) (/Users/james/Work/polkadot-sdk/polkadot/xcm/procedural)
    ├── Inflector v0.11.4 (*)
    ├── proc-macro2 v1.0.67 (*)
    ├── quote v1.0.33 (*)
    └── syn v2.0.37 (*)

I'd rather avoid hiding this behind substrate-compat if possible, because I don't want to encourage or force anybody to need that feature flag for doing anything in Subxt ideally, so offhand I think we have two options:

1. Try copying the types into subxt. I think that they shouldn't change (but can't be sure). We also only need to be scale-encoding compatible so we don't need to copy eg bounded vecs etc. It might be worth a go trying this just to see how complicated it ends up being, and assessing our approach then.
2. If that doesn't work out, then we can still add AssetId to the Config trait, but just not provide one with multilocation and instead provide an example of how one can plug in the generated MultiLocation type (or the staging-xcm one) themselves to the Config to make it work nicely for a chain.

Might be worth trying both of these to see what they are like, and if we go for 2 it'd be good to double check that it works as expected too :)