feat(rpc): add flag for connext to unlock response #1967

sangaman · 2020-10-28T16:59:14Z

This adds a flag to the UnlockNode response to indicate whether a Connext client is connected and ready to accept calls. In the cli, ETH gets added to the list of wallets that are unlocked and ready when this new flag is true.

Closes #1932.

raladev · 2020-10-29T18:39:24Z

lib/cli/commands/unlock.ts

  }
  if (response.lockedLndsList.length) {
-    console.log(`The following wallets could not be unlocked: ${response.lockedLndsList.join(', ')}`);
+    console.log(`The following wallets could not be unlocked: ${readyClients.join(', ')}`);


it is incorrect handling of could not be unlocked case.

This case means that something wrong with L2 client and it can not be unlocked (response.lockedLndsList was used because of that before).

But for now it looks like could not be unlocked = client already unlocked as a result it leads to incorrect erroe handling (my lnd can not be unlocked because something wrong with my docker network):

Thanks for catching this, it was an oversight on my part. If a client was already unlocked before the call, it should show up under the list of The following wallets are unlocked and ready - along with the clients that were unlocked when we made this call. Only if the client could not be unlocked (due to being offline, wrong password, etc...) should it show up as could not be unlocked.

raladev

no ETH parameter in happy flow

sangaman · 2020-11-02T18:03:43Z

* [ ]  no ETH parameter in happy flow

This is very strange, I put in tests to double check the logic and everything seems fine. Can you try testing this one more time, and then right after connection can you do a getinfo call and ensure that Connext is running and connected? Thanks.

raladev · 2020-11-03T12:18:39Z

checked again, same result, maybe the problem is in timing:

The following wallets are unlocked and ready: BTC, LTC is printing faster then 03/11/2020 04:03:38.661 [CONNEXT] info: new status: Initialized connext initialization/unlocking

native@ubuntu:~/xud/bin$ ./xucli unlock
Enter master xud password: 
Error: 3 INVALID_ARGUMENT: password is incorrect
native@ubuntu:~/xud/bin$ ./xucli unlock
Enter master xud password: 
xud was unlocked successfully
The following wallets are unlocked and ready: BTC, LTC
native@ubuntu:~/xud/bin$ ./xucli getinfo

General XUD Info
┌───────────────┬────────────────────────────────────────────────────────────────────┐
│ Status        │ Ready                                                              │
├───────────────┼────────────────────────────────────────────────────────────────────┤
│ Alias         │ SnowAthlete                                                        │
├───────────────┼────────────────────────────────────────────────────────────────────┤
│ Node Key      │ 031cfe8fedaf44e4c4ea1b4529441241592dda4ddfc6f8a08b80824384b3bd25f9 │
├───────────────┼────────────────────────────────────────────────────────────────────┤
│ Address       │                                                                    │
├───────────────┼────────────────────────────────────────────────────────────────────┤
│ Network       │ simnet                                                             │
├───────────────┼────────────────────────────────────────────────────────────────────┤
│ Version       │ 1.2.0-3d6474dc                                                     │
├───────────────┼────────────────────────────────────────────────────────────────────┤
│ Peers         │ 2                                                                  │
├───────────────┼────────────────────────────────────────────────────────────────────┤
│ Pairs         │ 4                                                                  │
├───────────────┼────────────────────────────────────────────────────────────────────┤
│ Own orders    │ 0                                                                  │
├───────────────┼────────────────────────────────────────────────────────────────────┤
│ Peer orders   │ 0                                                                  │
├───────────────┼────────────────────────────────────────────────────────────────────┤
│ Pending swaps │ []                                                                 │
└───────────────┴────────────────────────────────────────────────────────────────────┘ 


Connext info:
┌─────────┬────────────────────────────────────────────┐
│ Status  │ Ready                                      │
├─────────┼────────────────────────────────────────────┤
│ Version │ 1.3.6                                      │
├─────────┼────────────────────────────────────────────┤
│ Address │ 0xf99A59A58113AC2C8557DF847A46cA25319B8852 │
├─────────┼────────────────────────────────────────────┤
│ Network │ connext simnet                             │
└─────────┴────────────────────────────────────────────┘ 


LND-BTC Info:
┌──────────┬───────────────────────────────────────────────────────────────────────┐
│ Status   │ Ready                                                                 │
├──────────┼───────────────────────────────────────────────────────────────────────┤
│ Version  │ 0.11.1-beta commit=v0.11.1-beta-simnet                                │
├──────────┼───────────────────────────────────────────────────────────────────────┤
│ Address  │ 02860c8f08a66a0ae3c6f2392584769e84be371ca91798355ff953c2fd27ab8af6    │
│          │ @vztngkaco6erimd6z6rdeqqqrxkltc54fenrhasrkkkgkka4yqfy7xyd.onion:29735 │
├──────────┼───────────────────────────────────────────────────────────────────────┤
│ Alias    │ 02860c8f08a66a0ae3c6                                                  │
├──────────┼───────────────────────────────────────────────────────────────────────┤
│ Channels │ Active: 3                                                             │
├──────────┼───────────────────────────────────────────────────────────────────────┤
│ Network  │ bitcoin simnet                                                        │
└──────────┴───────────────────────────────────────────────────────────────────────┘ 


LND-LTC Info:
┌──────────┬───────────────────────────────────────────────────────────────────────┐
│ Status   │ Ready                                                                 │
├──────────┼───────────────────────────────────────────────────────────────────────┤
│ Version  │ 0.11.0-beta.rc1 commit=v0.11.0-beta.rc1                               │
├──────────┼───────────────────────────────────────────────────────────────────────┤
│ Address  │ 02933ba8898d18d94b2ce59319847e0d8c8788c8aa628a45d591f5124afbe3963a    │
│          │ @tvg4yb7gv2n4gcaoafnl2dwf6syyvl3cevnkz5ejeudyaaex5fbdbgyd.onion:30735 │
├──────────┼───────────────────────────────────────────────────────────────────────┤
│ Alias    │ 02933ba8898d18d94b2c                                                  │
├──────────┼───────────────────────────────────────────────────────────────────────┤
│ Channels │ Active: 1                                                             │
├──────────┼───────────────────────────────────────────────────────────────────────┤
│ Network  │ litecoin simnet                                                       │
└──────────┴───────────────────────────────────────────────────────────────────────┘ 

native@ubuntu:~/xud/bin$ ./xucli getbalance

Balance:
┌──────────┬───────────────┬───────────────────────────────┬────────────────────────────┐
│ Currency │ Total Balance │ Wallet Balance (Not Tradable) │ Channel Balance (Tradable) │
├──────────┼───────────────┼───────────────────────────────┼────────────────────────────┤
│ BTC      │ 4.99966626    │ 0.49984726                    │ 4.499819                   │
├──────────┼───────────────┼───────────────────────────────┼────────────────────────────┤
│ DAI      │ 4000          │ 2500                          │ 1500                       │
├──────────┼───────────────┼───────────────────────────────┼────────────────────────────┤
│ ETH      │ 29.99880449   │ 19.99880449                   │ 10                         │
├──────────┼───────────────┼───────────────────────────────┼────────────────────────────┤
│ LTC      │ 10            │ 5                             │ 5                          │
├──────────┼───────────────┼───────────────────────────────┼────────────────────────────┤
│ USDT     │ 2000          │ 1000                          │ 1000                       │
└──────────┴───────────────┴───────────────────────────────┴────────────────────────────┘
native@ubuntu:~/xud/bin$

connext_unlock.log
xud_unlock.log

sangaman · 2020-11-06T13:56:07Z

Thanks a lot for the thorough checks as usual, I'll get to the bottom of this tho it's still not clear what the issue is (almost certainly something small).

sangaman · 2020-11-10T08:00:10Z

Could you give this another try? Thanks for the hint about connext not initializing in time, I'm pretty sure that's what the issue was.

raladev

Works fine, but i think we should print wallets that were unlocked and could not be unlocked together: @sangaman @kilrau

(for now could not be unlocked message is printing only if all wallets could not be unlocked)

sangaman · 2020-11-10T16:28:23Z

Works fine, but i think we should print wallets that were unlocked and could not be unlocked together: @sangaman @kilrau

(for now could not be unlocked message is printing only if all wallets could not be unlocked)

It looks like in all of those examples you pasted, BTC and LTC are always unlocked right? I don't have this printing ETH in the list of locked clients since it's never actually "locked" - which is also why I tried to use the "ready" terminology instead of just "unlocked". BTC or LTC should get printed in the could not be unlocked list even if some wallets were successfully unlocked, I've got a test that verifies exactly that.

I suppose I could add ETH to the list of "locked" clients (and probably rename it to "locked or not ready") but currently that would be set any time connext is disabled. So I might need to change the connext ready field from a true/false flag to an enum that's ready/not ready/disabled (and simply not print anything in the cli when its disabled). What do you think @kilrau?

I'll rebase this PR again to resolve a conflict.

This adds a flag to the `UnlockNode` response to indicate whether a Connext client is connected and ready to accept calls. In the cli, `ETH` gets added to the list of wallets that are unlocked and ready when this new flag is true. Closes #1932.

kilrau · 2020-11-10T18:25:16Z

For unlock, in my opinion we should invert the logic and only display wallets that failed to unlock. The following wallets are unlocked and ready is not needed - that should resolve the issue that ETH is not in this list.

sangaman · 2020-11-11T04:54:39Z

For unlock, in my opinion we should invert the logic and only display wallets that failed to unlock. The following wallets are unlocked and ready is not needed - that should resolve the issue that ETH is not in this list.

OK, if that's what we want then what we really need is a flag for when connext is not ready, and it's only true when both connext is enabled AND connext is not connected. Does that sound right?

kilrau · 2020-11-11T09:29:36Z

Does that sound right?

Yep!

kilrau · 2020-12-14T10:47:16Z

Can we continue with this one? @sangaman

sangaman added grpc gRPC API connext labels Oct 28, 2020

sangaman requested review from erkarl, kilrau and raladev October 28, 2020 16:59

sangaman self-assigned this Oct 28, 2020

erkarl previously approved these changes Oct 29, 2020

View reviewed changes

raladev requested changes Oct 29, 2020

View reviewed changes

sangaman requested a review from raladev November 1, 2020 22:00

sangaman dismissed erkarl’s stale review via 444cb67 November 1, 2020 23:31

sangaman force-pushed the rpc/connext-unlock branch from 40ffaba to 444cb67 Compare November 1, 2020 23:31

raladev requested changes Nov 2, 2020

View reviewed changes

sangaman force-pushed the rpc/connext-unlock branch 3 times, most recently from b9b1f9c to ac18c97 Compare November 10, 2020 07:28

sangaman requested a review from raladev November 10, 2020 07:59

raladev reviewed Nov 10, 2020

View reviewed changes

sangaman force-pushed the rpc/connext-unlock branch from ac18c97 to 43e7e2f Compare November 10, 2020 16:28

sangaman closed this Jan 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(rpc): add flag for connext to unlock response #1967

feat(rpc): add flag for connext to unlock response #1967

sangaman commented Oct 28, 2020

raladev Oct 29, 2020

sangaman Nov 1, 2020

raladev left a comment

sangaman commented Nov 2, 2020

raladev commented Nov 3, 2020

sangaman commented Nov 6, 2020

sangaman commented Nov 10, 2020

raladev left a comment •

edited

sangaman commented Nov 10, 2020

kilrau commented Nov 10, 2020

sangaman commented Nov 11, 2020

kilrau commented Nov 11, 2020

kilrau commented Dec 14, 2020

feat(rpc): add flag for connext to unlock response #1967

feat(rpc): add flag for connext to unlock response #1967

Conversation

sangaman commented Oct 28, 2020

raladev Oct 29, 2020

Choose a reason for hiding this comment

sangaman Nov 1, 2020

Choose a reason for hiding this comment

raladev left a comment

Choose a reason for hiding this comment

sangaman commented Nov 2, 2020

raladev commented Nov 3, 2020

sangaman commented Nov 6, 2020

sangaman commented Nov 10, 2020

raladev left a comment • edited

Choose a reason for hiding this comment

sangaman commented Nov 10, 2020

kilrau commented Nov 10, 2020

sangaman commented Nov 11, 2020

kilrau commented Nov 11, 2020

kilrau commented Dec 14, 2020

raladev left a comment •

edited