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

Fixed claimable GAS calculation #607

Merged
merged 1 commit into from
Jan 28, 2018
Merged

Conversation

mhuggins
Copy link
Contributor

@mhuggins mhuggins commented Jan 26, 2018

What current issue(s) from Trello/Github does this address?
#522 (among others)

What problem does this PR solve?
Calculating claimable GAS can be off due to bad floating point math.

How did you solve this problem?
I replaced floating points with bignumber.js math.

How did you make sure your solution works?
Although this calculation is used for display only, I verified that claiming still works as expected.

I observed the redux store to ensure the calculation is correct. I also claimed to ensure it works as expected. (See screenshot below.) The transaction in the blockchain confirms the correct amount was claimed: http://testnet.neoverse.io/transactions/1a7874a8158eaa84d4cb1180d4196e6d0c5ef450525358e0daa65bf745478f1b

screen shot 2018-01-25 at 11 26 45 pm

Are there any special changes in the code that we should be aware of?
N/A

Is there anything else we should know?
N/A

  • Unit tests written?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 44.345% when pulling 83ed92a on neoverse:claim-gas-bug into 6276a54 on CityOfZion:dev.

@mhuggins mhuggins changed the title Fixed GAS claim amount Fixed claimable GAS calculation Jan 26, 2018
@evgenyboxer evgenyboxer merged commit 737f3cd into CityOfZion:dev Jan 28, 2018
dvdschwrtz-zz pushed a commit that referenced this pull request Feb 3, 2018
dvdschwrtz-zz pushed a commit that referenced this pull request Feb 14, 2018
mhuggins added a commit to neoverse/neon-wallet that referenced this pull request Feb 22, 2018
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