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

[5] Deposit/Withdraw Modal - Add GDEX Support #1276

Closed
wmbutler opened this issue Mar 8, 2018 · 28 comments
Closed

[5] Deposit/Withdraw Modal - Add GDEX Support #1276

wmbutler opened this issue Mar 8, 2018 · 28 comments
Assignees
Labels
[3] Feature Classification indicating the addition of novel functionality to the design
Milestone

Comments

@wmbutler
Copy link
Contributor

wmbutler commented Mar 8, 2018

No description provided.

@wmbutler wmbutler added the [3] Feature Classification indicating the addition of novel functionality to the design label Mar 8, 2018
@wmbutler wmbutler added this to the 180401 milestone Mar 8, 2018
@startailcoon
Copy link
Contributor

I'll take this

@startailcoon startailcoon self-assigned this Mar 11, 2018
@startailcoon startailcoon changed the title [5] Withdraw Modal - Add GDEX [5] Deposit/Withdraw Modal - Add GDEX Support Mar 25, 2018
@startailcoon
Copy link
Contributor

Extended this issue to include Deposit as well.

GDEX will require some work since their API isn't sending the data back as the others are. Their JSON structure will have to be compiled to something that we commonly use for the other backers.

@wmbutler
Copy link
Contributor Author

wmbutler commented Mar 25, 2018

Let's reach out to GDEX and see if they will provide / adopt this conforming API call.

{  
   "coinType":"bts",
   "walletName":"BitShares 2.0",
   "name":"BitShares",
   "symbol":"BTS",
   "walletSymbol":"BTS",
   "walletType":"bitshares2",
   "transactionFee":"0.",
   "precision":"100000.",
   "backingCoinType":null,
   "supportsOutputMemos":true,
   "restricted":false,
   "authorized":null,
   "notAuthorizedReasons":null,
   "gateFee":0,
   "intermediateAccount":"openledger-wallet",
   "coinPriora":0
}

@wmbutler wmbutler removed this from the 180401 milestone Apr 1, 2018
@wmbutler
Copy link
Contributor Author

wmbutler commented Apr 1, 2018

Waiting for GDEX response.

@abitmore
Copy link
Member

abitmore commented Apr 1, 2018

I'll help contact GDEX.

@startailcoon
Copy link
Contributor

As a follow-up on what GDEX is doing today.

Today GDEX handles returned data from their APIs with the following code, that transforms it into an array like the following. This does not comply with how we handle gateway assets otherwise.

{
  "innerAssetId": 1002,
  "innerAssetName": "GDEX.BTC",
  "innerSymbol": "GDEX.BTC",
  "outerAssetId": 1001,
  "outerAssetName": "bitcoin",
  "outerSymbol": "BTC",
  "status": 0,
  "gateFee": 0,
  "needMemo": 0,
  "minTransactionAmount": 1e-8,
  "type": 2,
  "relationPrecision": 8
}

@tianyekuo
Copy link
Contributor

We will look into it. I notice there are 3 APIs: coins, trading-paires, active-wallet. Are they all necessery?
@startailcoon

@wmbutler
Copy link
Contributor Author

wmbutler commented Apr 10, 2018

I think all we are missing is intermediateAccount. It's possible this could be hardcoded to whatever value crypto-bridge uses, or maybe @mindphlux1 would either update the api or confirm the proper intermediateAccount to uses.

@tianyekuo
Copy link
Contributor

intermediateAccount is crypto in anthor API.
Just add intermediateAccount in the asset list(coins) api for show , is that ok?

@startailcoon
Copy link
Contributor

There are a few different ways we do the connections for backed coins. Let me compile a list for you what we need for each.

@startailcoon
Copy link
Contributor

I've looked through the function of GDEX today, and it's quite different approach from what the other gateways uses. Both in fetching assets and linking the backing wallet and asset.

It's not impossible to make this work together with the new modal, together with the other gateways.

  • Either we build a new function that handles the GDEX way but will require some additional code.
  • Or GDEX applies the way the wallet works with now.

I'm compiling a document explaining the current system that the gateways uses, which I will submit in a few days when I've gotten everything correct.

I recommend we look over a possible merge after this.

@wmbutler
Copy link
Contributor Author

Maybe we just close this and don't include GDEX on the reference wallet since they aren't expressing interest in being part of the discsussion.

@wmbutler wmbutler changed the title [5] Deposit/Withdraw Modal - Add GDEX Support Deposit/Withdraw Modal - Add GDEX Support Apr 12, 2018
@tianyekuo
Copy link
Contributor

Sorry for late response, since the APIs do not work with other gateways, we will make a new set of APIs.

Implement the 3 APIs coins, trading-paires, active-wallet is all we need? @startailcoon @wmbutler

@tianyekuo
Copy link
Contributor

If gdex want to catch up the milestone 21, could you give some help, thanks!

@startailcoon
Copy link
Contributor

I'm working on a document for what is required, why and how. Give me a few days.

Including in next milestone would probably be possible if it turns out good.

@tianyekuo
Copy link
Contributor

Thanks you, I will wait for your good news.

@wmbutler wmbutler reopened this Apr 14, 2018
@wmbutler wmbutler changed the title Deposit/Withdraw Modal - Add GDEX Support [5] Deposit/Withdraw Modal - Add GDEX Support Apr 14, 2018
@wmbutler wmbutler added this to the 180418 milestone Apr 14, 2018
@startailcoon startailcoon removed this from the 180418 milestone Apr 14, 2018
@startailcoon startailcoon added this to the 180501 milestone Apr 14, 2018
@startailcoon
Copy link
Contributor

Moving this to the next milestone as it's quite a challenge to make sure this works.

@startailcoon
Copy link
Contributor

@tianyekuo I have updated a draft documentation for how the Gateway Integration works in the Bitshares UI Reference wallet.

You can review this on https://github.com/bitshares/bitshares-ui/wiki/Gateway-Integration-Requirements

@wmbutler
Copy link
Contributor Author

We pushed the release to 4/18. Any chance you can get this in prior to release? @tianyekuo

@startailcoon
Copy link
Contributor

startailcoon commented Apr 16, 2018

I think it's to little time and I have yet no response from @tianyekuo as well as a very busy schedule at the moment.

I'm also still waiting for approval of my current changes and last minute fixes for it, if needed, which is my priority this milestone.

I do not want to rush this, nor take a task I can't complete.

I'm happy to do the task though.

@tianyekuo
Copy link
Contributor

Our APIs will adjust as the document required, we try to finish it early next week. The next milestone(5/1) is better. Would that be OK with you guys.

@startailcoon
Copy link
Contributor

Great news 👍

@startailcoon
Copy link
Contributor

Issue Update

After extensive communication with Brian Zhang from the GDEX Exchange, we are now very close to add their new API calls to the Deposit/Withdraw Modals.

A few changes to the overall structure will be done to accommodate a few case specific things for GDEX.

We are still on the track to have this completed for the set time.

@wmbutler
Copy link
Contributor Author

@startailcoon great work. Really appreciate you going the extra mile on this.

@tianyekuo
Copy link
Contributor

@startailcoon , thanks for your great work, the document makes the integration better.

@startailcoon
Copy link
Contributor

startailcoon commented Apr 28, 2018

Forked update for BitShares UI with GDEX Gateway support is now available at https://github.com/bitshares/bitshares-ui/tree/1276_GDEXGateway

The following is not included in this as discussed

  • Required User agreement approval
  • Beta user setting
  • Other GDEX specific actions

The old Withdraw/Deposit page will still have all the old bells and whistles and have not changed.

This should be checked and signed of by the GDEX team before merging.

bitshares-gdex

@wmbutler
Copy link
Contributor Author

@tianyekuo

@tianyekuo
Copy link
Contributor

That's great, please help to release it, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[3] Feature Classification indicating the addition of novel functionality to the design
Projects
None yet
Development

No branches or pull requests

4 participants