fix: connext status shouldn't show Ready unless connection verified

[rsercano]

resolves #2038

• I'm still unable to access connext and it was still showing ready, so since it's not really much to do to fix this, I decided to take this one.

Can you please check?

[erkarl]

Thanks for the fix. Let's wait for https://github.com/ExchangeUnion/xud-docker/pull/821 to get merged so that this can be properly tested against master.

[rsercano]

• [ ] not sure that Disconnected is correct status for 500 Err, because xud was able to connect to connext client (just connext client was not able to connect to node)

• Disconnected status - xud cant connect to connext client (CONNREFUSED err, i get it sometimes);
• Starting status - connext client is trying to connect to vector node
• Error status - connext was not able to connect to node (response code ~=200)
• Ready - Connext works fine and i able to retrieve balance

is it possible implement connext statuses in such way? @rsercano @erkarl

This PR shows the status that's being kept in the ConnextClient, so it's current status is Disconnected according to our existing implementation.

It's definetly possible to change this but this means we need to change implementation of setting status during connection to connext @sangaman @erkarl @raladev @kilrau

[erkarl]

It's definetly possible to change this but this means we need to change implementation of setting status during connection to connext @sangaman @erkarl @raladev @kilrau

I'd lean towards not touching it for the next couple of days as I'm making more changes to ConnextClient. I think we can get this short PR in as is.

What changes are you proposing?

[rsercano]

It's definetly possible to change this but this means we need to change implementation of setting status during connection to connext @sangaman @erkarl @raladev @kilrau

I'd lean towards not touching it for the next couple of days as I'm making more changes to ConnextClient. I think we can get this short PR in as is.

What changes are you proposing?

I mean we need to set & create new statuses to provide the implementation @raladev asked for @erkarl. And need to set these statuses accordingly while connecting

[sangaman]

It's definetly possible to change this but this means we need to change implementation of setting status during connection to connext @sangaman @erkarl @raladev @kilrau

I'd lean towards not touching it for the next couple of days as I'm making more changes to ConnextClient. I think we can get this short PR in as is. What changes are you proposing?

I mean we need to set & create new statuses to provide the implementation @raladev asked for @erkarl. And need to set these statuses accordingly while connecting

Yes, I think that's out of scope of this PR, and not a trivial change. Here are our current statuses.

enum ClientStatus {
  /** The starting status before a client has initialized. */
  NotInitialized,
  /** The client has been initialized but has not attempted to connect to the server yet. */
  Initialized,
  /** The client is permanently disabled. */
  Disabled,
  /** The server cannot be reached or is not responding properly. */
  Disconnected,
  /** The server is reachable and operational. */
  ConnectionVerified,
  /** The server is reachable but is not ready pending completion of blockchain or network sync. */
  OutOfSync,
  /** The server is reachable but needs to be unlocked before it accepts calls. */
  WaitingUnlock,
  /** The server has been unlocked, but its status has not been verified yet. */
  Unlocked,
  /** The client could not be initialized due to faulty configuration. */
  Misconfigured,
  /** The server is reachable but hold invoices are not supported. */
  NoHoldInvoiceSupport,
}

I think Initialized corresponds to the "Starting" status. We don't have an "Error" status exactly but we can add one that covers this case for Connext, or think about combining it with and possibly renaming an existing status.

Perhaps a better solution altogether is to use the error field on ConnextInfo to display a message about why connext is disconnected. Either way I don't think it makes sense to address this in this PR.