btcpayserver/BTCPayServer.Lightning

LND swagger client fails to parse response for CloseChannelAsync

canndrew opened this issue · 1 comments

I'm calling LndSwaggerClient.CloseChannelAsync. The response I'm getting from lnd is:

{"result":{"close_pending":{"txid":"WlbkadALmDboZFa94uXU/g2IhANqirhx2QlLeMlytFw=","output_index":0}}}
{"result":{"chan_close":{"closing_txid":"WlbkadALmDboZFa94uXU/g2IhANqirhx2QlLeMlytFw=","success":false}}}

The swagger client fails to parse this response and throws an exception:

   System.AggregateException : One or more errors occurred. (Could not deserialize the response body.)
  ----> BTCPayServer.Lightning.LND.SwaggerException : Could not deserialize the response body.
  ----> Newtonsoft.Json.JsonReaderException : Additional text encountered after finished reading JSON content: {. Path '', line 2, position 0.

Clearly it's not happy that lnd returned multiple results. A look at the code shows that the entire response body is passed to a single call to JsonConvert.DeserializeObject, which fails because the response contains multiple objects.

A look at the lnd swagger API spec says that this is expected behaviour (annotated):

"operationId": "CloseChannel",
"responses": {
  "200": {
    "description": "A successful response.(streaming responses)",       // <- "streaming responses"
    "schema": {
      "type": "object",
      "properties": {
        "result": {
          "$ref": "#/definitions/lnrpcCloseStatusUpdate"
        },
        "error": {
          "$ref": "#/definitions/runtimeStreamError"
        }
      },
      "title": "Stream result of lnrpcCloseStatusUpdate"                // <- "Stream result"
    }
  },
  "default": {
    "description": "An unexpected error response",
    "schema": {
      "$ref": "#/definitions/runtimeError"
    }
  }
},

I'm not sure how to fix this since the LndSwaggerClient code is automatically generated from the API spec. Closing a channel takes time and happens in stages, so CloseChannelAsync needs to return some kind of stream or event source that the user can use to monitor the progress of the operation. It seems like it's already supposed to do that since LnrpcCloseStatusUpdate inherits from System.ComponentModel.INotifyPropertyChanged, so maybe CloseChannelAsync is correct to return a single LnrpcCloseStatusUpdate and that value should then be getting updated when new responses come in. But it's not clear to me whether or how that's implemented, and either way it's not working correctly.

Ah, that's probably why they don't manage to make it work there.

#37 (comment)

@canndrew you can modify manually the LndSwaggerClient.
We initially generated it, but we do changes manually since we don't want to break backward compat.