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

Support Binance order #148

Closed
roy1210 opened this issue Oct 13, 2020 · 24 comments
Closed

Support Binance order #148

roy1210 opened this issue Oct 13, 2020 · 24 comments

Comments

@roy1210
Copy link

roy1210 commented Oct 13, 2020

Hi @hewigovens and team.
Thank you for updating docs for Dapp page.

I have tried to run freeze order according to new instruction which is updated on the docs. But unfortunately, ._sendCallRequest(request) is never going to return the request. After debugging, looks function is stacking on utf8ToBytes function forever.
I think method: 'trust_signTransaction', is not working properly since get_accounts order is working well.
This is my code to run freeze function. Kindly please check and advise.
(Please feel free to clone the repo and reproduce this issue!)

Code:
https://github.com/roy1210/beptool_fork/blob/feat/%40walletconnect/client/src/components/pages/Freezer.js#L161

Demo:
https://beptool-fork-git-feat-walletconnectclient.roy1210.vercel.app/

P.S.
The old library, @trustwallet/walletconnect will handle freeze order without any issue. Hence, kindly investigate how this issue happens. Maybe missing something in my code?? Much appreciate if you can add an example of Binance orders as well. Thank you!

Code (Using @trustwallet/walletconnect): https://github.com/roy1210/beptool_fork/blob/feat/%40trustwallet/walletconnect/src/components/pages/Freezer.js#L150

Demo:
https://beptool-fork-git-feat-trustwalletwalletconnect.roy1210.vercel.app/

const request = connector._formatRequest({
    method: 'trust_signTransaction',
    params: [
        {
            network,
            transaction: JSON.stringify(tx),
        },
    ],
});

connector
 // Memo: Didn't return anything
  ._sendCallRequest(request)
  .then(result => {
    // Returns transaction signed in json or encoded format
    console.log(result);
  })
  .catch(error => {
    // Error returned when rejected
    console.error(error);
  });

Screenshot 2020-10-14 at 12 52 50 AM

@hewigovens
Copy link
Contributor

where is utf8ToBytes function call? could you please try to create tx with wallet core proto? https://github.com/trustwallet/wallet-core/blob/master/typescript/tests/index.test.ts#L48

@trustwallet/walletconnect is old wallet connect version, maybe something breaks in latest walletconnect version

@roy1210
Copy link
Author

roy1210 commented Oct 14, 2020

Hi @hewigovens !

What I am trying to say is both 2 branch I showed in above is 99% same code.

The only difference is using @trustwallet/walletconncet or @walletconnect/client(According to the latest docs).

The problem is @trustwallet/walletconncet works without any issue while @walletconnect/client is stacking on .-sendCallRequest(request) function. Both are the same tx object which followed WalletCore's proto message.

So do you think it doesn't make sense the new wallet connect doesn't work while the old wallet connect version works well ?

could you please try to create tx with wallet core proto? https://github.com/trustwallet/wallet-core/blob/master/typescript/tests/index.test.ts#L48

Do you mean make tx object by TW.Binance.Proto.SigingInput.create() ? then pass the tx to transaction: JSON.stringify(tx) in order to make connector._formatRequest?

Would you mind making an example for freeze order with TW.Binance.Proto.SigingInput.create() ? That would be much appreciated since there is no documentation and example code in wallet-core/typescript!

 (btw, I think privateKey is not able to get from outside of the Trust wallet app.)

where is utf8ToBytes function call?

This function is called after .-sendCallRequest(request). I think maybe called by @walletconnect/client library.

@hewigovens
Copy link
Contributor

ok, will try to debug it later this week, meanwhile https://github.com/thorchain/bepswap-web-ui might help you

@roy1210
Copy link
Author

roy1210 commented Oct 14, 2020

Thank you very much for your support.
Yes, I checked this repo when I debugging before.  :D
Looks like they are using the old library @trustwallet/walletconnect: 0.0.12 too

Looking forward to seeing your example code!
After trying the freeze order with the new @walletconnect/client, I’ll try to apply timelock order again!

@roy1210
Copy link
Author

roy1210 commented Oct 14, 2020

Hi @hewigovens

I found the reason why .-sendCallRequest(request) fails.
It's my bad. I didn't set network property in here.

const request = connector._formatRequest({
    method: 'trust_signTransaction',
    params: [
        {
           // Should be `network: NETWORK_ID` instead of `NETWORK_ID`
            NETWORK_ID,
            transaction: JSON.stringify(tx),
        },
    ],
});

I'm sorry about I didn't find out this typo...
After fixed network, the freeze order was running successfully!!

P.S: freezer order was ran successfully but I made tx variable by json format which didn't created by wallet-core proto.
https://github.com/roy1210/beptool_fork/blob/feat/%40walletconnect/client/src/components/pages/Freezer.js#L128

@roy1210
Copy link
Author

roy1210 commented Oct 14, 2020

@hewigovens

could you please try to create tx with wallet core proto? https://github.com/trustwallet/wallet-core/blob/master/typescript/tests/index.test.ts#L48

I have tried to create txInput by wallet-core but didn't success due to Error: TypeError: Cannot read property 'prototype' of null. Could you kindly advise where I missed in this code, please? It would be much helpful to understand wallet-core library more 👍
Thank you!!

           // Error: TypeError: Cannot read property 'prototype' of null
          const txInput = TW.Binance.Proto.SigningInput.create({
            chainId: "Binance-Chain-Tigris",
            accountNumber: account.account_number.toString(),
            sequence: account.sequence.toString(),
            freezeOrder: {
              from: addr,
              symbol: selectedCoin,
              amount: amount,
            }
          })

          const request = context.wallet.walletconnect._formatRequest({
            method: "trust_signTransaction",
            params: [
              {
                network: NETWORK_ID,
                transaction: txInput,
              },
            ],
          });

Ref: core_proto.d.ts

Values:
Screenshot 2020-10-14 at 11 23 12 PM

Not sure if this is the problem.
image

@hewigovens
Copy link
Contributor

hey, I can receive freeze/timeLock order after correcting the types: roy1210/beptool_fork#3

@roy1210
Copy link
Author

roy1210 commented Oct 15, 2020

@hewigovens Thank you very much!!! Let me study later!!

@roy1210
Copy link
Author

roy1210 commented Oct 15, 2020

Hi @hewigovens
Looks like signed timeLock msg is invalided in your PR too. It's a same error, Error: msg should not be nil.
Kindly check my reply.
Once again, thank you very much for your help!

@hewigovens
Copy link
Contributor

@catenocrypt can you help confirm that timeLockOrder is working on mainnet (manually sign a tx with wallet core), or just some data not set correctly

@roy1210
Copy link
Author

roy1210 commented Oct 15, 2020

@hewigovens
By the way, my wallet app version is 5.0. Do you think timelock order should work in this APP version?

Screenshot 2020-10-14 at 11 23 12 PM

@hewigovens
Copy link
Contributor

Yes please always try latest build, we have pushed new release to App Store

@roy1210
Copy link
Author

roy1210 commented Oct 16, 2020

@hewigovens
Should I download from App Store(ver5.2) or from TestFlight(ver5.0)?

@roy1210
Copy link
Author

roy1210 commented Oct 16, 2020

image

@roy1210
Copy link
Author

roy1210 commented Oct 16, 2020

@catenocrypt
Much appreciated if you can test timelock function with your TW app :D

@roy1210
Copy link
Author

roy1210 commented Oct 17, 2020

Hi @hewigovens, @catenocrypt

(↓ is same as my comment in this PR)
I have found a way to achieve timelock with wallet-core. Basically I did following 2 approach to make it.

  • Use the client directly with a call to client.tokens.timeLock. (Which provide by Binance JS SDK)
  • Set a signing delegate on the client that calls into WalletConnect class instance.

However, time relock and time unlock are not working due to return Error: signature verification failed.
I guess wallet-core library has a problem to run time relock and time unlock.
Kindly please help to check it. Thanks!

(Maybe a problem with id in somewhere in the library? Because timelock_order is working well without id...)

image

image

@hewigovens
Copy link
Contributor

Hey @roy1210 , I found the problem, it is caused by id, will push a fix to wallet core soon, Thanks for you detail and patience debugging!

@roy1210
Copy link
Author

roy1210 commented Oct 18, 2020

Hi @hewigovens @catenocrypt
Thank you for your investigate!

There is one more thing wallet-core should aware about time relock.
When a user want to relock with same amount as original locked value (juse extend the lock span), Binance chain assume null or [] (just empty array) in parameter of amount.

This issue may help to explain.
bnb-chain/node-binary#254

And this is our fix in our fork of Binance JS SDK which may help to explain too.
SwingbyProtocol/javascript-sdk@898215c

I’m not seeing anything about the case where coins is omitted. Since it’s repeated in the PB message, it’s essentially an array. It should become null or [] in the json sign doc when empty for the purpose of relocking with the same amount.

@hewigovens
Copy link
Contributor

relock test also added: trustwallet/wallet-core#1143

@roy1210
Copy link
Author

roy1210 commented Oct 19, 2020

Hi @hewigovens
Thank you for your update!
May I know estimated when I can try new version of wallet-core to try time relock and time unlock?

@hewigovens
Copy link
Contributor

will let you know once we have new iOS build

@roy1210
Copy link
Author

roy1210 commented Oct 19, 2020

Noted and thank you!

@roy1210
Copy link
Author

roy1210 commented Oct 26, 2020

Hi @hewigovens
Sorry for asking again, when can I try the latest version of TW or wallet-core for "time relock" and "time unlock" ?

@roy1210
Copy link
Author

roy1210 commented Nov 6, 2020

@hewigovens
The latest version of TW is supporting timelock, relockand unlock order.
Thank you for your great support!!

@roy1210 roy1210 closed this as completed Nov 6, 2020
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

No branches or pull requests

2 participants