search

joemphilips / DotNetLightning

Utility to work with Lightning network with .NET
https://joemphilips.github.io/DotNetLightning
MIT License
38 stars 19 forks source link

TODO: add testcase from BOLT07 for extended-queries.json #64

Open joemphilips opened 4 years ago

joemphilips commented 4 years ago

I've just realized that BOLT has json file of testcase for extended queries here

Right now, it only has property tests. It will be good starting point if someone wants to start understanding this library and F# / Expecto / System.Text.Json in general. Imitating TLVSerialize.fs might be good way to get done.

knocte commented 4 years ago

Why System.Text.Json instead of Newtonsoft.Json?

joemphilips commented 4 years ago

Just for the sake of consistency and performance. It is fine to use Newtonsoft as well.

knocte commented 4 years ago

Consistency with what? WRT the latter: IMO correctness is better.

joemphilips commented 4 years ago

Consistancy with other part of the test code. Correctness does not change by witch json library we use.

knocte commented 4 years ago

Consistency with other part of the test code.

This made me realise I was misunderstanding our conversation. It turns out the first time I read this issue I misread "System.Text.Json" as "ServiceStack.Json" (https://www.nuget.org/packages/ServiceStack.Text/). Now it's all clear.

Correctness does not change by witch json library we use.

You would be surprised! I was actually raising this because I know cases of differing behaviour between ServiceStack.Json and Newtonsoft.Json (that would make you understand why the former is actually faster).

knocte commented 4 years ago

I know cases of differing behaviour between ServiceStack.Json and Newtonsoft.Json

One example (and this is just one of them): https://stackoverflow.com/questions/11882987/why-servicestack-text-doesnt-default-dates-to-iso8601

joemphilips commented 4 years ago

Interesting. Well, in this case it is just for reading json from flat file so I don't think it is a big deal. But I got your point.