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

fix: bifrost type issue & small fixes in tests #1345

Merged
merged 1 commit into from
Oct 27, 2023
Merged

Conversation

Imod7
Copy link
Contributor

@Imod7 Imod7 commented Oct 27, 2023

Description

This PR includes 3 separate fixes :

  1. Add check in sanitize numbers for Bifrost object type
  2. Add entry isEthereum in test results
  3. Small fix in jest config

1. Bifrost Object Type

Relates to issue #1082

Issue

When connected to wss://bifrost-rpc.liebi.com/ws and requesting one of the following blocks

Sidecar returns the message :

"Unexpected non-string and non-number key while sanitizing a Map-like type"

This is because the delegations entry have as key a Multilocation object and in Sidecar code when we sanitize here we expect a key only of type string or number.

Suggested Solution

Solution suggested from @hqwangningbo in this comment which checks if the key is of type object and makes it a string. With this change, when requesting one of the affected blocks it returns (e.g. for block 5296335) the object :

...
    "delegations": {
                              "{\"parents\":\"1\",\"interior\":{\"x2\":[{\"parachain\":\"2023\"},{\"accountKey20\":{\"network\":null,\"key\":\"0x5d9bc481749420cffb2bf5aef5c5e2a0ffe04e88\"}}]}}": "1148554603043768874193",
                              "{\"parents\":\"1\",\"interior\":{\"x2\":[{\"parachain\":\"2023\"},{\"accountKey20\":{\"network\":null,\"key\":\"0xd8b890aaaf926adc863f04c9223164afa5ea3388\"}}]}}": "3944544267848663635461",
                              "{\"parents\":\"1\",\"interior\":{\"x2\":[{\"parachain\":\"2023\"},{\"accountKey20\":{\"network\":null,\"key\":\"0xf971c8f8d2437234733814b40e7dfef75e219545\"}}]}}": "1445924498529099301246"
...

This result aligns also with what we see in :

  • pjs apps at the same block when connected to Bifrost
delegations: BTreeMap<XcmV3MultiLocation, u128>

{
  {parents:1,interior:{x2:[{parachain:2023},{accountKey20:{network:null,key:0x5d9bc481749420cffb2bf5aef5c5e2a0ffe04e88}}]}}: 1,148,554,603,043,768,874,193
  {parents:1,interior:{x2:[{parachain:2023},{accountKey20:{network:null,key:0xd8b890aaaf926adc863f04c9223164afa5ea3388}}]}}: 3,944,544,267,848,663,635,461
  {parents:1,interior:{x2:[{parachain:2023},{accountKey20:{network:null,key:0xf971c8f8d2437234733814b40e7dfef75e219545}}]}}: 1,445,924,498,529,099,301,246
}
  • subscan here > entry delegations

2. Fix in tests by adding isEthereum entry

Added the missing entry isEthereum in the corresponding json files.

3. Fix in jest config

Issue

When running

yarn test:historical-e2e-tests

we are receiving the warning :

ts-jest[ts-jest-transformer] (WARN) Define `ts-jest` config under `globals` is deprecated. Please do
transform: {
    <transform_regex>: ['ts-jest', { /* ts-jest config goes here in Jest */ }],
},

Suggested Solution

Removed entry in globals source and added it in transform.

Todos

  • Run tests, historical tests and latest tests
  • Checked/Tested manually that the endpoints for the affected blocks in Bifrost are working now
  • Run linter
  • Checked docs. No need to update for the isEthereum entry

- Add check in sanitize numbers for Bifrost object type
- Add `isEthereum` entry in test results
- Small fix in jest config
@Imod7 Imod7 requested a review from a team as a code owner October 27, 2023 08:27
Copy link
Contributor

@bee344 bee344 left a comment

Choose a reason for hiding this comment

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

Amazing! Thanks for taking this up

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!

@Imod7 Imod7 merged commit 567170b into master Oct 27, 2023
14 checks passed
@Imod7 Imod7 deleted the domi-bifrost-type branch October 27, 2023 13:17
This pull request was closed.
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.

3 participants