-
-
Notifications
You must be signed in to change notification settings - Fork 112
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
Force HTTP over Tor for CLNRest #1133
Conversation
WalkthroughWalkthroughThe update focuses on enhancing the Changes
Assessment against linked issues
The changes made in the PR address the main concerns raised in the linked issue about connecting to CLN nodes over Tor, handling Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review due to trivial changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Description
I looked intoproxy-agents
as mentioned in #1107 (comment) but I realizedHttpsProxyAgent
does not supporthttps.Agent
options (see TooTallNate/proxy-agents#234 and TooTallNate/proxy-agents#243). So the naming is confusing since it useshttp.Agent
under the hood. So we can't use it to configure CA certificates.I decided that for now, we can support Tor over HTTP.Close #1107
Updated by copying proxy agents from
hpagent
.This should now work for HTTPS over clearnet (as already did) and HTTP(s) over Tor.
TODO:
add warning and/or info that onion URLs require HTTP (--clnrest-protocol http
)Screenshots
no screenshots because I still need to configure screenshotting on my laptop
Additional Context
For some reason, I get ETIMEDOUT instead of ENOTFOUND now:
Checklist
Are your changes backwards compatible? Please answer below:
Did you QA this? Could we deploy this straight to production? Please answer below:
For frontend changes: Tested on mobile? Please answer below:
Did you introduce any new environment variables? If so, call them out explicitly here: