Open/close channel txid show instead of success #1867

rsercano · 2020-09-04T12:14:46Z

attempts to resolve #1860

Unfortunately I couldn't test connext even I implemented open channel response for txhash, because my connext somehow returns 500 always :(
Connext closechannel (withdraw) doesn't return a tx hash, so I returned an empty array and added a TODO to this part, let me know if that's not correct.
Finally lnd swap client doesn't return tx id for me instead it returns https://github.com/lightningnetwork/lnd/blob/3ae46d81f4a2edad06ef778b2940d9b06386d93b/lnrpc/rpc.proto#L779 so I used it and converted it to object and then to string (https://github.com/ExchangeUnion/xud/compare/openclosechannel-txid-show?expand=1#diff-f1d7503e017b27e9c93329057cd73f05R784), not sure if that's the correct way, please let me know. But no worries if it returns tx id string I print it instead of converting bytes.

raladev · 2020-09-04T15:27:11Z

it would be good to have one naming for all commands (transactionId/transactionIdsList)
@erkarl can we return txid for closechannel eth?
txid from openchannel definitely does not look like valid txid, something wrong with convertion

openchannel
- LTC

{
  "transactionId": "ZcGmT7YkNLs/DkwjI9nl3KOtGNNnEjdi4WWzX55Lebg="
}

- BTC

{
  "transactionId": "4opi3rWGsgQXQBDvdKSymI0NCIdUasC099B6CMViRk0="
}


- ETH

{
  "transactionId": "0x985e0889d3abc577b312198d611894cd7986c8a5e1aa3abbbfcc0bd86b801408"
}

- USDT

simnet > openchannel USDT 5

{
  "transactionId": "0x62854e0db94ab99d9b2fb9710bc93156f479f7b70c25a618abf4966371ae9e7c"
}


withdraw
- LTC

{
  "transactionId": "821f3baa82572e6e1cd891f4c3f4430c64d111ca0d0f37345bc106af9d657b06"
}


- BTC

{
  "transactionId": "8c88bdf19b6e7cf9cb1b83e04f27eb5f8f8e8de20a9e99a49e79adfcfd1cec24"
}

- ETH

{
  "transactionId": "0x7cd0ebef1d5b2b93d1e75c62d4cb6fe5ec7be66d777174b8f8a0cd0fb386a986"
}


- USDT

{
  "transactionId": "0x11886ceef823e4a591f5c9214d19beb3ce3660c504286359608f40585743651e"
}



closechannel
- LTC 

{
  "transactionIdsList": [
    "0be2952abb5594fe65507f17015b91a66e702f3711e456cd94746dcde84a2b8d"
  ]
}


- BTC

{
  "transactionIdsList": [
    "912c16d107a51f2923fbc1ba21c282bb9a69e7df9dbde284aa4cb74dd7551003"
  ]
}


- ETH 

simnet > closechannel ETH

{
  "transactionIdsList": []
}


- USDT

{
  "transactionIdsList": []
}

raladev

Above

rsercano · 2020-09-05T06:10:40Z

it would be good to have one naming for all commands (transactionId/transactionIdsList)

Sure but, closechannel returns a list of transactions since it closes multiple channels.

txid from openchannel definitely does not look like valid txid, something wrong with convertion

Correct but I'm lost here, a little help would be awesome as I mentioned in the third element in my PR description. @raladev @sangaman

kilrau · 2020-09-07T11:24:23Z

it would be good to have one naming for all commands (transactionId/transactionIdsList)

Imo fine as is.

ghost · 2020-09-07T11:40:32Z

@erkarl can we return txid for closechannel eth?

Yes, we can and should.

rsercano · 2020-09-07T11:58:37Z

Let me know when I can implement it please @erkarl, also @sangaman could you please help me with showing txid for lnd grpc openchannel call, I'm really confused there.

proto/xudrpc.proto

lib/swaps/SwapClientManager.ts

lib/service/Service.ts

test/jest/LndClient.spec.ts

proto/xudrpc.proto

lib/lndclient/LndClient.ts

rsercano · 2020-09-08T08:10:42Z

Thank you @sangaman for your great help, but still I can't get the correct tx id I suppose, apart from that other reviews are closed, @raladev and @kilrau could you please check from your perspective?

kilrau · 2020-09-08T11:36:52Z

return txid for closechannel eth

txid from openchannel definitely does not look like valid txid, something wrong with convertion

If these two are done, good from my side

rsercano · 2020-09-09T06:21:39Z

Still having issue with:

txid from openchannel definitely does not look like valid txid, something wrong with convertion

:(

raladev · 2020-09-09T11:34:14Z

Still having issue with:

txid from openchannel definitely does not look like valid txid, something wrong with convertion

:(

#1867 (comment)

@rsercano u need to convert this base64 string to hex format
https://api.lightning.community/?python#openchannelsync

funding_txid_str | string | Hex-encoded string representing the byte-reversed hash of the funding transaction.

base64 -> bytes -> hex in python:

    a = 'ZcGmT7YkNLs/DkwjI9nl3KOtGNNnEjdi4WWzX55Lebg='
    b = base64.b64decode(a)# b = unreadable bytes
    result = b.hex()  # result = 65c1a64fb62434bb3f0e4c2323d9e5dca3ad18d367123762e165b35f9e4b79b8

rsercano · 2020-09-10T10:25:27Z

Thank you both @raladev and @sangaman, fixed.

raladev · 2020-09-10T14:00:10Z

it seems that connext client is returning txid for withdraw(closechannel) call @rsercano
https://github.com/connext/rest-api-client/blob/master/src/client.ts#L316

kilrau · 2020-09-10T15:34:27Z

it seems that connext client is returning txid for withdraw(closechannel) call @rsercano
https://github.com/connext/rest-api-client/blob/master/src/client.ts#L316

Oh yes, that should work in this PR if not the case yet.

sangaman · 2020-09-11T05:28:46Z

lib/lndclient/LndClient.ts

@@ -780,7 +780,8 @@ class LndClient extends SwapClient {
      await this.connectPeerAddresses(uris);
    }

-    await this.openChannelSync(remoteIdentifier, units, pushUnits);
+    const openResponse = await this.openChannelSync(remoteIdentifier, units, pushUnits);
+    return openResponse.hasFundingTxidStr() ? openResponse.getFundingTxidStr() : base64ToHex(openResponse.getFundingTxidBytes_asB64());


Do we need this ternary statement here? I think just return base64ToHex(openResponse.getFundingTxidBytes_asB64()) is enough to keep things simple. Might be worth tidying up but I don't think it's necessarily worth holding up the PR.

According to the documentation this's the field that actually should have transaction id, so I thought it would worth to check it with this statement.

Which documentation?

rsercano · 2020-09-11T07:43:47Z

it seems that connext client is returning txid for withdraw(closechannel) call @rsercano
https://github.com/connext/rest-api-client/blob/master/src/client.ts#L316

Oh yes, that should work in this PR if not the case yet.

Yeap checking now @raladev @kilrau

rsercano · 2020-09-11T07:56:02Z

Implemented close channel txhash too for connext @kilrau @raladev

But would you please squash and merge my commits for this one @sangaman I literally suck at it, and your help would be really appreciated..

Closes #1860.

sangaman · 2020-09-11T13:28:41Z

But would you please squash and merge my commits for this one @sangaman

Done.

Btw in spots like this the trick I've learned is to do git merge origin/master and then git reset --soft origin/master so that you're left with the same commit history as master and just the uncommitted changes from the PR/branch. Then you can add that as a single commit.

Are you able to run one more quick check @raladev ? I can give it a quick manual test too later today.

rsercano · 2020-09-11T18:28:14Z

Thank you @sangaman ☺️ Nice trick!

rsercano requested review from sangaman, kilrau and raladev September 4, 2020 12:15

rsercano self-assigned this Sep 4, 2020

rsercano added lightning Lightning network & lnd integration command line (CLI) Relating to the command line interface tools labels Sep 4, 2020

raladev requested changes Sep 4, 2020

View reviewed changes

sangaman requested changes Sep 7, 2020

View reviewed changes

sangaman added the grpc gRPC API label Sep 7, 2020

raladev requested review from raladev and sangaman September 10, 2020 10:52

raladev previously approved these changes Sep 10, 2020

View reviewed changes

raladev self-requested a review September 10, 2020 13:59

sangaman reviewed Sep 11, 2020

View reviewed changes

rsercano dismissed raladev’s stale review via 15c61db September 11, 2020 07:55

raladev previously approved these changes Sep 11, 2020

View reviewed changes

feat(rpc): add txid to open/close channel response

0669a3f

Closes #1860.

sangaman dismissed raladev’s stale review via 0669a3f September 11, 2020 13:19

sangaman force-pushed the openclosechannel-txid-show branch from 15c61db to 0669a3f Compare September 11, 2020 13:19

raladev requested review from raladev and sangaman September 11, 2020 14:32

raladev approved these changes Sep 11, 2020

View reviewed changes

sangaman approved these changes Sep 11, 2020

View reviewed changes

sangaman merged commit 2dcaf01 into master Sep 11, 2020

sangaman deleted the openclosechannel-txid-show branch September 11, 2020 17:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Open/close channel txid show instead of success #1867

Open/close channel txid show instead of success #1867

rsercano commented Sep 4, 2020 •

edited

Loading

raladev commented Sep 4, 2020 •

edited

Loading

raladev left a comment

rsercano commented Sep 5, 2020 •

edited

Loading

kilrau commented Sep 7, 2020

ghost commented Sep 7, 2020

rsercano commented Sep 7, 2020

rsercano commented Sep 8, 2020

kilrau commented Sep 8, 2020

rsercano commented Sep 9, 2020

raladev commented Sep 9, 2020 •

edited

Loading

rsercano commented Sep 10, 2020 •

edited

Loading

raladev commented Sep 10, 2020

kilrau commented Sep 10, 2020

sangaman Sep 11, 2020

rsercano Sep 11, 2020

sangaman Sep 11, 2020

rsercano commented Sep 11, 2020

rsercano commented Sep 11, 2020

sangaman commented Sep 11, 2020

rsercano commented Sep 11, 2020 •

edited

Loading

Open/close channel txid show instead of success #1867

Open/close channel txid show instead of success #1867

Conversation

rsercano commented Sep 4, 2020 • edited Loading

raladev commented Sep 4, 2020 • edited Loading

raladev left a comment

Choose a reason for hiding this comment

rsercano commented Sep 5, 2020 • edited Loading

kilrau commented Sep 7, 2020

ghost commented Sep 7, 2020

rsercano commented Sep 7, 2020

rsercano commented Sep 8, 2020

kilrau commented Sep 8, 2020

rsercano commented Sep 9, 2020

raladev commented Sep 9, 2020 • edited Loading

rsercano commented Sep 10, 2020 • edited Loading

raladev commented Sep 10, 2020

kilrau commented Sep 10, 2020

sangaman Sep 11, 2020

Choose a reason for hiding this comment

rsercano Sep 11, 2020

Choose a reason for hiding this comment

sangaman Sep 11, 2020

Choose a reason for hiding this comment

rsercano commented Sep 11, 2020

rsercano commented Sep 11, 2020

sangaman commented Sep 11, 2020

rsercano commented Sep 11, 2020 • edited Loading

rsercano commented Sep 4, 2020 •

edited

Loading

raladev commented Sep 4, 2020 •

edited

Loading

rsercano commented Sep 5, 2020 •

edited

Loading

raladev commented Sep 9, 2020 •

edited

Loading

rsercano commented Sep 10, 2020 •

edited

Loading

rsercano commented Sep 11, 2020 •

edited

Loading