Page MenuHomePhabricator
Differential D17455

[Cashtab] Do not show unacceptable offers to buyers, highlight them uniquely to sellers
ClosedPublic
Actions

Authored by bytesofman on Thu, Jan 2, 14:24.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC5f27e3d2661a: [Cashtab] Do not show unacceptable offers to buyers, highlight them uniquely to…
Summary

It is possible for agora offers to be "unacceptable", i.e. the terms of the offer are such that the min buy is smaller than the tokens available to buy.

Cashtab has a bug where validation is not correctly catching this, allowing such offers to be created. But even if (er, when) we catch it in Cashtab, anyone could make such an offer, so we need logic to handle them.

Test Plan

npm test, see screenshot. will patch the UI validation that is making these possible in a subsequent diff. For now this bug is helpful for testing this.

image.png (458×1 px, 55 KB)

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bytesofman created this revision.Thu, Jan 2, 14:24
Herald added a reviewer: Restricted Project. · View Herald TranscriptThu, Jan 2, 14:24
bytesofman edited the test plan for this revision. (Show Details)Thu, Jan 2, 14:25
Harbormaster completed remote builds in B31885: Diff 51915.Thu, Jan 2, 14:34
bytesofman updated this revision to Diff 51916.Thu, Jan 2, 14:45

confirm behavior with test

Harbormaster completed remote builds in B31886: Diff 51916.Thu, Jan 2, 14:50
bytesofman updated this revision to Diff 51917.Thu, Jan 2, 14:55

Modify behavior so that selectedOffer cannot be an unrendered offer

bytesofman published this revision for review.Thu, Jan 2, 14:57
Harbormaster completed remote builds in B31887: Diff 51917.Thu, Jan 2, 15:00
Fabien requested changes to this revision.Thu, Jan 2, 15:10
Fabien added a subscriber: Fabien.
Fabien added inline comments.
cashtab/src/components/Agora/OrderBook/index.tsx
630 ↗(On Diff #51917)

You should not account for the non rendered quantities, otherwise you can end up with an order book with depth not reaching 100% which is weird (maybe check this in your unit test ?)

649 ↗(On Diff #51917)

See above, you should continue at the very beginning of the loop before any processing

This revision now requires changes to proceed.Thu, Jan 2, 15:10
bytesofman marked 2 inline comments as done.Thu, Jan 2, 15:30
bytesofman added inline comments.
cashtab/src/components/Agora/OrderBook/index.tsx
630 ↗(On Diff #51917)

in this case ... we probably "should" test it. It is (probably) possible to get the impact of this on the visual component by confirming expected width of the depth bars.

However

  • pretty complex mocks required just for this test
  • this value (now) is only used for rendering, which is imo easier to confirm by just glancing at any orderbook

once we start displaying this offer in a tooltip, then we will be testing for the actual value (not its derived impact on a css property), and this would be more straighforward to do and also provide a more valuable behavior confirmation. Since we are planning to do this imminently, I think it makes sense to wait.

bytesofman updated this revision to Diff 51924.Thu, Jan 2, 15:30
bytesofman marked an inline comment as done.

continue before any analytic logic

bytesofman edited the summary of this revision. (Show Details)Thu, Jan 2, 15:30
bytesofman removed a parent revision: D17453: [Cashtab] Highlight your own offers in agora orderbook.
Harbormaster completed remote builds in B31890: Diff 51924.Thu, Jan 2, 15:41
Fabien added a comment.Thu, Jan 2, 15:42

It is (probably) possible to get the impact of this on the visual component by confirming expected width of the depth bars.

Why would you do that ? Just test that the value is not accounted in the depth computation. Anyway it's really a nit

Fabien accepted this revision.Thu, Jan 2, 15:44
This revision is now accepted and ready to land.Thu, Jan 2, 15:44
Closed by commit rABC5f27e3d2661a: [Cashtab] Do not show unacceptable offers to buyers, highlight them uniquely to… (authored by bytesofman). · Explain WhyThu, Jan 2, 16:18
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABC5f27e3d2661a: [Cashtab] Do not show unacceptable offers to buyers, highlight them uniquely to….
bytesofman mentioned this in D17426: [Cashtab] Do not show wild-ass agora offers in OrderBook.Thu, Jan 2, 17:20

Revision Contents
Changeset List

PathSize
cashtab/
4 lines
2 lines
src/
assets/
styles/
2 lines
components/
Agora/
OrderBook/
__tests__/
82 lines
46 lines
11 lines
fixtures/
51 lines

Diff 51925

View Options

cashtab/package-lock.json

Loading...
View Options

cashtab/package.json

Loading...
View Options

cashtab/src/assets/styles/theme.ts

Loading...
View Options

cashtab/src/components/Agora/OrderBook/__tests__/index.test.js

Loading...
View Options

cashtab/src/components/Agora/OrderBook/index.tsx

Loading...
View Options

cashtab/src/components/Agora/OrderBook/styled.ts

Loading...
View Options

cashtab/src/components/Agora/fixtures/mocks.js

Loading...