Skip to content

Prepare lightning-invoice for export in C #903

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

TheBlueMatt
Copy link
Collaborator

This is mostly just C-no export tags. I have a bindings branch that can build bindings given these tags, modulo one or two small things and adding u5 support in Java.

@TheBlueMatt TheBlueMatt force-pushed the 2021-04-invoice-bindings branch from 76edfd3 to 4f73f7e Compare April 28, 2021 21:06
@codecov
Copy link

codecov bot commented Apr 28, 2021

Codecov Report

Merging #903 (c9afea2) into main (0725098) will increase coverage by 0.03%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #903      +/-   ##
==========================================
+ Coverage   90.38%   90.42%   +0.03%     
==========================================
  Files          57       57              
  Lines       29500    29500              
==========================================
+ Hits        26665    26676      +11     
+ Misses       2835     2824      -11     
Impacted Files Coverage Δ
lightning-invoice/src/ser.rs 91.94% <ø> (ø)
lightning/src/chain/chainmonitor.rs 95.52% <ø> (ø)
lightning/src/ln/features.rs 98.81% <ø> (ø)
lightning-invoice/src/de.rs 81.13% <80.00%> (ø)
lightning-invoice/src/lib.rs 83.33% <88.88%> (ø)
lightning/src/ln/functional_tests.rs 97.03% <0.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 0725098...c9afea2. Read the comment docs.

There is generally never a reason to return a non-mutable reference
to a u64 vs just copying it, same applies here. It makes the API
slightly less consistent, but is easier to map in bindings and just
makes more sense.
This prevents aliasing the global secp256k1::Signature name in C
bindings and also makes it a little more explicit that the object
is different from other signature types.
@TheBlueMatt TheBlueMatt force-pushed the 2021-04-invoice-bindings branch from 4f73f7e to 2484c1a Compare April 29, 2021 15:48
@TheBlueMatt
Copy link
Collaborator Author

Oops, pushed one more commit to prevent aliasing two Signature types.

The ChannelSigner bounds are specified both in `impl<>` and in the
`where` clause, which the C bindings generator doesn't like. There
is no reason to have them specified twice.
@TheBlueMatt
Copy link
Collaborator Author

Added a commit which isn't related to invoices, but is required for some bits of lightning bindings.

@TheBlueMatt
Copy link
Collaborator Author

This is all super trivial, gonna merge so we can get invoice bindings out ASAP.

@TheBlueMatt TheBlueMatt merged commit cb0b4bf into lightningdevkit:main Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants