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 in-transit XCM msgs in blocks endpoint #1412

Merged
merged 7 commits into from Mar 21, 2024
Merged

Conversation

Imod7
Copy link
Contributor

@Imod7 Imod7 commented Mar 12, 2024

Description

This pull request introduces the in transit horizontal messages within the Relay chain.

Closes issue #732 since this is the remaining part. First part was completed in the #1364 PR.

Sample Responses

Request
http://127.0.0.1:8080/blocks/17792973?decodedXcmMsgs=true

while connected to Kusama Relay Chain

Response
The item decodedXcmMsgs now shows also the in transit Horizontal Messages as shown below :

"decodedXcmMsgs": {
    "horizontalMessages": [
      {
        "originParaId": "2084",
        "destinationParaId": "2000",
        "data": [
          {
            "v2": [
              {
                "withdrawAsset": {
          ....
           },
      {
        "originParaId": "2110",
        "destinationParaId": "2114",
        "data": [
          {
            "v2": [
              {
                "withdrawAsset": [
          ....
             ]
      }
    ],
    "downwardMessages": [],
    "upwardMessages": []

This block has 2 in transit Horizontal Messages with different origin and different destination paraId.

Request
http://127.0.0.1:8080/blocks/21943991?decodedXcmMsgs=true

while connected to Kusama Relay Chain

Response
The item decodedXcmMsgs shows 2 in transit Horizontal Messages with same origin and same destination paraId:

"decodedXcmMsgs": {
    "horizontalMessages": [
      {
        "originParaId": "2001",
        "destinationParaId": "2023",
        "data": [
          {
            "v3": [
              {
                "withdrawAsset": [
                  {
                   ....
          },
          {
            "v3": [
              {
                "withdrawAsset": [
                  {
                    ....
      }
    ],
    "downwardMessages": [],
    "upwardMessages": []
  }

*** Same response structure when connected to a Parachain, e.g. block 5646401 from Polkadot Asset Hub

Request
http://127.0.0.1:8080/blocks/22238422?decodedXcmMsgs=true

while connected to Kusama Relay Chain

Response
The item decodedXcmMsgs shows 3 Upward Messages from same origin paraId :

 "decodedXcmMsgs": {
    "horizontalMessages": [],
    "downwardMessages": [],
    "upwardMessages": [
      {
        "originParaId": "2001",
        "data": [
          {
            "v3": [
              {
                "withdrawAsset": [
                 ....
          {
            "v3": [
              {
                "withdrawAsset": [
                  {
                   ....
          },
          {
            "v3": [
              {
                "withdrawAsset": [
                  {
                  ...
    ]
  }

Tested Cases with Example Blocks

Cases that were tested (while connected to a Kusama/Polkadot Relay chain):

  1. Blocks with 1 Horizontal Message.
  2. Blocks with 1 Horizontal and 1 Upward Message.
  3. Blocks with 2 Horizontal Messages with same origin paraId and different destination paraId.
  4. Blocks with 2 Horizontal Messages with different origin and different destination paraId.
  5. Blocks with multiple Upward Messages with same origin paraId .

*** Above cases were also tested with the query param paraId

All examples tested can be found here

Changelog (with Fixes - Breaking Changes)

  • ❗In Upward Messages of Relay, data is now an array since multiple XMC messages from same paraId can be retrieved.
  • ❗The structure/shape (and consequently the associated interface) of Horizontal Messages in the Relay Chain differs from that of Horizontal Messages in Parachain. As a result:
    • IHorizontalMessageInRelayChain interface was introduced.
    • IHorizontalMessage was renamed to IHorizontalMessageInParachain to provide clearer distinction.
  • ❗The field paraId was renamed to originParaId in the following interfaces / responses:
    • IHorizontalMessageInParachain
    • IUpwardMessage
      to explicitly indicate that it refers to the origin paraId rather than the destination. It also ensures consistency with the IHorizontalMessageInRelayChain interface.
  • The code has been refactored:
    • when checking for Upward and Horizontal Messages in Relay and
    • when checking for Horizontal Msgs in Parachain
      to avoid repetitive blocks of code
  • Corrected ISanitizedParachainInherentData so that it reflects how the information is retrieved.

Todos

  • Update Documentation

@Imod7 Imod7 requested a review from a team as a code owner March 12, 2024 08:26
Copy link
Collaborator

@marshacb marshacb left a comment

Choose a reason for hiding this comment

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

Solid PR. LGTM!

Copy link
Contributor

@IkerAlus IkerAlus left a comment

Choose a reason for hiding this comment

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

As discussed in the call, the output should something like:

When connected in Relay and I have 5 Horizontal Msgs like

  • 4 from para 2010, 2 to para 2034, 1 to para 2031, 1 to para 2032.
  • 1 from para 2020 to para 2032,

they should be shown as:

"decodedXcmMsgs": {
    "horizontalMessages": [
      {
        "sentAt": "blockNumber",
        "originId":"2010",
        "destinationId": "2034",
        "data":           {
            "v3": [
              {
                "withdrawAsset": [
                  {...
            ]
          },
          {
            "v3": [
              {
                "withdrawAsset": [
                  {
                 ...
           ]
        },
        {
        "sentAt": "blockNumber",
        "originId":"2010",
        "destinationId": "2031",
        "data": [
          {
            "v3": [
              {
                "withdrawAsset": [
                  {
                ...
        },
        {
        "sentAt": "blockNumber",
        "originId":"2010",
        "destinationId": "2032",
        "data": [
          {
            "v3": [
              {
                "withdrawAsset": [
                  {
                ...
        },
        {
        "sentAt": "blockNumber",
        "originId":"2020",
        "destinationId": "2032",
        "data": [
          {
            "v3": [
              {
                "withdrawAsset": [
                  {
                ...
          }
        ]
      }

Perhaps there are better choices for the naming of originId and destinationId but the structure should be like the one above.

xcmMessages.upwardMessages?.push(msg_decoded);
}
data.backedCandidates.forEach((candidate) => {
if (paraId === undefined || candidate.candidate.descriptor.paraId.toString() === paraId.toString()) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (paraId === undefined || candidate.candidate.descriptor.paraId.toString() === paraId.toString()) {
if (!paraId || candidate.candidate.descriptor.paraId.toString() === paraId.toString()) {

xcmMessages.horizontalMessages?.push(horizontalMessage);
} else if (paraId === undefined) {
horizontalMessage = {
if (paraId === undefined || index.toString() === paraId.toString()) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (paraId === undefined || index.toString() === paraId.toString()) {
if (!paraId || index.toString() === paraId.toString()) {

: (msg as ISanitizedBackedCandidateHorizontalMessage).data.slice(1);
const xcmMessageDecoded: string = this.decodeMsg(api, msgData);

if (paraId === undefined || paraIdCandidate === paraId.toString()) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (paraId === undefined || paraIdCandidate === paraId.toString()) {
if (!paraId || paraIdCandidate === paraId.toString()) {

Copy link
Member

@TarikGul TarikGul left a comment

Choose a reason for hiding this comment

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

LGTM small nits

- added `IHorizontalMessageInRelayChain` interface
- renamed `IHorizontalMessage` to `IHorizontalMessageInParachain`.
- renamed `paraId` to `originParaId` in `IHorizontalMessageInParachain` and `IUpwardMessage` interface.
- corrected `ISanitizedParachainInherentData`
- updated tests
Copy link
Contributor

@IkerAlus IkerAlus left a comment

Choose a reason for hiding this comment

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

LGTM, good job!

I see there is no "sentAt" for horizontal messages in the Relay chain but I think that in this context it is not so relevant, I am ok with leaving as it is now.

Also, consider the doc update so it is clear to the user what we are showing in each of the cases.

@Imod7
Copy link
Contributor Author

Imod7 commented Mar 18, 2024

LGTM, good job!

I see there is no "sentAt" for horizontal messages in the Relay chain but I think that in this context it is not so relevant, I am ok with leaving as it is now.

Also, consider the doc update so it is clear to the user what we are showing in each of the cases.

Yes, there is no "sentAt" field available so that is why I didn't put it. Yes, definitely I will update the docs to make everything very clear and also the description of the PR with the latest changes. Thank you!

…originPara)

- upward msgs response to be aligned with horizontal msgs response structure
- added Tarik's suggestions
docs/src/openapi-v1.yaml Outdated Show resolved Hide resolved
Imod7 and others added 2 commits March 21, 2024 10:53
Co-authored-by: IkerAlus <34474035+IkerAlus@users.noreply.github.com>
@Imod7 Imod7 merged commit 6028086 into master Mar 21, 2024
15 checks passed
@Imod7 Imod7 deleted the domi-xcm-transit branch March 21, 2024 10:08
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.

None yet

4 participants