Page MenuHomePhabricator

[Cashtab] Improve bignumber parsing for token agora
ClosedPublic

Authored by bytesofman on Wed, Oct 30, 11:51.

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABC6f30fa0e468a: [Cashtab] Improve bignumber parsing for token agora
Summary

Handle big token quantities in agora offers

We have updated all of the math to use bigint in the OrderBook component. However, I did not realize we are still limited by the html slider field. This field only accepts string values, which are then automatically converted to Number for math operations. Since values there are outside of Number.MAX_SAFE_INTEGER, we are getting bad calculations for takeTokenSatoshis (out of line with calculated tokenSatoshisStep). agora throws errors trying to calculate price with invalid values.

I went down a rabbit hole improving the capability of string-based big number helpers in Cashtab. This was (somewhat) fruitless, but the functions are "better", so why not. We do need toBigInt as JS will not convert stringified scientific notation to bigint.

The fix is in preparing all values that come in from the slider. Going forward, we may find it important to build our own slider and have it accept bigint.

Test Plan

npm test, visit https://cashtab-local-dev.netlify.app/, make sure version is 2.49.0, nav to agora screen, ctrl+f "rocash", use the slider, we do not see white screen of death app crash

image.png (400×473 px, 57 KB)

Diff Detail

Repository
rABC Bitcoin ABC
Branch
agora-patch-number
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 30921
Build 61345: Build Diffcashtab-tests
Build 61344: arc lint + arc unit

Event Timeline

bytesofman edited the test plan for this revision. (Show Details)
bytesofman edited the test plan for this revision. (Show Details)
emack requested changes to this revision.Wed, Oct 30, 12:21
emack added a subscriber: emack.

image.png (340×668 px, 64 KB)

Is this because the quantity exceeds circulating supply? Can't you end the slider range at 745220833808901734.4?

And perhaps for another diff, add a Max button to buy the full offer?

cashtab/src/wallet/index.js
310 ↗(On Diff #50551)

this should never happen via the UI right?

This revision now requires changes to proceed.Wed, Oct 30, 12:21

image.png (340×668 px, 64 KB)

Is this because the quantity exceeds circulating supply? Can't you end the slider range at 745220833808901734.4?

Not related to circulating supply. The issue here (unrelated to this diff)

If you "accept" a partial offer in such a way where the remainder is less than the contractual min buy of the offer -- the remaining offer is unacceptable. The user has to cancel it, or it just exists forever as unacceptable.

There is validation in ecash-agora to prevent this type of accept. But it's better to prevent the user from trying to accept.

So, we don't want to end the slider range -- because the user can accept the full offer.

And perhaps for another diff, add a Max button to buy the full offer?

Maybe. In practice I haven't had trouble pushing the slider all the way over.

cashtab/src/wallet/index.js
310 ↗(On Diff #50551)

right. also I didn't add a test case.

backing this out. can handle if we need later.

bytesofman marked an inline comment as done.

back out unused and untested negative number logic

This revision is now accepted and ready to land.Thu, Oct 31, 02:58