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

feat: add async XCM contract call scenario #941

Merged
merged 21 commits into from
Jul 6, 2023

Conversation

ashutoshvarma
Copy link
Member

@ashutoshvarma ashutoshvarma commented May 20, 2023

Pull Request Summary
Add Async XCM contract call experimental scenario to xcm-simulator tests.

NOTE
Based on #939

This PR adds,

  • ink! contract PoC for async call to be used as fixture in tests
  • xcm simulator scenarios performing async xcm calls from above contract.

Async XCM Contract Call

  • Contract on ParaA will send below XCM to ParaB using call_runtime to perform some operation (like Transact).
  • If Error, a Transact instr will be used to send an XCM back to ParaA which will call ERROR contract method.
  • If Success, same as above but call SUCCESS method.
Xcm(vec![
	WithdrawAsset(..)
	BuyExecution(..)
	SetAppendix(Xcm(vec![
		Transcat { /* pallet_xcm::send() runtime call to call SUCCESS contract method in ParaA */ }
	]))
	SetErrorHandler(Xcm(vec![
		Transcat { /* pallet_xcm::send() runtime call to call ERROR contract method in ParaA */ }
	]))
	...
])

Check list

  • added or updated unit tests
  • updated Astar official documentation
  • added OnRuntimeUpgrade hook for precompile revert code registration
  • updated spec version
  • updated semver

@ashutoshvarma ashutoshvarma added the tests If the PR/issue is related to tests, like xcm-simulator tests, rpc-tests etc. label May 20, 2023
@ashutoshvarma ashutoshvarma marked this pull request as ready for review May 21, 2023 07:44
Copy link
Member

@shaunxw shaunxw left a comment

Choose a reason for hiding this comment

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

Some nitpicks. Not a pallet-contract expert so I basically skipped the contract impl part.

tests/xcm-simulator/contracts/async-xcm-call-no-ce/lib.rs Outdated Show resolved Hide resolved
tests/xcm-simulator/src/tests/experimental.rs Show resolved Hide resolved
tests/xcm-simulator/src/tests/experimental.rs Outdated Show resolved Hide resolved
tests/xcm-simulator/contracts/async-xcm-call-no-ce/lib.rs Outdated Show resolved Hide resolved
Copy link
Member

@Dinonard Dinonard left a comment

Choose a reason for hiding this comment

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

This adds the test - which is good!

Question - how will the non-runtime devs, or to be more precise, how will the smart contract devs know about this feature?

tests/xcm-simulator/build.rs Outdated Show resolved Hide resolved
@ashutoshvarma
Copy link
Member Author

ashutoshvarma commented Jun 1, 2023

@Dinonard I'm not sure why smart contract dev needs to know about this feature 🤔 , since this is for Runtime tests fixtures but I understand this needs to be put somewhere so that even runtime devs know about this.
I've added a small section in readme for this
- af03b05.

EDIT: Sorry I miss understood, I thought you meant the build script.
I'm not sure we should advertise this since it is experimental and should not be used in production (not sure if can be used in production due to SafeCallFilter in other chains).
We can add a section in docs for this and with appropriate warning like

  • use upgradeable contracts (in case Call changed)
  • implement your own replay protection in smart contract

@ashutoshvarma ashutoshvarma changed the base branch from feat/improve-simulator-fixtures to master June 12, 2023 03:36
@github-actions
Copy link

github-actions bot commented Jul 4, 2023

Code Coverage

Package Line Rate Branch Rate Health
pallets/contracts-migration/src 0% 0%
precompiles/sr25519/src 79% 0%
chain-extensions/types/assets/src 0% 0%
pallets/block-reward/src 85% 0%
chain-extensions/xvm/src 0% 0%
pallets/pallet-xvm/src/pallet 8% 0%
precompiles/substrate-ecdsa/src 78% 0%
pallets/custom-signatures/src 52% 0%
precompiles/assets-erc20/src 76% 0%
precompiles/utils/src 69% 0%
chain-extensions/types/dapps-staking/src 0% 0%
precompiles/xvm/src 61% 0%
primitives/src 70% 0%
primitives/src/xcm 70% 0%
precompiles/xcm/src 85% 0%
precompiles/dapps-staking/src 93% 0%
pallets/pallet-xcm/src 54% 0%
pallets/collator-selection/src 70% 0%
chain-extensions/dapps-staking/src 0% 0%
chain-extensions/types/xvm/src 0% 0%
precompiles/utils/macro/src 0% 0%
pallets/xc-asset-config/src 54% 0%
chain-extensions/pallet-assets/src 0% 0%
pallets/pallet-xvm/src 5% 0%
pallets/dapps-staking/src 82% 0%
pallets/dapps-staking/src/pallet 87% 0%
Summary 55% (2196 / 3996) 0% (0 / 0)

Minimum allowed line rate is 50%

Copy link
Member

@shaunxw shaunxw left a comment

Choose a reason for hiding this comment

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

LGTM

@ashutoshvarma ashutoshvarma merged commit 14d73c5 into master Jul 6, 2023
12 checks passed
@ashutoshvarma ashutoshvarma deleted the feat/async-contract-call-xcm-scenario branch July 6, 2023 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests If the PR/issue is related to tests, like xcm-simulator tests, rpc-tests etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants