lightningdevkit / ldk-sample

Sample node implementation using LDK
Apache License 2.0
166 stars 94 forks source link

Return results in JSON format with serde #91

Closed ch1ru closed 1 year ago

ch1ru commented 1 year ago

The purpose of this PR is twofold:

Currently a lot of the responses can only be viewed in the terminal and not all functions fail on an error, instead just printing out the error and returning. It would also be difficult to use the sample node non-interactively, for example via http responses. Serde_json offers consistent formatting for responses and is already a dependency in the sample node.

The PR has the following advantages:

tnull commented 1 year ago

Mh, I'm not sure we want to go this way. ldk-sample is really meant as an educational example to showcase how the different modules of LDK fit together, i.e., it's primary goal is to be read, even more so than to be run. So the more boilerplate and complexity we add, the more the reader has to parse and understand.

Moreover, I really don't think we want to optimize our logging outputs w.r.t. machine readability. If you need to interact with the code non-interactively, you should specify and build-out some kind of API, not just parse in log messages.

ch1ru commented 1 year ago

Fair enough I can understand that. Personally though I find this slightly more readable even if it adds some extra lines of code. I just thought that if it's returning in json format it would be useful to return an actual json object.

tnull commented 1 year ago

It's actually not returning JSON, but simply the default Debug output of the Rust objects.

ch1ru commented 1 year ago

I didn't know that, guess I'll have to look more into that. Thanks