Closed jsdw closed 11 months ago
@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?
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?
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
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:
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/ortrait 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 orAssetId
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:
ChargeAssetTxPayment
to specify type of asset ID / location.AssetId
type to Config
trait. This ttype should trickly into DefaultExtrinsicParams
/DefaultExtrinsicParamsBuilder
to set that ChargeAssetTxPayment
type.PolkadotConfig
set this AssetId
type to u32
.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).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 configI think this has a nice balance of:
SubstrateConfig
for "Just Works" behaviour at a little type safety cost.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.
See https://github.com/bee344/subxt/tree/anp-asset-conversion also; a custom impl that might shed some light on things too.
@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.
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:
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 :)
ChargeAssetTxPayment
is generic over the asset ID type. On the kitchensink runtime it'su32
, and so that's what was implemented (actually this was done a while ago). On Asset Hub, it'sMultiLocation
.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 foru32
orMultiLocation
. 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 theConfig
, and then use this to determine the type in theChargeAssetTxPayment
signed extension impl. This would generally be cleaner, but has the downsides of:Config
a little more complex; one more type to know how to configureConfig
impl to be used across these different chains, rather than one config that will work with both.Advantages though would be:
MultiLocation
(and then ignore one or the other. It's more "correct".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.