-
Notifications
You must be signed in to change notification settings - Fork 248
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
ChargeAssetTxPayment: support providing u32 or MultiLocation in default impl #1227
ChargeAssetTxPayment: support providing u32 or MultiLocation in default impl #1227
Conversation
Offhand the approach here looks good to me; keeps it simple and shows the user how to support MultiLocations via generated code or whatever :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
There's still a bit of me that is wondering whether it's better to expose AssetId
as a config param like this, or to have an AssetId
generic on ChargeAssetTxPayment
and default it to u32, but then people can change it there instead.
Given that the signed extension and need to configure this feels like a fairly common thing, I do think that the approach in this PR is best for now though!
@@ -51,6 +51,9 @@ pub trait Config: Sized + Send + Sync + 'static { | |||
|
|||
/// This type defines the extrinsic extra and additional parameters. | |||
type ExtrinsicParams: ExtrinsicParams<Self>; | |||
|
|||
/// Asset Id | |||
type AssetId: Debug + Encode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could also impl EncodeAsType
possibly (same as a couple of the others above like singature I think), which would at least mean that if somebody tried setting a tip with an asset ID, and it was configured wrong, we'd get an EncodeAsType
error back (eg because it failed to encode an Option<u32>
into an Option<MultiLocation>
) rather than have some error from the node that it failed to decode the tx provided.
For now I'd leave it as is, but we should probably consider moving some things in Config to EncodeAsType
instead of Encode
so that we get this extra protection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth an issue to not forget about it 🙏
Co-authored-by: James Wilson <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one!
…lt impl (#1227) * Asset Id in Config trait * add example configuring the config * fmt * fix Default trait bound * merge examples, fix default again * adjust config in examples * Update subxt/src/config/mod.rs Co-authored-by: James Wilson <[email protected]> --------- Co-authored-by: James Wilson <[email protected]>
* skeleton commit * signed extension decoding * fix some minor things * make api more similar to Extrinsics * defer decoding of signed extensions * fix byte slices * add test for nonce signed extension * adjust test and extend for tip * clippy * support both ChargeTransactionPayment and ChargeAssetTxPayment * address PR comments * Extend lifetimes, expose pub structs, remove as_type * add signed extensions to block subscribing example * add Decoded type * fix merging bug and tests * add decoded type in CustomSignedExtension * fix minor issues, extend test * cargo fmt differences * remove the `decoded` function * new as_signed_extra fn, do not expose as_type anymore * fix Result-Option order, simplify obtaining Nonce * tx: Remove `wait_for_in_block` helper method (#1237) Signed-off-by: Alexandru Vasile <[email protected]> * Update smoldot to 0.12 (#1212) * Update lightclient Signed-off-by: Alexandru Vasile <[email protected]> * testing: Fix typo Signed-off-by: Alexandru Vasile <[email protected]> * testing: Update cargo.toml Signed-off-by: Alexandru Vasile <[email protected]> * lightclient: Add tracing logs to improve debugging Signed-off-by: Alexandru Vasile <[email protected]> * lightclient: Add socket buffers module for `PlatformRef` Signed-off-by: Alexandru Vasile <[email protected]> * lightclient: Update `SubxtPlatform` Signed-off-by: Alexandru Vasile <[email protected]> * cargo: Add lightclient dependencies Signed-off-by: Alexandru Vasile <[email protected]> * Update cargo.lock of wasm tests Signed-off-by: Alexandru Vasile <[email protected]> * lightclient: Add constant for with-buffer module Signed-off-by: Alexandru Vasile <[email protected]> * lightclient: Replace rand crate with getrandom Signed-off-by: Alexandru Vasile <[email protected]> * example: Update cargo lock file Signed-off-by: Alexandru Vasile <[email protected]> * examples: Update deps Signed-off-by: Alexandru Vasile <[email protected]> --------- Signed-off-by: Alexandru Vasile <[email protected]> Co-authored-by: Tadeo Hepperle <[email protected]> * ChargeAssetTxPayment: support providing u32 or MultiLocation in default impl (#1227) * Asset Id in Config trait * add example configuring the config * fmt * fix Default trait bound * merge examples, fix default again * adjust config in examples * Update subxt/src/config/mod.rs Co-authored-by: James Wilson <[email protected]> --------- Co-authored-by: James Wilson <[email protected]> * generic AssetId * fix generics * fmt --------- Signed-off-by: Alexandru Vasile <[email protected]> Co-authored-by: James Wilson <[email protected]> Co-authored-by: Alexandru Vasile <[email protected]>
fixes #1162