0-value invoices not minimally encoded?

[t-bast]

We've had a report in ACINQ/eclair#1478 that amountless invoices generated by c-lightning wasn't correctly read by our decoder.

The reason is that c-lightning includes a "0" for the amount instead of leaving it blank.
The spec tolerates that (and I updated eclair to handle this case), but maybe you want to instead put a blank ("") value to have it minimally encoded? Or is there a reason you explicitly put the value "0" in there?

[ZmnSCPxj]

Or is there a reason you explicitly put the value "0" in there?

Because the spec tolerates it and it helps catch bugs in other implementations? LOL. Probably just because it results in simpler code to pass around a 0 instead of having a special "not exist" value.

[t-bast]

having a special "not exist" value

That's actually exactly why many languages have the concept of Option / Maybe ;)
But this doesn't exist in C which is totally fine.

Because the spec tolerates it

I'd argue that the spec tolerating too many variations of the same thing can be more a nuisance than a feature because it leads to an explosion of cases to handle and not-very-interesting-nor-useful-for-users bugs to fix, but well this one was a one-line fix so not worth arguing about. Note that I believe lnd may reject these invoices, which may be painful for users.

[ZmnSCPxj]

But this doesn't exist in C which is totally fine.

We can emulate it by passing around a pointer-to-amount, with NULL meaning Nothing and non-NULL meaning Just foo. Adds an indirection and an alloc though. BUT being concerned about the indirection+alloc is arguably optimizing for the Wrong Thing, Lelelelelelelelel.

[rustyrussell]

Was accidental, will fix before upcoming release.

I'm sure we didn't do that originally... hm....

[t-bast]

Yes I think that changed recently, otherwise we would have noticed it sooner :)

[fiatjaf]

That happens with invoices created by passing amount=0.

However, amountless invoices should be created with amount=any, that will generate lnbc1... invoices instead of lnbc01....

[rustyrussell]

@fiatjaf @t-bast Confirmed. If you use amount "any" you get the expected amountless invoice. If you use "0" you get the (expected?) 0-value invoice:

rusty@builder:~$ lightning-cli invoice 0 test-zero-invoice test-zero-invoice 
{
   "payment_hash": "af4f37f5b9410e78b64ab145aacd64629d556010e1ffdefd4c6787209ed4e9e9",
   "expires_at": 1598921607,
   "bolt11": "lntb01p05gcg8pp54a8n0adegy883dj2k9z64ntyv2w42cqsu8laal2vv7rjp8k5a85sdquw3jhxapd0fjhymedd9h8vmmfvdjsxqyjw5qcqp2sp5q4356xwnck97qvqj8a9u7ad3p2lm7nj4qq3aelv9hu5td5yew2dqrzjqvte55g7f27f9smexgsxeyeddyqlaxf8addrveej2kpmd5q4lscw7xtljvqqqhcqqqqqqqqpqqqqqzsqqc9q4gqqqqqqqqqqqqqqqq9qsqvk4c7h7rk72cd7a5nusz4dwve0n62xzv9s5kxyzwynkz2rq6qupnkxgcr54wjvnlld48p576w0q046h40lymz56q27qtz9j5lwny2dgq24sdj3"
}
rusty@builder:~$ lightning-cli invoice any test-any-invoice test-any-invoice 
{
   "payment_hash": "d3058a77d390db061fc785a766c14d0645fa05e719764bc9b47429b1da6f712b",
   "expires_at": 1598921618,
   "bolt11": https://github.com/ShahanaFarooqui"lntb1p05gcgjpp56vzc5a7njrdsv878sknkds2dqezl5p08r9myhjd5ws5mrkn0wy4sdq6w3jhxapdv9h8jttfdemx76trv5xqyjw5qcqp2sp5qqrrj3r59693a6hxuvfccpsg7tvg6ejjauvgp8depgk3ck9x43csrzjqtku54mw7858a2w582dmynlllzmr7sd9afzxapp3dc0zmncu3085qxttmqqqzmgqqqqqqqlgqqqqqqgq9q9q4gqqqqqqqqqqqqqqqq9qsqaf5s6g44slwzz2dhvamhxv6enffpak8g479l06yfdyxgxmsupzyspvf688eysf73gwfylpq7wegatr0dwsg9yl9sh02sqmw6m3sestsp2c0y5y"
}

IMHO we should fail to create zero-value invoices, since clearly @ShahanaFarooqui was confused by this...

[saubyk]

Hi @t-bast @rustyrussell @ShahanaFarooqui
Recently addressed this in the 'C-Lightning-REST library', which RTL is using. If amount passed to the API is 0, I am now catching it and over writing it with "any" string, before calling the invoice command.
Ride-The-Lightning/c-lightning-REST@d53e441