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

test: add liquid assets script to test network #312

Merged
merged 2 commits into from
Nov 6, 2023
Merged

Conversation

bee344
Copy link
Contributor

@bee344 bee344 commented Nov 3, 2023

Two questions I have regarding this:

  • Currently I have to force pjs to accept <any> when dealing with the asset-conversion-pallet, because the types it expects come from the substrate-node implementation and are incompatible with the Kusama Asset Hub implementation, but due to this I have to disable eslint for those two lines.
const createLiquidityPoolCall = (api: ApiPromise) => {
	// For now, we have to override the types of the Assets until PJS is updated
	// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
	return api.tx.assetConversion.createPool(<any>native, <any>asset);
};

const addLiquidityCall = (api: ApiPromise, amountNative: number, amountAsset: number, to: KeyringPair) => {
	// For now, we have to override the types of the Assets until PJS is updated
	// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
	return api.tx.assetConversion.addLiquidity(<any>native, <any>asset, amountNative, amountAsset, 0, 0, to.address);
};
  • Trappist doesn't support asset-conversion so all the process runs on KAH, wondering how we feel about that.

Edit: Issue to add asset-conversion to trappist.
Edit 2: Types on PJS that are used on Asset Conversion

const createLiquidityPoolCall = (api: ApiPromise) => {
// For now, we have to override the types of the Assets until PJS is updated
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
return api.tx.assetConversion.createPool(<any>native, <any>asset);
Copy link
Member

Choose a reason for hiding this comment

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

This is totally fine with me. I double checked the ts compilter issue happening with PJs and this is the right approach.

const addLiquidityCall = (api: ApiPromise, amountNative: number, amountAsset: number, to: KeyringPair) => {
// For now, we have to override the types of the Assets until PJS is updated
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
return api.tx.assetConversion.addLiquidity(<any>native, <any>asset, amountNative, amountAsset, 0, 0, to.address);
Copy link
Member

Choose a reason for hiding this comment

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

Same with this, totally fine.

@TarikGul
Copy link
Member

TarikGul commented Nov 6, 2023

Trappist doesn't support asset-conversion so all the process runs on KAH, wondering how we feel about that.

I think this should be totally fine, not much we can do. That being said if we can still create the liquid pool assets thats a step forward. In this case though we just can't transfer them to Trappist if I understand this correctly?

Copy link
Member

@TarikGul TarikGul left a comment

Choose a reason for hiding this comment

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

Overall, amazing script. LGTM!

@bee344
Copy link
Contributor Author

bee344 commented Nov 6, 2023

Trappist doesn't support asset-conversion so all the process runs on KAH, wondering how we feel about that.

I think this should be totally fine, not much we can do. That being said if we can still create the liquid pool assets thats a step forward. In this case though we just can't transfer them to Trappist if I understand this correctly?

Correct, right now there's no way to send the LP tokens to Trappist, but we create them just fine and we can even send them between accounts, so that's ok.

@bee344 bee344 changed the title test: add liquid assets script to test network [WIP] test: add liquid assets script to test network Nov 6, 2023
@TarikGul
Copy link
Member

TarikGul commented Nov 6, 2023

Trappist doesn't support asset-conversion so all the process runs on KAH, wondering how we feel about that.

I think this should be totally fine, not much we can do. That being said if we can still create the liquid pool assets thats a step forward. In this case though we just can't transfer them to Trappist if I understand this correctly?

Correct, right now there's no way to send the LP tokens to Trappist, but we create them just fine and we can even send them between accounts, so that's ok.

Okay I think that's perfect then and the script does exactly what it intends on doing. Feel free to merge it in, whenever your'e ready

@bee344 bee344 merged commit 55e531d into main Nov 6, 2023
13 checks passed
@bee344 bee344 deleted the anp-liqui-assets-script branch November 6, 2023 13:55
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

Successfully merging this pull request may close these issues.

2 participants