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

Local Pluralities Get Free Xcm Execution #125

Merged

Conversation

muharem
Copy link
Contributor

@muharem muharem commented Dec 13, 2023

Local Root and Pluralities Get Free Xcm Execution

After running e2e tests, we discovered that xcm programs sent from local pluralities such as Relay Chain Staking Admin or Collectives Fellowship were failing due to insufficient balance to cover execution/delivery fees. This PR addresses this issue by granting them a free execution.

Additionally, we encountered similar failures when attempting to teleport slashed assets from Collectives to the Relay Chain Treasury. In this PR, we resolve this problem by teleporting those assets on behalf of the Collectives root location, allowing for a free delivery.

let treasury_acc: AccountIdOf<T> = TreasuryAccount::get();

<pallet_balances::Pallet<T>>::resolve_creating(&pallet_acc, amount);
let root_location: MultiLocation = Here.into();
Copy link
Contributor Author

@muharem muharem Dec 13, 2023

Choose a reason for hiding this comment

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

there are few options,

  • (current) resolve funds into root account temporarily and teleport on behalf of the root
  • calculated an expected fee for the execution and delivery and keep that amount within the account to pay the fee
  • keep as it is, and grant free execution to the pallet's account id. where an account id has to be whitelisted as a location X1(AccountId32(some_bytes_of_pallet_account_id))

I think the current is the most optimal

@muharem
Copy link
Contributor Author

muharem commented Dec 13, 2023

@NachoPal is running e2e with this branch, lets wait for results

@NachoPal
Copy link
Contributor

New waived Origins works. However, it is failing with Failed to convert root origin into account id in the on_unbalanced.

@NachoPal
Copy link
Contributor

Working now after the latest changes

@muharem muharem mentioned this pull request Dec 18, 2023
relay/kusama/src/xcm_config.rs Outdated Show resolved Hide resolved
relay/polkadot/src/xcm_config.rs Outdated Show resolved Hide resolved
@bkchr
Copy link
Contributor

bkchr commented Dec 22, 2023

@muharem could you please add some changelog entry?

@muharem
Copy link
Contributor Author

muharem commented Jan 2, 2024

@muharem could you please add some changelog entry?

added info about new location to account id mapping, the waived locations type changes are basically setup for the new fee manager, we have a changelog regarding it in Added section, the changes to ToParentTreasury type does not change it's behaviour

@bkchr
Copy link
Contributor

bkchr commented Jan 5, 2024

/merge

@fellowship-merge-bot fellowship-merge-bot bot merged commit 101882e into polkadot-fellows:main Jan 5, 2024
14 checks passed
@fellowship-merge-bot
Copy link
Contributor

Enabled auto-merge in Pull Request

Available commands
  • /merge: Enables auto-merge for Pull Request
  • /merge cancel: Cancels auto-merge for Pull Request
  • /merge help: Shows this menu

For more information see the documentation

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.

6 participants