0.1 bindings #3545
Merged
0.1 bindings #3545
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
...as the bindings generation does not currently have the ability to map a reference to a `NodeId` inside a tuple.
Bindings can't handle references in return types, so reduce the visibility to pub(crate).
The bindings currently get confused by the implicit `Sign` type, so we temporarily remove it on the `impl` here.
Re-exports in Rust make `use` statements a little shorter, but for otherwise don't materially change a crate's API. Sadly, the C bindings generator currently can't figure out re-exports, but it also exports everything into one global namespace, so it doesn't matter much anyway.
Having struct fields with references to other structs is tough in our bindings logic, but even worse if the fields are in an enum. Its simplest to just take the clone penalty here.
The scorer currently relies on an associated type for the fee parameters. This isn't supportable at all in bindings, and for lack of a better option we simply hard-code the parameter for all scorers to `ProbabilisticScoringFeeParameters`.
These are not expressible in C/most languages, and thus must be hidden.
Rather than `ChannelMonitor` only being clonable when the signer is clonable, we require all signers to be clonable and then make all `ChannelMonitor`s clonable.
As we cannot express slices without inner references in bindings wrappers.
Bindings don't accept dyn traits, but instead map any traits to a single dynamic struct. Thus, we can always take a specific trait to accept any implementation, which we do here.
Mapping an `Hmac<Sha256>` would require somewhat custom logic as we'd have to behave differently based on generic parameters, so its simplest to just swap it to a `[u8; 32]` instead.
The bindings really should support this, but currently they don't and its late enough in the release cycle I don't want to try to fix that.
tnull
reviewed
Jan 16, 2025
Contributor
There was a problem hiding this comment.
Given we're not exposing the lightning-transaction-sync crate in bindings and all the modules should be hidden behind features anyways, why are these changes necessary?
Collaborator
Author
There was a problem hiding this comment.
Oh, they were just in the changeset cause we'd started trying to expose tx-sync. Might as well leave them so that we can eventually-maybe expose it.
valentinewallace
approved these changes
Jan 16, 2025
TheBlueMatt
merged commit 7e2c5ef
into
lightningdevkit:0.1-bindings
6 of 23 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Basically just a rebase of the existing stuff, but it seems to work.