remove all occurance of libtor #400
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #400 +/- ##
==========================================
+ Coverage 70.43% 72.74% +2.31%
==========================================
Files 34 32 -2
Lines 4245 4099 -146
==========================================
- Hits 2990 2982 -8
+ Misses 1255 1117 -138 ☔ View full report in Codecov by Sentry. |
Wukong247
force-pushed
the
2025-02-06-remove-libtor
branch
3 times, most recently
from
f418975 to
2c7410b
Compare
Cargo.toml
Outdated
| # Used for connecting to the Tor socks port | ||
| tor = [] |
There was a problem hiding this comment.
Nah, we dont, its among those rough edges to remove...
| /// target listening port | ||
| pub target_port: u16, | ||
| /// service port | ||
| pub service_port: u16, | ||
| /// control port | ||
| pub control_port: u16, |
There was a problem hiding this comment.
target port, is the port on which the application run.
service port, is the port what we use after onion address: xyz:{}
control port, this helps to know about tor service.
| @@ -57,6 +68,7 @@ impl Default for MakerConfig { | |||
| ConnectionType::CLEARNET | |||
| } | |||
| }, | |||
| hostname: "ocqkq73acs4qryk5snoiwtpskb2w3wp65basfzw2xcw6mrp57yonygyd.onion".to_string(), | |||
There was a problem hiding this comment.
Instead of using a random default, can we read the hostname from the etc/var files?
There was a problem hiding this comment.
We will require the user to provide that. The reason we don’t want to use etcd is due to permission restrictions.
There was a problem hiding this comment.
There is a problem with the flow currently.
When the maker is started for the first time, the config file doesn't exist, and there is nothing for user to update. Then a default config file is created, and the maker simply goes on with its process sending this default tor address to DNS, so the user then need to stop the server, update the file and restart again. It can happen that by that time the maker has already sent this address to DNS resulting in wrong DNS entry.
There is no visible error for user to notify if their tor address is wrong. Even if it changes for some reason, (maybe they deleted the torrc file) it will simply carry on with the old address, everything looks normal, just they will never receive any client connection.
We need to automate the tor address fetching somehow. asking user to manually ensure makes it prone to error.
Wukong247
force-pushed
the
2025-02-06-remove-libtor
branch
from
964e8c7 to
7be1bb4
Compare
| Add this to `torrc`: | ||
| ```ini | ||
| HiddenServiceDir /var/lib/tor/hidden_service/ | ||
| HiddenServicePort 80 127.0.0.1:8080 |
There was a problem hiding this comment.
Should this be changed to 6102 to match with the defaults?
| rpc_port: 6103, | ||
| min_swap_amount: MIN_SWAP_AMOUNT, | ||
| target_port: 6102, | ||
| service_port: 6102, | ||
| control_port: 9051, | ||
| socks_port: 19050, |
There was a problem hiding this comment.
this should be 9050 as the default tor socks port.
| /// RPC listening port | ||
| pub rpc_port: u16, | ||
| /// Minimum Coinswap amount | ||
| pub min_swap_amount: u64, | ||
| /// target listening port | ||
| pub target_port: u16, |
There was a problem hiding this comment.
lets rename this back to network_port. This is the port where network connections are coming into, i.e, 6102. so that's a more intuitive name.
| /// service port | ||
| pub service_port: u16, |
There was a problem hiding this comment.
lets add a comment saying that we can keep this value same as the network_port, and explain in which cases they should differ.
| }) | ||
| } | ||
|
|
||
| // Method to serialize the MakerConfig into a TOML string and write it to a file | ||
| pub(crate) fn write_to_file(&self, path: &Path) -> std::io::Result<()> { | ||
| let toml_data = format!( | ||
| "network_port = {} | ||
| "target_port = {} |
There was a problem hiding this comment.
we also should add the service_port too.
| fidelity_timelock = {} | ||
| connection_type = {:?}", | ||
| self.network_port, | ||
| self.target_port, | ||
| self.rpc_port, |
There was a problem hiding this comment.
the hostname is not written to file, so theres no way to change the value in default config file right now.
Draft:
Error message in case of no tor process detection and a nice detection logic rather than just pinging sites via socks.