lncli+proto: use original name in jsonpb marshaler and remove json_name fields in proto

[guggero]

We now use the jsonpb marshaler to convert the RPC responses to JSON in lncli and REST.
The jsonpb has a setting (OrigName: true) to use the original name as defined in the proto file and the explicit json_name definition is not necessary any more.

This PR also formats all .proto files to remove some inconsistencies with white space use.

[halseth]

This is cool. Did you check that all fields get the exact same json names after doing it this way? In case we had typos in the manually crafted names...

[guggero]

Hmm... Good thinking!
I did a little regex magic and found 4 discrepancies:

// 1: autopilot.proto, message QueryScoresRequest
bool ignore_local_state = 2 [json_name = "no_state"];

// 2: rpc.proto, message Utxo: 
AddressType type = 1 [json_name = "address_type"];

// 3: rpc.proto, message ChannelFeeReport:
string chan_point = 1 [json_name = "channel_point"];

// 4: rpc.proto, message PolicyUpdateRequest
bool min_htlc_msat_specified = 8 [json_name = "set_min_htlc_msat"];

1. Should was superfluous anyway because it's a request message and we don't have REST for the subservers yet (so no JSON is involved anywhere).
2. Would be a breaking change for those that use the output of lncli listunspent with other tools by parsing the output.
3. Same here, possibly breaking stuff for people that parse the output of lncli feereport.
4. Would break for users that use REST and specify the field with set_min_htlc_msat instead of min_htlc_msat_specified (both work before this PR). But the API documentation only mentions min_htlc_msat_specified so nobody should be using the shorter form anyway I think.

[halseth]

1. Should was superfluous anyway because it's a request message and we don't have REST for the subservers yet (so no JSON is involved anywhere).

👍

2. Would be a breaking change for those that use the output of lncli listunspent with other tools by parsing the output.

What if we change the proto field name to address_type?

3. Same here, possibly breaking stuff for people that parse the output of lncli feereport.

same, rename to channel_point?

4. Would break for users that use REST and specify the field with set_min_htlc_msat instead of min_htlc_msat_specified (both work before this PR). But the API documentation only mentions min_htlc_msat_specified so nobody should be using the shorter form anyway I think.

How come both work?

[guggero]

Yes, it looks like renaming address to address_type and chan_point to channel_point in the RPC shouldn't break older clients: https://stackoverflow.com/a/45431953

Only projects using our code directly (lndclient doesn't use the RPCs in question) will need to rename also but then it's a compile time error when updating the referenced lnd version.
That's why I renamed the two fields 2 and 3.

How come both work?

The json_name in the .proto file affects the json= part in the rpc.pb.go:
protobuf:"varint,8,opt,name=min_htlc_msat_specified,json=set_min_htlc_msag,proto3" and apparently the REST library just tried both.

[cfromknecht]

thanks for looking into this @guggero. i'm okay the breaking changes for address_type and channel_point as they are fairly contained. i don't think we have a strong requirement for backwards compat in lncli as it is primarily a debugging tool, e.g. in 0.9 we modified all base64 values to hex. changing the field name actually seems like a safer change, since it fail harder rather than giving a possibly bad value. unless there are strong arguments otherwise i think this should be good to go for 0.10.

[halseth]

So now we'll intentionally rename the grpc names for AddressType and ChannelPoint, while the other two ingore_local_state and set_min_whatever will work as before both for grpc and REST?

I think that sounds like a good solution.

[halseth]

LGTM 👍

[cfromknecht]

@guggero needs rebase but otherwise LGTM

[guggero]

Rebased.