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.

Sign up for GitHub

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

Jump to bottom

fix: toBignumber conversion error with high balance #12010

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: toBignumber conversion error with high balance #12010

wants to merge 3 commits into from
+6 −2

Conversation

siibars
Copy link
Contributor

@siibars siibars commented Oct 24, 2024 •
edited
Loading

Description

There is a bignumber conversion attempt which is failing, this change fixes this.

Related issues

Fixes: STAKE-848

Manual testing steps

  1. connect with an account with a high ETH balance
  2. enable the native staking feature flag
  3. click stake to get to the StakeinputView

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@siibars siibars requested a review from a team as a code owner October 24, 2024 17:12
@github-actions GitHub Actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

Matt561
Matt561 previously approved these changes Oct 24, 2024
View reviewed changes
@siibars siibars added team-stake Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 24, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 24, 2024 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: fc7623c
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/40b358b8-44bc-4313-bf09-f955ae877fa7

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@legobeat
Copy link
Contributor

legobeat commented Oct 24, 2024 •
edited
Loading

Do you have a link (or would you be able to describe in PR description) the issue this fixes?

Considering if this is related at all to solving it:

Also, is there any possible way to add a regression test for this bugfix?

legobeat
legobeat reviewed Oct 24, 2024
View reviewed changes
app/components/UI/Stake/hooks/useBalance.ts
@@ -50,7 +51,10 @@ const useBalance = () => {
);

const { pooledStakesData } = usePooledStakes();
const assets = pooledStakesData.assets ?? 0;
const assets = useMemo(
() => new BN(pooledStakesData.assets),
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better if we can use hexToBN from @metamask/controller-utils if possible

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Oct 24, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
80.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@Matt561 Matt561 self-requested a review October 24, 2024 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@legobeat legobeat legobeat left review comments

@Matt561 Matt561 Awaiting requested review from Matt561

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Run Smoke E2E Triggers smoke e2e on Bitrise team-stake
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@siibars @legobeat @Matt561