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

Use native-tls for better close_notify handling #41

Conversation

ok300
Copy link
Contributor

@ok300 ok300 commented Apr 26, 2024

This PR enables the native-tls feature of ureq as described here: https://docs.rs/ureq/latest/ureq/#https--tls--ssl

This brings better handling of TLS close_notify alerts. Before this change, the default ureq setup caused some POST calls (like /swapstatus) to fail.

Fixes #39

@i5hi
Copy link
Collaborator

i5hi commented Apr 26, 2024

Hey @ok300 Thanks for the PR!

You will need to setup PGP signing for your commits. Didn't catch it on your last few PR's.

You can check out the docs here: https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits

Also the rust-fmt test is complaining :(

@ok300 ok300 force-pushed the ok300-fix-swapstatus-close_notify-errors branch from 704abee to 0763847 Compare April 26, 2024 15:10
@ok300
Copy link
Contributor Author

ok300 commented Apr 26, 2024

Done, signed the commits and ran cargo fmt.

@i5hi
Copy link
Collaborator

i5hi commented Apr 26, 2024

Awesome!

There is a huge PR that I have to review first and get merged which came in before this. This PR will create a merge conflict with that one. I'll have to hold off on merging this. Once that is merged, we will get back to this.

For now, you can carry on using your fork in your project so you aren't blocked.

@i5hi
Copy link
Collaborator

i5hi commented May 1, 2024

Hey, the other PR has been merged. You can sync and commit your changes.

@i5hi
Copy link
Collaborator

i5hi commented May 1, 2024

you potentially saved me hours of debugging with this PR!

we just faced this issue on android in an upstream project that uses boltz-dart.

I've updated boltz.rs & boltzv2.rs to use native-tls for POST calls.

Thank you!

Please share a Liquid address so I can tip you :)

@ok300
Copy link
Contributor Author

ok300 commented May 2, 2024

Awesome, I'll rebase soon and update this PR!

Maybe you can use my LN address [email protected] , much appreciated 🙏

@ok300
Copy link
Contributor Author

ok300 commented May 2, 2024

I see the relevant changes are already in trunk, so no need to rebase and use this PR anymore.

I'll just close it, to keep things simple.

@ok300 ok300 closed this May 2, 2024
@i5hi
Copy link
Collaborator

i5hi commented May 3, 2024

Pheonix says unable to connect to oak-node.net

@ok300 ok300 deleted the ok300-fix-swapstatus-close_notify-errors branch August 15, 2024 11:45
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.

swap status fails on mainnet
2 participants