btcpayserver / btcpayserver/BTCPayServer.Lightning

LND swagger client fails to parse response for CloseChannelAsync

Open
#38 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C#
Stars
106
Forks
52
Avg merge
2d 1h
Merged PRs (30d)
4

Description

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](https://github.com/lightningnetwork/lnd/blob/3ae46d81f4a2edad06ef778b2940d9b06386d93b/lnrpc/rpc.swagger.json#L599) 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.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with LndSwaggerClient.CloseChannelAsync and the generated response handling that passes the full body to JsonConvert.DeserializeObject. Compare this behavior with the CloseChannel streaming response in the referenced lnd swagger API spec and inspect LnrpcCloseStatusUpdate. Done means CloseChannelAsync can handle the multiple response objects and expose channel-close progress without throwing a deserialization exception.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
api
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.