Fix bug in Channel

[joemphilips]

Before this commit, fn get_inbound_outbound_available_balance_msat always returns 0.
It is because using cmp::min instead of cmp::max .

[codecov]

Codecov Report

Merging #669 into master will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #669      +/-   ##
==========================================
+ Coverage   91.35%   91.40%   +0.05%     
==========================================
  Files          35       35              
  Lines       21688    21703      +15     
==========================================
+ Hits        19813    19838      +25     
+ Misses       1875     1865      -10     

Impacted Files	Coverage Δ
lightning/src/ln/channel.rs	87.20% <100.00%> (ø)
lightning/src/ln/functional_tests.rs	97.18% <100.00%> (+0.18%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d2d393...54916db. Read the comment docs.

[TheBlueMatt]

LGTM. @valentinewallace can you take a look?

[valentinewallace]

Thanks @joemphilips!!

Pretty weird -- so afaict, this method was added in 3d55d71 to set the corresponding fields in ChannelDetails, but the fields were/are never actually accessed anywhere, hence why no tests caught this. I'm guessing there was a follow-up plan to add them to more tests/fuzzing that didn't happen?

The ChannelDetails as a struct is used a bunch of times, e.g. in list_channels and similar functions, but it's usually just to get the channel ID or to count the number of channels, rarely are other fields accessed.
ChannelDetails's user_id, channel_value_satoshis also seem to be lightly tested or untested.

So, I guess as an alternative, these fields and method could be removed? @TheBlueMatt But I'm fine with merging as is

[joemphilips]

@valentinewallace If list_channels is removed, what is the alternative way to get inbound and outbound capacity? I think the information is vital for some application. e.g. rebalancing and autopilot Ah never mind I misread your comment.

for user_id and channel_value_satoshis , I think both has valid usage. former is useful if several users can create a channel from one LN node. latter is useful to calculate current inbound and outbound htlc stats by taking the diff from [in|out]bound_capacity_msat field.

[TheBlueMatt]

Oops, I didn't look at the full history of this, my bad! Indeed, these should get functional-level tests, I think, but doesn't have to happen here.

[valentinewallace]

@valentinewallace If list_channels is removed, what is the alternative way to get inbound and outbound capacity? I think the information is vital for some application. e.g. rebalancing and autopilot Ah never mind I misread your comment.

for user_id and channel_value_satoshis , I think both has valid usage. former is useful if several users can create a channel from one LN node. latter is useful to calculate current inbound and outbound htlc stats by taking the diff from [in|out]bound_capacity_msat field.

Oh, well you crossed it out but that's a good point that those fields would be useful for users, heh. And, agreed user_id and channel_value fields are good to have around too actually. They just seem under-tested at the moment.