-
Notifications
You must be signed in to change notification settings - Fork 158
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 decoded XCM data in blocks
endpoint
#1364
Conversation
- rebuilt docs - removed moonbeam specific code from xcm decoder
- updated docs in blocks controller comments
- in block 18468942 we have 2 upward msgs from two different parachains so with the same mocked data we can add 2 tests - added a test to check the query param paraId is working as expected
- in block 3356195 we have downward and horizontal msgs so with the same mocked data we can test two different directions
- changed structure of the decodedMsgs response so that it always returns three arrays (filled or empty), one for each direction. - removed an if statement as unnecessary in XCMDecoder - updated corresponding tests with the new structure of the response
…onnected to parachain
src/services/test-helpers/mock/mockAssetHubKusamaApiBlock3356195.ts
Outdated
Show resolved
Hide resolved
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.
Overall, really amazing stuff. There are some key changes needed in the syntax for the XCMDecoder class. The docs need to have a the decoded messages in the response section, and there are a few more nits here and there.
Really amazing feature, really excited to get this in.
- the response is a combination of Block and BlockXCM response with the use of allOf - added schema for Liquidity Pool because the swagger was complaining - changed the order of a hash field so that it aligns of it actually appears in the corresponding response
- changed type of paraId after validation - renamed boolean option for decodedXcmMsgs so it is shorter
- changes in imports - replaced class name with the keyword 'this' - replaced static with readonly for class properties - replaced static with private for class methods - changed specname to lowercase
- added the 'type' keyword in more imports
reserved: null, | ||
}, | ||
}, | ||
perClass: Promise.resolve().then(() => { |
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.
This doesn't need to be a promise since its a const
.
perClass: defaultMockApi.consts.system.blockWeights.perClass
Should suffice.
blockWeights: { | ||
baseBlock: new BN(5481991000), | ||
maxBlock: assetHubKusamaRegistryV1000000.createType('u64', 15), | ||
perClass: Promise.resolve().then(() => { |
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 with this, this doesn't need to be a promise. It can just be:
perClass: defaultMockApi.consts.system.blockWeights.perClass
blockWeights: { | ||
baseBlock: new BN(5481991000), | ||
maxBlock: assetHubKusamaRegistryV1000000b.createType('u64', 15), | ||
perClass: Promise.resolve().then(() => { |
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.
This can also just be:
perClass: defaultMockApi.consts.system.blockWeights.perClass
reserved: null, | ||
}, | ||
}, | ||
perClass: Promise.resolve().then(() => { |
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.
This can also just be:
perClass: defaultMockApi.consts.system.blockWeights.perClass
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.
Few small nits, but once those are wrapped up the PR should be good to merge, Great Job!
Co-authored-by: Tarik Gul <[email protected]>
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.
LGTM 🚀
Relates to: #732
Description
This PR adds the following 2 query parameters:
decodedXcmMsgs
: of typeboolean
paraId
: of typenumber
to the
/blocks/{blockNumber}
endpoint.Expected Behaviour
When the query param
decodedXcmMsgs
is set totrue
, the endpoint is expected to return the XCM messages decoded in a specific block as follows :parachain
, then it returns the decoded XCM messages found in :downwardMessages
horizontalMessages
relay
chain, then it returns the decoded XCM messages found in :upwardMessages
When the query param
paraId
is also set to a specific number/parachain ID, it will only check if there are any XCM messages that are related to that parachain ID. If there are additional XCM messages related to other parachains, they will be ignored and not returned in the response.Sample Response per Endpoint
Request
http://127.0.0.1:8080/blocks/4777964?decodedXcmMsgs=true
while connected to Kusama Asset Hub
Response
At the end of the usual response from
/blocks
, the itemdecodedXcmMsgs
is added that looks as follows :The object
decodedXcmMsgs
will always return 3 arrays (horizontalMessages
, downwardMessages,
upwardMessages`), one for each direction and depending on the XCM message it will fill the corresponding array(s). The rest it will leave empty.Request
http://127.0.0.1:8080/blocks/18468942?decodedXcmMsgs=true¶Id=2000
while connected to Polkadot Relay chain, it will return the XCM message related to parachain = 2000. In this specific block, there is also another XCM message (for parachain 2012) that will not appear.
Testing
This script was created to test multilple blocks from different relay chains and parachains and check the corresponding decoded XCM messages that are returned.
Notes
To be able to decode horizontal messages when connected to Moonbeam, you need to make the changes that are shown in this commit. In the current PR these changes were removed since Sidecar should be as generic as possible and not include changes that are parachain specific.
Todos
decodedXcmMsgs
works as expectedparaId
to work also with horizontal messages when connected to a parachain. Right now it filters only in upward messages when connected to a relay.Downward
Upward
Horizontal
paraId
query param works as expected when connected to Relay chain for upward msgs.paraId
query param works as expected when connected to parachain chain for horizontal msgs.upwardMessages
,downwardMessages
andhorizontalMessages
. Based on what is returned from every requested block, each field is filled or left empty.Credits
Many credits to :