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

LND swagger client fails to parse response for CloseChannelAsync #38

Open
canndrew opened this issue Oct 7, 2020 · 1 comment
Open

Comments

@canndrew
Copy link

canndrew commented Oct 7, 2020

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.

@NicolasDorier
Copy link
Member

NicolasDorier commented Oct 8, 2020

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.

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