Skip to content

Update xcm-format with v3 Instructions and Registers#31

Merged
KiChjang merged 24 commits intopolkadot-fellows:masterfrom
CrackTheCode016:master
Apr 12, 2023
Merged

Update xcm-format with v3 Instructions and Registers#31
KiChjang merged 24 commits intopolkadot-fellows:masterfrom
CrackTheCode016:master

Conversation

@CrackTheCode016
Copy link
Copy Markdown
Contributor

@CrackTheCode016 CrackTheCode016 commented Mar 9, 2023

In an effort to sync, update, and expand the documentation around XCM in the Polkadot Wiki, I propose adding the newly added instructions and registers.

Most of the definitions for the instructions came from the XCM v3 commit code in the Polkadot repo.

Instructions Added:

  • ExpectAsset
  • ExpectError
  • ExpectOrigin
  • QueryPallet
  • ExpectPallet
  • ReportTransactStatus
  • ClearTransactStatus
  • LockAsset
  • UnlockAsset
  • NoteUnlockable
  • RequestUnlock

Registers Added:

  • Transact Status
  • Topic

TODO

  • MultiLocation
  • Junction
    • GeneralKey
    • network fields
  • MultiAsset/WildMultiAsset
  • max_assets removal
  • NetworkId
  • BodyId/BodyPart
  • Concept of UniversalLocation (for the GlobalConsensus Junction)
  • Errors
  • Go through XCMv3 PR and ensure that all changes are reflected in the spec.

CrackTheCode016 and others added 2 commits March 9, 2023 18:15
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
@CrackTheCode016
Copy link
Copy Markdown
Contributor Author

CrackTheCode016 commented Mar 9, 2023

Thank you for catching that!! Let me know if there is anything else you think would be wise to include/changed.

CrackTheCode016 and others added 4 commits March 10, 2023 19:53
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
@KiChjang
Copy link
Copy Markdown
Contributor

For reference, the XCM v3 PR is here: paritytech/polkadot#4097, make sure that every new feature mentioned in the description there is reflected on this PR.

Copy link
Copy Markdown
Contributor

@franciscoaguirre franciscoaguirre left a comment

Choose a reason for hiding this comment

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

Took a dive on the XCM v3 PR and diffed it here to add what was missing

Thank you for keeping the standard up-to-date with the code 🎉

@gavofyork
Copy link
Copy Markdown
Contributor

gavofyork commented Mar 15, 2023

Junction also changed more than this diff would imply: GeneralKey and all items which contained a network field also changed.

@vstam1
Copy link
Copy Markdown
Contributor

vstam1 commented Mar 16, 2023

@gavofyork Currently we describe most of the errors of the xcvm instructions as fallible or infallible. Do we want to change these error descriptions to the actual errors mentioned in the 8 The types of error in XCM section based on the implementation in the xcm-executor?

@CrackTheCode016
Copy link
Copy Markdown
Contributor Author

CrackTheCode016 commented Mar 16, 2023

Maybe not relevant for this PR, but would including hyperlinks to the xcm code in the Polkadot repo be interesting? I feel it would bring the spec more cohesion.

i.e., to specific instructions, or interfaces like XcmContext etc

@vstam1 vstam1 requested a review from franciscoaguirre March 20, 2023 14:57
@gavofyork
Copy link
Copy Markdown
Contributor

@gavofyork Currently we describe most of the errors of the xcvm instructions as fallible or infallible. Do we want to change these error descriptions to the actual errors mentioned in the 8 The types of error in XCM section based on the implementation in the xcm-executor?

Yes - that could be helpful.

@gavofyork
Copy link
Copy Markdown
Contributor

Maybe not relevant for this PR, but would including hyperlinks to the xcm code in the Polkadot repo be interesting? I feel it would bring the spec more cohesion.

i.e., to specific instructions, or interfaces like XcmContext etc

Sounds sensible.

Copy link
Copy Markdown
Contributor

@franciscoaguirre franciscoaguirre left a comment

Choose a reason for hiding this comment

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

Left some comments on error types

README.md Outdated

Sets the Fees Mode Register.

- `jit_withdraw: bool`: The fees mode item; if set to `true` then fees for any instructions are withdrawn as needed using the same mechanism as `WithdrawAssets`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not a good explanation, because naturally people will ask why we don't simply just use WithdrawAsset to do the job. And I don't believe it is true that any fees are eligible to be withdrawn -- only a select few instructions do this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This better?

@vstam1 vstam1 requested a review from KiChjang April 6, 2023 14:59
Copy link
Copy Markdown
Contributor

@franciscoaguirre franciscoaguirre left a comment

Choose a reason for hiding this comment

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

This looks like it's good to go 🚀

Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
@KiChjang KiChjang merged commit 6193fa1 into polkadot-fellows:master Apr 12, 2023
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.

5 participants