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

Shakedex integration - end-to-end #304

Merged
merged 17 commits into from
Mar 21, 2021

Conversation

kurumiimari
Copy link
Contributor

Hello! Opening this PR to start the review process of integrating Shakedex into Bob. This PR contains the UI to allow bids to be placed/finalized through the UI. Still to do is the ability to list a name for auction - I'm getting to this later this weekend/early next week.

Screenshots follow:

place-bid
auctions

@chikeichan
Copy link
Contributor

Wow, this kicks ass @kurumiimari san! Super excited for this integration 😃 - I will QA this one in the next couple of days. Is there any special instruction for testing in testnet? (e.g. how did you get the yeetus's names in Live Auctions, assuming those are testnet names. Seems like I just need to generate presigns in testnet and manually stub getFullfillments.)

For the posting experience, I am also thinking that we should have a button that allow users to just generate and download presigns, vs just posting to shakedex.com. This gives regular users an option to not use shakedex.com and post their auction anywhere they want (also would become a good fallback in case the listing site is down. Wdyt?

Later on we should also let users choose their listing API, but we don't have to worry about it until a second option exist.

THANKS AGAIN 🙇 🙌

@kurumiimari
Copy link
Contributor Author

Hey hey! Responses follow:

Is there any special instruction for testing in testnet?

Yes. You'll need to pull down the API repo (https://github.com/kurumiimari/shakedex-api), and start the server with it pointing to Bob's HSD node. Make sure to specify the HSD_API_KEY and HSD_NETWORK environment variables. Then, I used the shakedex CLI to create a listing and cURL to upload the listing to the local server. The JSON body for the request is {"auctions": <the JSON in the proofs file>}.

You'll see that the hostname for ShakeDex Web is localhost:8080 in Bob; we'll need to remember to change that before going live.

For the posting experience, I am also thinking that we should have a button that allow users to just generate and download presigns, vs just posting to shakedex.com. This gives regular users an option to not use shakedex.com and post their auction anywhere they want (also would become a good fallback in case the listing site is down. Wdyt?

Sounds good, I'll make this happen.

@kurumiimari
Copy link
Contributor Author

Alright, new screenshots of the place bid flow:

Screen Shot 2021-02-21 at 3 12 25 PM
Screen Shot 2021-02-21 at 3 12 21 PM

@kurumiimari kurumiimari changed the title Shakedex integration - place bid/finalize bid Shakedex integration - end-to-end Feb 22, 2021
@chikeichan
Copy link
Contributor

Thanks @kurumiimari chan!

Just reviewed and QA locally on simnet. Here are some changes I would like to make before merging this in.

I can take care of all of below. Can I push direct to your branch (or make a PR, your call) as some of it require ux/design?

  • styling fix for tables (misalign) and exchange modal (content overflows)
  • surface error message when placing a listing
  • on domain page, when transferring to lockscript, Bob should says "Listing Pending" instead of "transfer pending"
  • separate shakedex api calls from bob calls, so that if shakedex is down, Exchange page is still functional
  • reorg the listing/launching flow to make listing and auction status more clear
  • auction status is SOLD as soon as it is launched. Ideally, the actions after presigns are generated should be "Download" or "List on Shakedex"
  • domains in reveals/unregistered state should not be available for listing
  • Unsold domains should still show up in Domain Manager view
  • probably a few more small items once im done with the above

@kurumiimari
Copy link
Contributor Author

Sounds good on all of the above!

auction status is SOLD as soon as it is launched. Ideally, the actions after presigns are generated should be "Download" or "List on Shakedex"

This is definitely a bug - it should say LIVE. Maybe I'm passing in the wrong arguments to the getCoin endpoint, I'll check it out tonight.

@chikeichan
Copy link
Contributor

Another question: say the users start an auction (finalized transfer to lockscript) and then uninstall Bob. What do they need to backup in order to keep access to their names? (say the auction is set at a really high price, no one purchased the name, and they don't have enough HNS to buy it back, so they must transfer it back to themselves)

@kurumiimari
Copy link
Contributor Author

They'll need to back up their Bob database (the one that contains their watchlist, etc.) since that DB contains the private keys used to generate the presigns.

@chikeichan
Copy link
Contributor

Just pushed the following:

  • add error handling for create listing
  • rework UX for create listing
  • styling fix
  • fix listing status logic where auction was shown as SOLD while it should be ACTIVE
  • fix listing status logic where auction was shown as FINALIZE_CONFIRMED while it should be FINALIZE_CONFIRMING

The general idea is to make each step of the listing process more clear, and to make the whole UX less dependent on a hosted API. You can see that instead of Launch Auction, it is now Generate Presigns, followed by the ability to either download the presign file or submit listing to shakedex.

Screen Shot 2021-02-24 at 1 24 44 AM

Screen Shot 2021-02-24 at 1 22 58 AM

@kurumiimari
Copy link
Contributor Author

Looks great! Let me know if you need anything else from me.

@chikeichan
Copy link
Contributor

Pushed up another change -- closer to done!

  • separate namespace for listings and fills by wallet name
  • fix few issues where fulfillment status is calculated incorrectly
  • styling fixes
  • refactor ux to make offline first-class

shakedex-list-simnet

shakedex-fills-simnet

@rithvikvibhu
Copy link
Collaborator

Just referencing it here, to take care of before tagging a release: kurumiimari/shakedex#10 (shakedex hsd's npm dependency)
P.S. really excited this feature :}

@rithvikvibhu
Copy link
Collaborator

Another thing I noticed: the status says READY TO FINALIZE TRANSFER (when the buyer is paying for it, so in the Your Fills table) along with the Finalize button even if it's not possible to finalize yet. Clicking on Finalize does nothing.
Then, generate 9 new blocks (regtest wait), then the finalize button actually creates a new tx, shows up in My Account, etc.
Maybe hide the Finalize button and change the status until it can be used? Even better a countdown to when it can be finalized.

app/utils/encrypt.js Outdated Show resolved Hide resolved
@chikeichan
Copy link
Contributor

chikeichan commented Mar 2, 2021

Hi @kurumiimari !

I am done with the product refactor and have added the following:

  • make offline exchange the defacto experience
  • add encryption to on-disk private key
  • separate exchange data by wallet id
  • move all exchange related data to a new directory called exchange_data
  • add ability to backup and restore listing in Settings/Exchange
  • upgrade to latest shakedex v0.0.6

TODO:

  • seems like you added cancellation to v0.0.6! I did not put it in this PR, but will do so in another. I won't release it in Bob until there is ability to cancel :)

Since the work begins with you, I would like to get your buy-in on this before merging!

@sdtsui @rithvikvibhu @brandondees @pinheadmz PR is ready for review! Feedback welcome :) . You can run this in simnet/regtest to see how the whole flow work.

@kurumiimari
Copy link
Contributor Author

This looks great to me! Thank you so much for your work on this!

@chikeichan chikeichan merged commit d3f0899 into kyokan:master Mar 21, 2021
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.

4 participants