-
Notifications
You must be signed in to change notification settings - Fork 51
XCMv5 guide #477
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
base: master
Are you sure you want to change the base?
XCMv5 guide #477
Conversation
Assets become trapped whenever execution halts and there are leftover assets. | ||
This can happen for example if: | ||
|
||
- An XCM execution throws an error in any instruction when assets are in holding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nested lists are not working like this. How can I do them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@franciscoaguirre, I think you need four spaces instead of 2. For example:
- An XCM execution throws an error in any instruction when assets are in holding
- `DepositAsset` can't deposit because the account doesn't exist
This should render as nested list
- `DepositAsset` can't deposit because the account doesn't exist | ||
- `Transact` can't execute the call because it doesn't exist | ||
- `PayFees` not enough funds or not paying enough for execution | ||
- or others... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- or others... | |
- and others... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed some files and stopped the review here to do not bombard you with too many comments, pls feel free to tag me for a re-review at any time
Assets become trapped whenever execution halts and there are leftover assets. | ||
This can happen for example if: | ||
|
||
- An XCM execution throws an error in any instruction when assets are in holding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@franciscoaguirre, I think you need four spaces instead of 2. For example:
- An XCM execution throws an error in any instruction when assets are in holding
- `DepositAsset` can't deposit because the account doesn't exist
This should render as nested list
The latest iteration of XCM is version 5. | ||
The main RFCs defining the changes in version 5 are the following: | ||
|
||
- [Legacy #37 - SetAssetClaimer](https://github.com/polkadot-fellows/xcm-format/blob/master/proposals/0037-custom-asset-claimer.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest adding the {target=\_blank}
modifier at the end of each link, so we don't make users leave the website, they only get a new tab opened in the browser. The syntax would be:
- [Legacy #37 - SetAssetClaimer](https://github.com/polkadot-fellows/xcm-format/blob/master/proposals/0037-custom-asset-claimer.md) | |
- [Legacy #37 - SetAssetClaimer](https://github.com/polkadot-fellows/xcm-format/blob/master/proposals/0037-custom-asset-claimer.md){target=\_blank} |
Could you please check other instances?
## Why? | ||
|
||
Version 5 addresses key pain points that developers faced in previous versions: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs either a more detailed explanation or simply not to use a title for such a short sentence. 'Before' and 'after' can be bullet points, and the internal content can also be nested bullet points.
|
||
- Cross-chain transfers were limited to only one transfer type, requiring multiple different XCMs to send the entirety of the tokens | ||
- Fee management across multiple hops was extremely complicated to get right, particularly because of delivery fees | ||
- Cross-chain transfers cleared the origin, making it impossible to `Transact` in the same XCM as a transfer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When possible, it'd be cool to link this references to the rust docs, so for example here - https://paritytech.github.io/polkadot-sdk/master/staging_xcm/v5/struct.XcmBuilder.html#method.transact
Note: Not sure if that's the best link we can find in the rust docs, but you get the point. Could you please check throughout the pages and add references when possible?
This section aims to give you all the changes in this new version you need to make the most of the latest | ||
and greatest in cross-chain interactions. | ||
|
||
We'll be using [Polkadot-API (PAPI)](/develop/toolkit/api-libraries/papi) throughout these docs to write XCMs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, you should add {target=\_blank}
at the end
There is another key difference between `PayFees` and `BuyExecution`. | ||
With `BuyExecution`, if too much was supplied for fees, the leftover after paying for execution would be returned | ||
to the holding register to be used in the rest of the XCM. | ||
With `PayFees`, the full amount put into `assets` is stored in the fees register, nothing is returned to the holding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reference for holding register to link to?
## Migration Examples | ||
|
||
### Simple Teleport | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls, add an introductory sentence about the final goal of the snippets you are showing below
``` | ||
|
||
### Example 2: Multi-Asset Transfer and a Transact | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Introductory sentece
## 🚨 Breaking changes to watch out for | ||
|
||
### `fallback_max_weight` in `Transact` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a short explanation of what the fallback_max_weight
instruction is about?
## Getting help | ||
|
||
- Technical questions: https://substrate.stackexchange.com/ | ||
- Bug reports: https://github.com/paritytech/polkadot-sdk/issues | ||
- Real time help: https://discord.gg/polkadot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a "Get Support" section, so I'd just remove this
@@ -0,0 +1,175 @@ | |||
// `ahp` is the name we gave to `npx papi add` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls check the comment about the we
words and apply it to the snippets as well, just to keep consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent content.
Some follow-up content that I think is needed in our Docs:
A full, "holy-grail" teleport example utilizing all the best practices:
- Fee estimation (
TransactionPaymentApi
, XcmRuntimeApi`, etc) AssetClaimer
RefundSurplus
- Dry run with proper checks e.g. account existence / ED
Teleporting a non-sufficient to Asset Hub
- Using
batch
- Fee estimation
- Estimating fees in the non-sufficient token using
quote_price_exact_tokens_for_tokens
- Estimating fees in the non-sufficient token using
- Using
ExchangeAsset
instruction- Swapping for ED and for paying fees
|
||
## The ClaimAsset instruction | ||
|
||
The `ClaimAsset` instruction allows retriving assets trapped on a chain: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The `ClaimAsset` instruction allows retriving assets trapped on a chain: | |
The `ClaimAsset` instruction allows retrieving assets trapped on a chain: |
## Execution | ||
|
||
All XCMs have a weight associated to them. | ||
Each XCM instruction is benchmarked for a particular system (blockchain), which assigns them a weight. | ||
The weight of an XCM is the sum of the weight of all instructions. | ||
It's important to correctly benchmark this with the worst case so that your system is safe from Denial-of-Service attacks. | ||
|
||
This generated weight represents how much time, and space, is needed for executing the XCM. | ||
It directly translates to **execution fees**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are transaction fees and XCM execution fees the same thing?
}), | ||
]) | ||
``` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can sufficient assets be used for PayFees
and remote_fees
?
|
||
The `remote_fees` parameter only takes one asset, `assets` takes a list of many. | ||
Both need to specify which transfer type they're using. | ||
The developer to specify whether or not those assets are transferred via a `Teleport`, a `ReserveWithdraw` or a `ReserveDeposit`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The developer to specify whether or not those assets are transferred via a `Teleport`, a `ReserveWithdraw` or a `ReserveDeposit`. | |
The developer needs to specify whether or not those assets are transferred via a `Teleport`, a `ReserveWithdraw` or a `ReserveDeposit`. |
XcmV5Instruction.PayFees(/* execution + delivery on A */), | ||
XcmV5Instruction.InitiateTransfer({ | ||
// ... | ||
remote_fees: /* execution + delivery on B */, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it is mentioned in transfers.md
but it would be nice to have an explicit note on remote_fees
since this is the "Fees" page and so the reader understands the difference between PayFees
and remote_fees
. Something like:
remote_fees
automatically results in aPayFees
instruction on the destination chain for the underlying transfer instruction e.g.remote_fees: Enum('ReserveDeposit', /* fee asset */),
``` | ||
|
||
### 3. Consider origin preservation | ||
When origin preservation is available, trapped assets are automatically associated with the original origin, making claiming easier without additional hints. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you don't need to explicitly specify an AssetClaimer
instruction?
hints: [Enum('AssetClaimer', claimerLocation)] | ||
}) | ||
``` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A note on calculating the total fee associated with claiming a trapped asset with AssetClaimer
might be good.
I know there was a case in the past where some users had trapped assets. The amount was so small that it was not worth claiming the assets because the fee was equal to or more to the amount of trapped asset per user.
// The message is just assembling all of these parameters we've defined before. | ||
const xcm = XcmVersionedXcm.V5([ | ||
XcmV5Instruction.WithdrawAsset([dotToWithdraw]), | ||
XcmV5Instruction.PayFees({ asset: dotToPayFees }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why we are not using RefundSurplus
here?
Added a section for XCM guides. In there I added a first section called
From apps
andPAPI
which are meant to show how to use XCM from an app using PAPI. I plan to expand those guides in the future withFrom smart contracts
andFrom runtimes
also showing different tools. All of these guides will use XCMv5.Other than that, I created a section on versions where I list differences between versions. I started it with highlighting the differences between v4 and v5.