[Cashtab] Patch bug in manage agora offers
ClosedPublic
Actions

Authored by bytesofman on Sat, Nov 2, 14:36.

Details

Reviewers

emack

Group Reviewers

Restricted Project

Commits

rABC5b9ca8ccad18: [Cashtab] Patch bug in manage agora offers

Summary

When Orderbook was refactored so that each component owned its offer lookups, did not properly manage the distinction in how we fetch offers by pub key (agora call returns all offers, including multiple offers for the same token) and other offers (agora call returns tokenIds only, uniquely).

Correct this by using a Set to track tokenIds of managed offers. Convert to a sorted array before rendering.

Test Plan

npm test

Create two offers with the same token ID
npm start
navigate to Agora page
toggle "manage" switch
no "duplicate key" error, no repeated orderbooks

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bytesofman created this revision.Sat, Nov 2, 14:36

Herald added a reviewer: Restricted Project. · View Herald TranscriptSat, Nov 2, 14:36

bytesofman edited the test plan for this revision. (Show Details)Sat, Nov 2, 14:37

Harbormaster completed remote builds in B30971: Diff 50619.Sat, Nov 2, 14:40

bytesofman published this revision for review.Sat, Nov 2, 14:46

Have verified the merging of orderbooks for the same token offer, though I wasn't getting a duplicate key error in the first place on master (v2.49.1).

This revision is now accepted and ready to land.Sun, Nov 3, 11:18

actually, after you've listed two offers, verified they render as one merged orderbook, then I've noticed:

you need to cancel twice on the same orderbook, is this because it's representative of 2 offers?
the orderbook in manage listings become a "Buy token", which makes sense since you no longer have any offers left, but shouldn't it not be displayed at all?

This revision now requires changes to proceed.Sun, Nov 3, 11:24

actually, after you've listed two offers, verified they render as one merged orderbook, then I've noticed:

you need to cancel twice on the same orderbook, is this because it's representative of 2 offers?

Yes, each bar represents one AgoraPartial. So, you can cancel individual offers. We do not have a feature for "cancel all my offers this orderbook" -- anyway, this would require a tx to cancel each offer.

the orderbook in manage listings become a "Buy token", which makes sense since you no longer have any offers left, but shouldn't it not be displayed at all?

This should be resolved by updates in D17064, which gives the OrderBook the ability to know if it has no offers and render accordingly.

emack accepted this revision.Mon, Nov 4, 00:43

This revision is now accepted and ready to land.Mon, Nov 4, 00:43

Closed by commit rABC5b9ca8ccad18: [Cashtab] Patch bug in manage agora offers (authored by bytesofman). · Explain WhyMon, Nov 4, 17:24

This revision was automatically updated to reflect the committed changes.

bytesofman added a commit: rABC5b9ca8ccad18: [Cashtab] Patch bug in manage agora offers.