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

Add statemint as trustable chain for pendulum + integration tests #192

Merged
merged 19 commits into from
Apr 7, 2023

Conversation

RustNinja
Copy link
Contributor

@RustNinja RustNinja commented Mar 29, 2023

PR related to #174 and #191

  • configured xcm that Statemint is trustable source for reserve transfer assets for Pendulum runtime
  • add integration tests and basic configuration for network
    • parachain (pendulum-runtime)
    • polkadot (polkadot-runtime)
    • statemint (statemint-runtime)
  • use xcm-emulator instead of xcm-simulator
  • add dependency of statemint-runtime from parity cumulus.
    configure statemint runtime for MockNet.
  • register asset with id 1984 to simulate USDT as sufficient on system parachain.
  • cover bidirectional transfer from Statemint to Pendulum and back.
  • unit test to check transfer DOT from pendulum to relay chain

@RustNinja RustNinja self-assigned this Mar 29, 2023
@RustNinja RustNinja linked an issue Mar 29, 2023 that may be closed by this pull request
@RustNinja RustNinja marked this pull request as draft March 29, 2023 21:58
RustNinja and others added 4 commits March 31, 2023 11:17
configure pendulum runtime for xcm simulator
* finished configuration of polkadot + pendulum TestNet

* add transfer_ksm_from_relay_chain_to_pendulum

* add unit tests to integration

* use xcm emulator instead of xcm simulator from polkadot repo. (#194)

* transfer_polkadot_from_relay_chain_to_pendulum works
…fer Pendulum <-> Statemint (#196)

* add statemint-runtime to integration tests

* add Statemint to RelayNet as system chain 1000

* add statemine_transfer_asset_to_pendulum unit tests

* check BOB balance in statemine_transfer_asset_to_pendulum test

* add statemine_transfer_asset_to_statemint

* rename statemine to statemint. because this is tests for polkadot net

* add statemint_transfer_incorrect_asset_to_pendulum_fails

* move pendulum and statemint configuration from lib.rs to setup.rs

* move test from lib.rs to test.rs

* move polkadot net configuration to polkadot_test_net

* split tests, setup and polkadot_test_net to different files

* remove unused import in all files

* rename to pendulum-runtime-integration-tests

* Revert "rename to pendulum-runtime-integration-tests"

This reverts commit 2f18be6.
@RustNinja RustNinja changed the title WIP. Add statemint as trustable chain for pendulum + integration tests Add statemint as trustable chain for pendulum + integration tests Apr 4, 2023
@RustNinja RustNinja marked this pull request as ready for review April 4, 2023 14:38
ebma
ebma previously requested changes Apr 5, 2023
Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

Overall this looks good and has some very valuable changes. Great job @RustNinja 👍

I think I would restructure the folder, so that it is /runtime/integration-tests/src/pendulum and not /runtime/integration-tests/pendulum/src. Maybe we don't need to have that pendulum subfolder anyways.

Also, @RustNinja I assume we would need to change the XCM config for amplitude as well to support non-native assets? Which XCM assets do we actually support with these changes, I suppose it's actually any asset from any chain that we have an open channel to?

runtime/integration-tests/pendulum/src/tests.rs Outdated Show resolved Hide resolved
runtime/integration-tests/pendulum/src/tests.rs Outdated Show resolved Hide resolved
runtime/integration-tests/pendulum/src/tests.rs Outdated Show resolved Hide resolved
runtime/integration-tests/pendulum/src/lib.rs Outdated Show resolved Hide resolved
runtime/integration-tests/pendulum/src/tests.rs Outdated Show resolved Hide resolved
runtime/integration-tests/pendulum/src/tests.rs Outdated Show resolved Hide resolved
* rename dot function ot one

* refactor setup rs file

* add #[cfg(test)] attribute for all modules

* add comments about fees and rename TEN to TEN_UNITS

* update comment and constants name to show what is it

* wrap Parachain into X1 to have the same approach everywhere

* update transfer_dot_from_pendulum_to_relay_chain assert statement

* add. comment statemint_transfer_incorrect_asset_to_pendulum_should_fails

* add comment statemint_transfer_asset_to_statemint

* move use pendulum_runtime::{RuntimeEvent, System}; to top of the file
@RustNinja
Copy link
Contributor Author

Overall this looks good and has some very valuable changes. Great job @RustNinja 👍

I think I would restructure the folder, so that it is /runtime/integration-tests/src/pendulum and not /runtime/integration-tests/pendulum/src. Maybe we don't need to have that pendulum subfolder anyways.

Also, @RustNinja I assume we would need to change the XCM config for amplitude as well to support non-native assets? Which XCM assets do we actually support with these changes, I suppose it's actually any asset from any chain that we have an open channel to?

Here is the all changes that i made in one pr

I would like to keep integration tests separate(in different packages) for each runtime(Pendulum/Amplitude/Foucoco).
The reason is that probably one day we will have a different version of polkadot for different networks. 0.39 for Pendulum and 0.43 for Foucoco. So it will be easy to support integration tests in different packages for each network.

Also, @RustNinja I assume we would need to change the XCM config for amplitude as well to support non-native assets? Which XCM assets do we actually support with these changes, I suppose it's actually any asset from any chain that we have an open channel to?

@ebma
by this changes in xcm_config Pendulum supports only USDT asset_id 1984 from Statemint.
We can enable them everything but we need a task and approval from product to do it.

Amplitude does not have any open channel with other chains. so we do not need to change xcm config for amplitude yet.

@RustNinja RustNinja requested a review from ebma April 5, 2023 13:14
Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

@RustNinja so if I understand correctly, USDT (1984) is the only asset that will work because it's the only asset that we implemented a conversion for in the xcm_config here?

@RustNinja
Copy link
Contributor Author

here
@ebma

Exactly. We declared here that we trust(support) as reserve backed assets only two multilocation:

MultiLocation::parent() for Relay-chain(our parrent)
and

MultiLocation::new(
	1,
	X3(
		Parachain(statemint::PARA_ID),
		PalletInstance(statemint::ASSET_PALLET_ID),
		GeneralIndex(statemint::USDT_ASSET_ID),
	),
))

no other assets until we get approval from product team.

@RustNinja RustNinja requested a review from ebma April 6, 2023 10:33
RustNinja and others added 2 commits April 6, 2023 11:35
@ebma
Copy link
Member

ebma commented Apr 6, 2023

Now that you added the changes to the Foucoco XCM config as well, maybe we should go ahead and change it also for Amplitude? Would it look any different for Amplitude compared to the other two?

@RustNinja
Copy link
Contributor Author

RustNinja commented Apr 6, 2023

Now that you added the changes to the Foucoco XCM config as well, maybe we should go ahead and change it also for Amplitude? Would it look any different for Amplitude compared to the other two?

@ebma

I added the changes to Foucoco to first upgrade foucoco runtime before Pendulum runtime upgrade and check that it will works with DOT transfer from relay chain to our chain and back.
Anyway it works in integration tests and i just want to use this new xcm settings for Foucoco.
for Amplitude there is no any other reserve backed assets from other chain that we should support so i decide to not touch amplitude runtime without actual requirements for it.
But you right. There should not be any difference in xcm config for pendulum and amplitude.
So let me know if you fill that i should also change amplitude in this PR.

Cargo.toml Outdated Show resolved Hide resolved
runtime/pendulum/src/lib.rs Show resolved Hide resolved
@RustNinja RustNinja requested a review from TorstenStueber April 6, 2023 14:24
@ebma
Copy link
Member

ebma commented Apr 6, 2023

for Amplitude there is no any other reserve backed assets from other chain that we should support so i decide to not touch amplitude runtime without actual requirements for it.

I see. Well, the reasoning makes sense, I would just be afraid that we miss or forget about it once we need it. But if there really is no need to have it on Amplitude yet then let's not add it there.

@RustNinja RustNinja merged commit a75ef03 into main Apr 7, 2023
@RustNinja RustNinja deleted the add-statemint-as-trustable-chain-for-pendulum branch April 7, 2023 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants