Skip to content
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

0.0.123.1 #126

Merged
merged 3 commits into from
Jun 6, 2024
Merged

0.0.123.1 #126

merged 3 commits into from
Jun 6, 2024

Conversation

TheBlueMatt
Copy link
Collaborator

Fixes #125
Fixes #109

Also adds lightning-transaction-sync to the bindings.

Up until now we'd always relied on `Vec`s being imported through a
prelude, but LDK sometimes forgets to keep doing that. Thus, here
we move to also accepting `alloc::vec::Vec` by mapping it to `Vec`
during name resolution.
@TheBlueMatt TheBlueMatt force-pushed the main branch 5 times, most recently from 671222d to 636543d Compare June 6, 2024 00:25
@TheBlueMatt
Copy link
Collaborator Author

Ugh, looks like we can't do lightning-transaction-sync. It currently depends on rustls, which depends on ring, which depends on the latest version of cc (with no reason given aside from that its "the minimum we've tested" 🤦...ring is so stupid) which we can't use because it breaks -Zbuild-std which we require for macOS builds.

@tnull
Copy link

tnull commented Jun 6, 2024

Ugh, looks like we can't do lightning-transaction-sync. It currently depends on rustls, which depends on ring, which depends on the latest version of cc (with no reason given aside from that its "the minimum we've tested" 🤦...ring is so stupid) which we can't use because it breaks -Zbuild-std which we require for macOS builds.

Hum, I'm a bit confused by this:
a) locally I can't reproduce the build failure, genbindings.sh seems to run just fine (although this is on an aarch64 mac)
b) I tried building cc with -Zbuild-std which also succeeds without issue.
c) Why is the inclusion of cc an issue in this case, but not for others (e.g., bitcoin also depends on it I believe?)

Are we positive we still need the pin? Also, I'm a bit confused why we need it exactly and why need to build with -Zbuild-std, which we however only seem to do in the follow-up tests?

@TheBlueMatt
Copy link
Collaborator Author

The issue will only trigger when trying to build for macOS from Linux (which seems to be the only way to get deterministic builds for macOS, see the CI file). It's cause -Zbuild-std doesn't use the Cargo lock file and fails with newer cc. The only way I know how to fix this is to delete the new cc out of the local cargo registry and build with --offline.

@TheBlueMatt
Copy link
Collaborator Author

We can figure out lightning-transaction-sync in a later release.

@TheBlueMatt TheBlueMatt merged commit 05c3f9d into lightningdevkit:main Jun 6, 2024
3 of 6 checks passed
@tnull
Copy link

tnull commented Jun 6, 2024

We can figure out lightning-transaction-sync in a later release.

😩

@TheBlueMatt
Copy link
Collaborator Author

Doesn't have to take long, I'm just at a loss for what we can practically do here, at least until we get a different HTTP client that doesn't need ring.

@tnull
Copy link

tnull commented Jun 6, 2024

Doesn't have to take long, I'm just at a loss for what we can practically do here, at least until we get a different HTTP client that doesn't need ring.

Yeah, although it's not the HTTP client that pulls in the dependency here, but rustls, which is the most reliable (and actually ~only) TLS option to provide TLS support across different architectures.

@TheBlueMatt
Copy link
Collaborator Author

rustls has support for various crypto providers, though, at least in newer versions, so maybe we can use that? aws-lc-rs may not have the same issues as ring.

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.

ChannelDetails::new is now no longer exposed Expose Hostname as_str
2 participants