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: bump polkadot-js deps #786

Merged
merged 12 commits into from
Dec 8, 2021
Merged

fix: bump polkadot-js deps #786

merged 12 commits into from
Dec 8, 2021

Conversation

TarikGul
Copy link
Member

@TarikGul TarikGul commented Dec 6, 2021

This introduced a problem when sanitizing leasePeriod via the tests. I found where the issue is happening and wrote a rough solution for it, but I still need to find the root of it in polkadot-js common, and what is causing this issue via BigInt.

There is also a state tracing update too.

@TarikGul TarikGul requested a review from a team as a code owner December 6, 2021 05:31
@TarikGul TarikGul marked this pull request as draft December 6, 2021 05:31
@TarikGul
Copy link
Member Author

TarikGul commented Dec 6, 2021

I am not able to reproduce the issue via polkadot-js for api.consts.slots.leasePeriod, therefore the issue must be in Sidecar.

@niklasad1 niklasad1 changed the title fix: bump polkdat-js deps fix: bump polkadot-js deps Dec 6, 2021
@TarikGul
Copy link
Member Author

TarikGul commented Dec 6, 2021

now.sub(offset).div(leasePeriod) instanceof BN -> false; (To ensure the value returned I updated the following logic)
new BN(now.sub(offset).div(leasePeriod).toNumber()) instanceof BN -> true

This fixes the serializing issue with 6.11.1.

The fix is here. @jsdw

@TarikGul TarikGul marked this pull request as ready for review December 6, 2021 17:43
src/sanitize/sanitizeNumbers.ts Outdated Show resolved Hide resolved
@TarikGul TarikGul merged commit 072ef06 into master Dec 8, 2021
@TarikGul TarikGul deleted the tarik-update-deps branch December 8, 2021 03:59
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.

5 participants