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

Modify existing signedExtension. #342

Closed
nuel77 opened this issue Dec 5, 2023 · 5 comments
Closed

Modify existing signedExtension. #342

nuel77 opened this issue Dec 5, 2023 · 5 comments

Comments

@nuel77
Copy link

nuel77 commented Dec 5, 2023

Hello,

I am quite new to this library and was having some trouble trying to use a modified type for our chains signedExtension. for example in statement , currently the ChargeAssetTxPayment payment type is given as

  assetId: 'TAssetConversion'

But suppose i want to use

assetId: `Option<u128>`

Can this be done within transaction-wrapper core or do i need to define it elsewhere?

@bee344
Copy link
Collaborator

bee344 commented Dec 6, 2023

Hi @nuel77,
Currently, you can't do that with txwrapper. There are improvements planned that will enable this and are high on the priority list, but for now it's not possible. Is there a reason why you are trying to do this with txwrapper instead of PJS?

@nuel77
Copy link
Author

nuel77 commented Dec 6, 2023

One of our partnered centralized exchanges is using the transaction wrapper for offline signing from their end for processing withdrawals, so any problem with that is of the highest priority to us as the users are not able to withdraw to our chain because of this issue. Recently we made a runtime upgrade to integrate pallet-asset-tx-payment pallet on our chain (link here ), with the only difference being the type of asset. AssetHub is using XcmV3Multilocation as the asset type but we use a Option(https://github.com/Polkadex-Substrate/Polkadex/blob/c4fb57e94db244c65c69f28af98523bfc258db71/runtimes/mainnet/src/lib.rs#L1639) in our chain

@bee344
Copy link
Collaborator

bee344 commented Dec 7, 2023

So if you are talking about the SignedExtension, PJS/API, it uses an Option<MultiLocation> or an Option<AssetId> depending on the runtime version, so it would be fine if you use that. We are waiting for a new release of PJS-API that contains this PR that allows txwrapper to use the asset-conversion-tx-payment.

@nuel77
Copy link
Author

nuel77 commented Dec 7, 2023

cool thanks for the info. We were able to solve the issue with a sort of hack, And the withdrawals are processing now. The problem we faced was because of the code here, the defineMethod is hardcoding assetId as type Compact<AssetId>. But for our chain the right type was Option<AssetId>. In the open pr #340 , its also hardcoding it to either Compact<AssetId> or Multilocation is there any way this can be made more generic?

@bee344
Copy link
Collaborator

bee344 commented Dec 12, 2023

@nuel77 thanks for the feedback, we will keep the idea of making it more generic in mind and look into it some time in the future, but it's not currently a top priority, so we will close this as you have found a way around. If you experience any other problems please reach out through an issue and we will look into it.

@bee344 bee344 closed this as completed Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants