Invoices crate

[valentinewallace]

Add the invoices crate to Rust-Lightning. cc @sgeisler We've had some user requests for using these features to RL language bindings, so the first step'd be pulling in the repo here.

[codecov]

Codecov Report

Merging #870 (c3d25ed) into main (3d80d98) will decrease coverage by 0.41%.
The diff coverage is 82.86%.

@@            Coverage Diff             @@
##             main     #870      +/-   ##
==========================================
- Coverage   90.62%   90.21%   -0.42%     
==========================================
  Files          51       55       +4     
  Lines       27509    28904    +1395     
==========================================
+ Hits        24931    26075    +1144     
- Misses       2578     2829     +251     

Impacted Files	Coverage Δ
lightning-invoice/src/tb.rs	0.00% <0.00%> (ø)
lightning-invoice/src/de.rs	81.00% <81.00%> (ø)
lightning-invoice/src/ser.rs	83.64% <83.64%> (ø)
lightning-invoice/src/lib.rs	84.18% <84.18%> (ø)
lightning/src/ln/functional_tests.rs	96.76% <0.00%> (-0.21%)	⬇️

---

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 3d80d98...c3d25ed. Read the comment docs.

[sgeisler]

I don't know much about licenses, but if you need my ok for this you hereby have it.

[TheBlueMatt]

For sanity, can you git commit --amend --author="Sebastian <[email protected]>" for the first commit and include a pointer in the commit message to the original repo?

Now that I'm thinking more about it, and its pretty much "the wrong way", but I think we could accomplish all we want by (a) adapting bindings to use all the workspace battery crates (which we should do anyway), (b) adapting this crate to be bindings-able, or adapt the bindings to it, either way its a bunch of work and, (c) making this crate depend on the main lightning crate to use its types and expose some utilities that build invoices automagically.

Certainly (c) is a somewhat-awkward "dependency flows the wrong way"-type thing, but rustc should take care of the binary size if a downstream user doesn't use the dependency bits, and it also avoids having to merge the crates entirely, which maybe @sgeisler would prefer, and certainly seems kinda nice in terms of keeping the actual API surface "small".

[sgeisler]

Yes, I'd prefer it to stay a separate crate, merging the repos makes sense though. Making it depend on rust-lightning isn't that pretty, but possibly necessary. Idk if splitting rust-lightning is anything you'd want to do at some point, sometimes that's possible for some base types and could avoid some of the weirdness in the dependency graph (e.g. lightning-invoice would only depend on lightning-types).

Thinking about it a bit more, why is it being a separate crate still useful when it depends on the whole of rust-lightning anyway? Maybe I'm not that opposed to uniting them if it would help, as the standalone use-case would be impacted either way.

Once this lands we should remember to archive the repo and redirect people here.

[TheBlueMatt]

Idk if splitting rust-lightning is anything you'd want to do at some point, sometimes that's possible for some base types and could avoid some of the weirdness in the dependency graph (e.g. lightning-invoice would only depend on lightning-types).

Right, in order to do this we'd have to create a crate that basically only provides Features and its derivatives. I still think thats more of a mess than its really worth, doubly so because the Features type captures a bunch of "what does the main lightning crate currently actually implement/understand in terms of lightning feature bits". The advantage, though, is that the main lightning crate could then depend on invoice without a circular dependency, allowing channel_manager.get_invoice(amount, description) instead of Invoice::from(&ChannelManager, amount, description).

Thinking about it a bit more, why is it being a separate crate still useful when it depends on the whole of rust-lightning anyway?

I guess its more subjective - I'm also happy with merging it wholesale, but if we do think there is value in a standalone/lightweight "invoice" crate, then its mostly a question of having it have a small API surface. Taking a full lightning dependency doesn't matter much because rustc --release will eat most of the underlying uncalled stuff and it won't change the actual build size, just the build runtime a bit the first time (after that it's cached).

[sgeisler]

The advantage, though, is that the main lightning crate could then depend on invoice without a circular dependency

I think as longs as you see merit in it and it's only currently not really a priority enough keeping lightning-invoice separate keeps this option a bit more open. So I still have a slight preference for that. Otherwise modules tend to grow into each other (often needlessly) in my experience.

[TheBlueMatt]

So I still have a slight preference for that

Which, how do you feel about invoice depending on lightning? Its not quite as nice in some ways, but certainly from a Features perspective it may be a bit nicer IMO.

[sgeisler]

Sorry for being vague, I must have been quite tired. I meant that I have a preference for lightning-invoice being its own crate. The dependency on rust-lightning is ok for now imo and I'd hope once rust-lightning settles a bit and there is more time to address minor inconveniences, the "extra features crate" solution will become worth it.

[TheBlueMatt]

I meant that I have a preference for lightning-invoice being its own crate. The dependency on rust-lightning is ok for now imo

Right, then lets go for that for now. We can take this PR as-is once comments are addressed then make it depend on lightnign itself for now and add missing features in a followup.

[sgeisler]

ACK under the assumption that f00bb10 is a pure copy operation which I did not check.