feat: better invoice expiration

[douglaz]

Anyone that has used lightning in the real world has probably suffered with the usual 1 hour default expiration time on invoices. Let's help a bit the users by giving them a better default.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (83c8f51) 57.10% compared to head (5142e28) 57.09%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3927      +/-   ##
==========================================
- Coverage   57.10%   57.09%   -0.02%     
==========================================
  Files         193      193              
  Lines       42907    42907              
==========================================
- Hits        24503    24498       -5     
- Misses      18404    18409       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[elsirion]

Should it just be configurable instead? Let the integrator worry about that: they can choose a default or give the user a way to configure it.

[douglaz]

Should it just be configurable instead? Let the integrator worry about that: they can choose a default or give the user a way to configure it.

It's already configurable: just use Some(expiry_time) instead of None. This default is only for clueless wallets and integrators.

But one option is to not let it be optional. Then everyone will be required to make a decision.

[dpc]

nitpick: That conversion back and forth from/to Duration looks weird. Would expire_time.map(Duration::from_secs).unwrap_or(DEFAULT_INVOICE_EXPIRY_TIME) work? I would just do somewhere before:

let expire_time = expire_type.map(Duration::from_secs);

to get it of the way, then expire_time.unwrap_or(DEFAULT_INVOICE_EXPIRY_TIME) would work just perfectly.

[dpc]

No idea if this is a good idea, but technically PR LGTM.

[elsirion]

@douglaz oh, you are right, I was just confused.

[justinmoon]

Default expiration of 24 hours sounds good to me