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

Feature/plmc 290 xcm instruction that reveals polimecs balance and pallet #114

Conversation

JuaniRios
Copy link
Contributor

@JuaniRios JuaniRios commented Nov 9, 2023

Objective

  • Do necessary checks to make sure we can start the migration of CT tokens without failing.

Tech

  • Excute XCM instructions QueryPallet and ReportHolding on the project chain after HRMP has been established
  • Receive the response on pallet-funding and evaluate that ReportHolding returns enough native tokens to migrate, and the polimec receiver pallet is there to use.
    • Note that right now we expect a full struct of PalletInfo because the fields are private and we cannot retrieve values and compare/store them. More importantly this means we have to expect the pallet to have a certain index which is dumb, but will be addressed in Polkadot-SDK PR #2231, after which we can store the index and use it for the migration.

Main Changes

  • New extrinsics:
    • start_migration_readiness_check: called automatically when HRMP is established, and by the issuer when the check failed, so we can retry.
    • migration_check_response: called by the chain after processing our QueryPallet and ReportHolding instructions.
  • HRMP logic moved to a do_function called by the xcm-executor impl.
  • Custom penpal added to the emulator, because we need to get free execution by the target chain. This is a design decision we should discuss.
    • My thinking is that the polimec soverign account should hold EXACTLY the amount of CT bought, so there's no confusion. And the migration should not be paid by us.
    • This includes the issuer adding enough tokens to pay for execution on the Polimec account, because then we cannot ensure all the transactions will go through.
    • We could argue that the fact that we can execute for free the query, does not mean we can execute for free all migrations, so maybe we should implement a dummy migration to check that it passes the barrier too.
  • New Polimec receiver pallet that is now a pallet-template but is used for querying its existence.
  • pallet-xcm is now tightly coupled to pallet-funding. This is because we need to let pallet_xcm know we expect a query response, before the executor receives that instruction. Open to any solutions that allow us to decouple it.
    • This means we also need to implement pallet xcm and executor in the mock runtime of pallet-funding. Even though we shouldn't use any xcm features on pallet tests since xcm does not work there.
  • A new integration test on the emulator for the query and response behaviour.
    • For this we needed the Instantiator, and we couldn't access it through the test config, so I did the quick approach which is the other access of both "std","testing-node". This means we cannot call the tests just by doing cargo test, but need to call cargo test --features std,testing-node. For more info why, check this link
    • Since the HRMP establishement we implemented in Feature/plmc 287 parachain information discovery and channel opening #94 is not possible to replicate in the emulator (because the emulator mocks the actual messaging protocol), we had to go around it by mocking the behaviour.
    • Small format changes

automatic hrmp connection working

Genesis instantiator usage and first draft of HRMP connection

formatting

feature propagation

cleanup

new node functioning. genesis not yet sure if working

new node functioning. genesis not yet sure if working

somehow compiling and test passing

save

save

save

feat(287): para_id setting extrinsic implemented and tested

save

same log crate across workspace

same log crate across workspace

save

save

save

feat(287): changed tight couple of pallet_xcm by extracting sender trait

feat(287): first commit

feat(285): POC Hrmp automatic acceptance
# Conflicts:
#	Cargo.lock
#	Cargo.toml
#	integration-tests/src/tests/mod.rs
#	pallets/funding/Cargo.toml
#	pallets/funding/src/functions.rs
#	pallets/funding/src/instantiator.rs
#	pallets/funding/src/lib.rs
#	pallets/funding/src/mock.rs
#	pallets/funding/src/types.rs
#	pallets/xcm-executor/src/config.rs
#	pallets/xcm-executor/src/lib.rs
#	runtimes/standalone/src/lib.rs
#	runtimes/testnet/src/xcm_config.rs
Copy link

linear bot commented Nov 9, 2023

@JuaniRios JuaniRios marked this pull request as draft November 9, 2023 12:09
@JuaniRios JuaniRios self-assigned this Nov 9, 2023
@JuaniRios JuaniRios marked this pull request as ready for review November 9, 2023 12:33
…s-polimecs-balance-and-pallet

# Conflicts:
#	Cargo.toml
@JuaniRios
Copy link
Contributor Author

/bot fmt

Copy link

…s-polimecs-balance-and-pallet

# Conflicts:
#	Cargo.toml
#	integration-tests/src/tests/mod.rs
Copy link
Member

@lrazovic lrazovic left a comment

Choose a reason for hiding this comment

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

Just few nitpicks that I would love to address offline, but overall a huge new set of feature towards the MVP!

integration-tests/src/constants.rs Outdated Show resolved Hide resolved
integration-tests/src/tests/ct_migration.rs Outdated Show resolved Hide resolved
integration-tests/src/tests/ct_migration.rs Show resolved Hide resolved
integration-tests/src/tests/ct_migration.rs Show resolved Hide resolved
integration-tests/src/tests/defaults.rs Outdated Show resolved Hide resolved
pallets/funding/src/functions.rs Outdated Show resolved Hide resolved
pallets/funding/src/functions.rs Outdated Show resolved Hide resolved
pallets/funding/src/functions.rs Outdated Show resolved Hide resolved
pallets/funding/src/functions.rs Outdated Show resolved Hide resolved
pallets/funding/src/functions.rs Show resolved Hide resolved
Copy link
Collaborator

@vstam1 vstam1 left a comment

Choose a reason for hiding this comment

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

lgtm with the changes requested by Leonardo. I do not really like the tight coupling on pallet-xcm, but I do not really see another way without implementing the same logic as pallet-xcm with adding a callback on query response.

@lrazovic lrazovic self-requested a review November 23, 2023 10:12
@JuaniRios JuaniRios merged commit 77f82b1 into main Nov 23, 2023
@JuaniRios JuaniRios deleted the feature/plmc-290-xcm-instruction-that-reveals-polimecs-balance-and-pallet branch November 23, 2023 11:13
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.

3 participants