wallet2: daemon-specific proxy for the wallet-rpc #9160
Conversation
0xFFFC0000
commented
Feb 9, 2024
•
0xFFFC0000
commented
- Daemon-specific proxy is exclusive with global proxy (--proxy).
- If you set global proxy (--proxy) you cannot set daemon-specific proxy.
- If you don't set a global proxy, you can set (or not set) a proxy for each daemon connection with the proxy field in jsonrpc to the wallet-rpc.
0xFFFC0000
force-pushed
the
dev/0xfffc/wallet-rpc-proxy
branch
from
6a61d18
to
04a8e84
Compare
|
Why would you want a daemon-specific proxy? |
|
@jeffro256 ofrnxmr asked for it on IRC For example when you initially connect to a clearnet node and then want to use |
|
Ah. I misinterpreted the OP. I thought that wallet2 would be running two proxies lol |
src/wallet/wallet2.cpp
Outdated
| bool wallet2::has_proxy_option(const boost::program_options::variables_map& vm) | ||
| { | ||
| return command_line::has_arg(vm, options().proxy); | ||
| } |
There was a problem hiding this comment.
| bool wallet2::has_proxy_option(const boost::program_options::variables_map& vm) | |
| { | |
| return command_line::has_arg(vm, options().proxy); | |
| } | |
| bool wallet2::has_proxy_option() const | |
| { | |
| return !m_proxy.empty(); | |
| } |
I'd recommend this approach since it reflects the value per wallet2 instance, even if it wasn't initiated by a CLI.
There was a problem hiding this comment.
What you are suggesting makes more sense. I fixed it in a new push. Thanks.
1. Daemon-specific proxy is exclusive with global proxy (--proxy). 2. If you set global proxy (--proxy) you cannot set daemon-specific proxy. 3. If you don't set global proxy, you can set proxy (or not set) proxy for each daemon connection with the proxy field in jsonrpc to the wallet-rpc.
0xFFFC0000
force-pushed
the
dev/0xfffc/wallet-rpc-proxy
branch
from
04a8e84
to
c50ade5
Compare
| { | ||
| boost::lock_guard<boost::recursive_mutex> lock(m_daemon_rpc_mutex); | ||
|
|
||
| if(m_http_client->is_connected()) | ||
| m_http_client->disconnect(); | ||
| CHECK_AND_ASSERT_MES2(m_proxy.empty() || proxy.empty() , "It is not possible to set global proxy (--proxy) and daemon specific proxy together."); | ||
| if(m_proxy.empty()) |
There was a problem hiding this comment.
Shouldn't this be if (!proxy.empty())?
There was a problem hiding this comment.
I see your point and that actually the way I wrote it at first. But I am trying to exactly mimic the way the code base already is. If you look at init method [1], we are setting proxy using set_proxy method regardless of what the value is. And set_proxy does simply sets direct connection when proxy is empty [2].
There was a problem hiding this comment.
I guess it depends if we want to behavior to be to reset the proxy when proxy is an empty string or not. I actually change my mind and like this behavior better. Let's keep it so that it resets every time.
There was a problem hiding this comment.
By the way. Keep in mind the global proxy is not changeable. That is kind of security requirement in a sense when user sets global proxy via --proxy flag. We should not be able to touch/change it, even if user asks.
But the daemon specific proxy is daemon specific. Meaning you set proxy via RPC, to only that daemon. Later if you change daemon you have to set proxy to new daemon too.
0xFFFC0000
changed the title
|
PR is already merged. Changing the title to reflect the PR name rules we have [1]. |
