Repository: farcaster-project/farcaster-node
Pull request #766 (2022-11-09 14:44:32)

Config: Define global static bind address
Defines a static bind address that will be used for all listens of a running farcaster instance. 

TheCharlatan: > Adding a required field after config got generated will break and force the user to update is config.

That's clear to me, but why does it break if there is no config present?



Patch:
diff --git a/compose/docker-compose.yml b/compose/docker-compose.yml
index f7c94be7..9706caf2 100644
--- a/compose/docker-compose.yml
+++ b/compose/docker-compose.yml
@@ -4,7 +4,7 @@ services:
     image: "ghcr.io/farcaster-project/farcaster-node/farcasterd:latest"
     pull_policy: "always"
     ports:
-      - "9735:9735"
+      - "7067:7067"
     # the last two keys assume that the build ARG DATA_DIR is /var/lib/farcaster,
     # which is the default value if using the image above; but you may need to
     # change it if using another image
@@ -16,5 +16,5 @@ services:
     depends_on:
       - "walletrpc"
   walletrpc:
-    image: "ghcr.io/farcaster-project/containers/monero-wallet-rpc:0.18.0.0"
+    image: "ghcr.io/farcaster-project/containers/monero-wallet-rpc:0.18.1.2"
     command: "/usr/bin/monero-wallet-rpc --stagenet --disable-rpc-login --wallet-dir wallets --daemon-host stagenet.community.rino.io:38081 --rpc-bind-ip 0.0.0.0 --rpc-bind-port 38083 --confirm-external-bind"
diff --git a/compose/farcasterd.toml b/compose/farcasterd.toml
index 11d52b70..b22b06fb 100644
--- a/compose/farcasterd.toml
+++ b/compose/farcasterd.toml
@@ -1,3 +1,7 @@
+[farcasterd]
+bind_port = 7067
+bind_ip = "0.0.0.0"
+
 [syncers.testnet]
 electrum_server = "ssl://blockstream.info:993"
 monero_daemon = "http://stagenet.community.rino.io:38081"
diff --git a/farcasterd.toml b/farcasterd.toml
index 5de04c7f..cd5ec970 100644
--- a/farcasterd.toml
+++ b/farcasterd.toml
@@ -5,6 +5,12 @@
 [farcasterd]
 # Set this to false if you only want manual restore. Defaut to true
 auto_restore = true
+# Sets the port where Farcaster will accept new peer connections through a
+# listening peerd. Default to 7067
+bind_port = 7067
+# Sets the binding ip for the listening peerd launched by Farcaster. Default to
+# 0.0.0.0
+bind_ip = "0.0.0.0"
 
 # Defines auto-funding
 [farcasterd.auto_funding]
diff --git a/src/bus/ctl.rs b/src/bus/ctl.rs
index 8ce64ecc..324e285c 100644
--- a/src/bus/ctl.rs
+++ b/src/bus/ctl.rs
@@ -183,7 +183,6 @@ pub enum ProgressStack {
 pub struct ProtoPublicOffer {
     pub offer: Offer,
     pub public_addr: InetSocketAddr,
-    pub bind_addr: InetSocketAddr,
     pub arbitrating_addr: bitcoin::Address,
     pub accordant_addr: monero::Address,
 }
diff --git a/src/cli/command.rs b/src/cli/command.rs
index 732c9775..75def1dd 100644
--- a/src/cli/command.rs
+++ b/src/cli/command.rs
@@ -307,8 +307,7 @@ impl Exec for Command {
                 fee_strategy,
                 maker_role,
                 public_ip_addr,
-                bind_ip_addr,
-                port,
+                public_port,
             } => {
                 // Monero local address types are mainnet address types
                 if network != accordant_addr.network.into() && network != Network::Local {
@@ -362,12 +361,10 @@ impl Exec for Command {
                     fee_strategy,
                     maker_role,
                 };
-                let public_addr = InetSocketAddr::socket(public_ip_addr, port);
-                let bind_addr = InetSocketAddr::socket(bind_ip_addr, port);
+                let public_addr = InetSocketAddr::socket(public_ip_addr, public_port);
                 let proto_offer = ctl::ProtoPublicOffer {
                     offer,
                     public_addr,
-                    bind_addr,
                     arbitrating_addr,
                     accordant_addr,
                 };
diff --git a/src/cli/opts.rs b/src/cli/opts.rs
index 8582995f..2156fee9 100644
--- a/src/cli/opts.rs
+++ b/src/cli/opts.rs
@@ -196,17 +196,18 @@ pub enum Command {
         #[clap(long, default_value = "1 satoshi/vByte")]
         fee_strategy: FeeStrategy<SatPerVByte>,
 
-        /// Public IPv4 or IPv6 address present in the public offer allowing taker to connect.
+        /// Public IPv4 or IPv6 address to advertise in the public offer. This allows taker to
+        /// connect; defaults to 127.0.0.1.
         #[clap(short = 'I', long, default_value = "127.0.0.1")]
         public_ip_addr: IpAddr,
 
-        /// IPv4 or IPv6 address to bind to, listening for takers.
-        #[clap(short, long, default_value = "0.0.0.0")]
-        bind_ip_addr: IpAddr,
-
-        /// Port to use; defaults to the native LN port.
-        #[clap(short, long, default_value = "9735")]
-        port: u16,
+        /// Public port to advertise in the public offer; defaults to the FC port 7067.
+        ///
+        /// This port should either be equal to 'farcasterd.bind_port' value in your config file or
+        /// you should setup a proxy to forward trafic from {-I}:{-p} to
+        /// {farcasterd.bind_ip}:{farcasterd.bind_port}
+        #[clap(short = 'p', long, default_value = "7067")]
+        public_port: u16,
     },
 
     /// Taker accepts offer and connects to maker's daemon to start the trade.
diff --git a/src/config.rs b/src/config.rs
index ce6fbc5e..25fe1f4f 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -14,9 +14,11 @@
 
 use crate::Error;
 use farcaster_core::blockchain::Network;
+use internet2::addr::InetSocketAddr;
 use std::fs::File;
 use std::io::prelude::*;
 use std::path::Path;
+use std::str::FromStr;
 
 use serde::{Deserialize, Serialize};
 
@@ -28,6 +30,9 @@ pub const FARCASTER_TESTNET_ELECTRUM_SERVER: &str = "ssl://blockstream.info:993"
 pub const FARCASTER_TESTNET_MONERO_DAEMON: &str = "http://stagenet.community.rino.io:38081";
 pub const FARCASTER_TESTNET_MONERO_RPC_WALLET: &str = "http://localhost:38083";
 
+pub const FARCASTER_BIND_PORT: u16 = 7067;
+pub const FARCASTER_BIND_IP: &str = "0.0.0.0";
+
 pub const GRPC_BIND_IP_ADDRESS: &str = "127.0.0.1";
 
 #[derive(Deserialize, Serialize, Debug, Clone)]
@@ -105,6 +110,24 @@ impl Config {
         }
     }
 
+    /// Returns the bind address to use to instanciate listening peerd, either loads the values
+    /// from the config file or use the default values '0.0.0.0:7067'
+    pub fn get_bind_addr(&self) -> Result<InetSocketAddr, Error> {
+        let addr = if let Some(FarcasterdConfig {
+            bind_ip, bind_port, ..
+        }) = &self.farcasterd
+        {
+            format!(
+                "{}:{}",
+                bind_ip.as_ref().unwrap_or(&FARCASTER_BIND_IP.to_string()),
+                bind_port.unwrap_or(FARCASTER_BIND_PORT)
+            )
+        } else {
+            format!("{}:{}", FARCASTER_BIND_IP, FARCASTER_BIND_PORT)
+        };
+        Ok(InetSocketAddr::from_str(&addr).map_err(|e| Error::Farcaster(e.to_string()))?)
+    }
+
     pub fn get_syncer_servers(&self, network: Network) -> Option<SyncerServers> {
         match network {
             Network::Mainnet => self.syncers.as_ref()?.mainnet.clone(),
@@ -114,6 +137,7 @@ impl Config {
     }
 }
 
+// Default implementation is used to generate the config file on disk if not found
 impl Default for Config {
     fn default() -> Self {
         Config {
@@ -129,19 +153,14 @@ impl Default for Config {
 pub struct FarcasterdConfig {
     /// Sets the auto-funding parameters, default to no auto-fund
     pub auto_funding: Option<AutoFundingConfig>,
+    /// Sets the bind port for potential makers
+    pub bind_port: Option<u16>,
+    /// Sets the bind ip for potential makers
+    pub bind_ip: Option<String>,
     /// Whether checkpoints should be auto restored at start-up, or not
     pub auto_restore: Option<bool>,
 }
 
-impl Default for FarcasterdConfig {
-    fn default() -> Self {
-        FarcasterdConfig {
-            auto_funding: None,
-            auto_restore: Some(true),
-        }
-    }
-}
-
 #[derive(Deserialize, Serialize, Debug, Clone)]
 #[serde(crate = "serde_crate")]
 pub struct GrpcConfig {
@@ -207,6 +226,7 @@ pub struct SyncerServers {
     pub monero_wallet_dir: Option<String>,
 }
 
+// Default implementation is used to generate the config file on disk if not found
 impl Default for SyncersConfig {
     fn default() -> Self {
         SyncersConfig {
@@ -229,6 +249,20 @@ impl Default for SyncersConfig {
     }
 }
 
+// Default implementation is used to generate the config file on disk if not found
+impl Default for FarcasterdConfig {
+    fn default() -> Self {
+        FarcasterdConfig {
+            auto_funding: None,
+            // write the default config for auto-restore
+            auto_restore: Some(true),
+            // write the default port and ip in the generated config
+            bind_port: Some(FARCASTER_BIND_PORT),
+            bind_ip: Some(FARCASTER_BIND_IP.to_string()),
+        }
+    }
+}
+
 pub fn parse_config(path: &str) -> Result<Config, Error> {
     if Path::new(path).exists() {
         let config_file = path;
diff --git a/src/farcasterd/trade_state_machine.rs b/src/farcasterd/trade_state_machine.rs
index 8032a6fb..f56a2620 100644
--- a/src/farcasterd/trade_state_machine.rs
+++ b/src/farcasterd/trade_state_machine.rs
@@ -405,10 +405,19 @@ fn attempt_transition_to_make_offer(
             arbitrating_addr,
             accordant_addr,
             public_addr,
-            bind_addr,
             ..
         })) => {
             // start a listener on the bind_addr
+            let bind_addr = match runtime.config.get_bind_addr() {
+                Err(err) => {
+                    event.complete_ctl(CtlMsg::Failure(Failure {
+                        code: FailureCode::Unknown,
+                        info: err.to_string(),
+                    }))?;
+                    return Ok(None);
+                }
+                Ok(bind_addr) => bind_addr,
+            };
             match runtime.listen(bind_addr) {
                 Err(err) => {
                     warn!("Failed to start peerd listen, cannot make offer: {}", err);
diff --git a/src/grpcd/proto/farcaster.proto b/src/grpcd/proto/farcaster.proto
index bb58f039..29768f8c 100644
--- a/src/grpcd/proto/farcaster.proto
+++ b/src/grpcd/proto/farcaster.proto
@@ -181,8 +181,7 @@ message MakeRequest {
     string fee_strategy = 11;
     SwapRole maker_role = 12;
     string public_ip_addr = 13;
-    string bind_ip_addr = 14;
-    uint32 port = 15;
+    uint32 public_port = 14;
 }
  
 message MakeResponse {
diff --git a/src/grpcd/runtime.rs b/src/grpcd/runtime.rs
index 5c23dba4..bf747a3f 100644
--- a/src/grpcd/runtime.rs
+++ b/src/grpcd/runtime.rs
@@ -752,8 +752,7 @@ impl Farcaster for FarcasterService {
             fee_strategy: str_fee_strategy,
             maker_role: grpc_swap_role,
             public_ip_addr: str_public_ip_addr,
-            bind_ip_addr: str_bind_ip_addr,
-            port,
+            public_port,
         } = request.into_inner();
 
         let network: Network = farcaster::Network::from_i32(grpc_network)
@@ -779,8 +778,6 @@ impl Farcaster for FarcasterService {
             .into();
         let public_ip_addr = IpAddr::from_str(&str_public_ip_addr)
             .map_err(|_| Status::invalid_argument("public ip address"))?;
-        let bind_ip_addr = IpAddr::from_str(&str_bind_ip_addr)
-            .map_err(|_| Status::invalid_argument("bind ip address"))?;
         let fee_strategy: FeeStrategy<SatPerVByte> = FeeStrategy::from_str(&str_fee_strategy).map_err(|_| Status::invalid_argument("
         fee strategy is required to be formated as a fixed value, e.g. \"100 satoshi/vByte\" or a range, e.g. \"50 satoshi/vByte-150 satoshi/vByte\" "))?;
 
@@ -831,12 +828,10 @@ impl Farcaster for FarcasterService {
             fee_strategy,
             maker_role,
         };
-        let public_addr = InetSocketAddr::socket(public_ip_addr, port as u16);
-        let bind_addr = InetSocketAddr::socket(bind_ip_addr, port as u16);
+        let public_addr = InetSocketAddr::socket(public_ip_addr, public_port as u16);
         let proto_offer = ProtoPublicOffer {
             offer,
             public_addr,
-            bind_addr,
             arbitrating_addr,
             accordant_addr,
         };


Test patch:
diff --git a/tests/cfg/fc1.ci.toml b/tests/cfg/fc1.ci.toml
index fd21cb49..316f678a 100644
--- a/tests/cfg/fc1.ci.toml
+++ b/tests/cfg/fc1.ci.toml
@@ -1,5 +1,7 @@
 [farcasterd]
 auto_restore = false
+bind_port = 7067
+bind_ip = "0.0.0.0"
 
 [grpc]
 enable = true
diff --git a/tests/cfg/fc1.toml b/tests/cfg/fc1.toml
index 2fd03ce7..ab402777 100644
--- a/tests/cfg/fc1.toml
+++ b/tests/cfg/fc1.toml
@@ -1,5 +1,7 @@
 [farcasterd]
 auto_restore = false
+bind_port = 7067
+bind_ip = "0.0.0.0"
 
 [grpc]
 enable = true
diff --git a/tests/cfg/fc2.ci.toml b/tests/cfg/fc2.ci.toml
index 9ba9e7d2..ab2a56d7 100644
--- a/tests/cfg/fc2.ci.toml
+++ b/tests/cfg/fc2.ci.toml
@@ -1,5 +1,7 @@
 [farcasterd]
 auto_restore = false
+bind_port = 7067
+bind_ip = "0.0.0.0"
 
 [grpc]
 enable = true
diff --git a/tests/cfg/fc2.toml b/tests/cfg/fc2.toml
index b0920b9d..96f1fee2 100644
--- a/tests/cfg/fc2.toml
+++ b/tests/cfg/fc2.toml
@@ -1,5 +1,7 @@
 [farcasterd]
 auto_restore = false
+bind_port = 7067
+bind_ip = "0.0.0.0"
 
 [grpc]
 enable = true
diff --git a/tests/cli.rs b/tests/cli.rs
index 49b39264..9858de19 100644
--- a/tests/cli.rs
+++ b/tests/cli.rs
@@ -40,8 +40,6 @@ async fn cli_make_offer() {
         "30",
         "--fee-strategy",
         "1 satoshi/vByte",
-        "-p",
-        "9376",
     ];
     args.append(&mut data_dir_maker.iter().map(std::ops::Deref::deref).collect());
 
diff --git a/tests/grpc.rs b/tests/grpc.rs
index 2df686b3..843fe5fa 100644
--- a/tests/grpc.rs
+++ b/tests/grpc.rs
@@ -85,8 +85,7 @@ async fn grpc_server_functional_test() {
         fee_strategy: "100 satoshi/vByte".to_string(),
         maker_role: farcaster::SwapRole::Bob.into(),
         public_ip_addr: "127.0.0.1".to_string(),
-        bind_ip_addr: "0.0.0.0".to_string(),
-        port: 9376,
+        public_port: 7067,
     };
     let request = tonic::Request::new(make_request.clone());
     let response = farcaster_client_1.make(request).await;
diff --git a/tests/swap.rs b/tests/swap.rs
index e9a0176e..b80eaa85 100644
--- a/tests/swap.rs
+++ b/tests/swap.rs
@@ -2028,7 +2028,8 @@ async fn make_and_take_offer(
     let btc_addr = btc_address.to_string();
     let xmr_addr = xmr_address.to_string();
 
-    let (stdout, _stderr) = run("../swap-cli", taker_info_args.clone()).unwrap();
+    let (stdout, stderr) = run("../swap-cli", taker_info_args.clone()).unwrap();
+    info!("stderrr: {:?}", stderr);
     let previous_swap_ids: HashSet<SwapId> =
         cli_output_to_node_info(stdout).swaps.drain(..).collect();
 
@@ -2841,10 +2842,8 @@ fn make_offer_args(
             "1 satoshi/vByte".to_string(),
             "--public-ip-addr".to_string(),
             "127.0.0.1".to_string(),
-            "--bind-ip-addr".to_string(),
-            "0.0.0.0".to_string(),
-            "--port".to_string(),
-            "9376".to_string(),
+            "--public-port".to_string(),
+            "7067".to_string(),
         ])
         .collect()
 }
@@ -2939,6 +2938,7 @@ fn restore_checkpoint_args(data_dir: Vec<String>, swap_id: SwapId) -> Vec<String
 }
 
 fn cli_output_to_node_info(stdout: Vec<String>) -> NodeInfo {
+    debug!("{:?}", stdout);
     serde_yaml::from_str(
         &stdout
             .iter()
@@ -2960,7 +2960,8 @@ fn cli_output_to_funding_infos(stdout: Vec<String>) -> FundingInfos {
 
 async fn retry_until_offer(args: Vec<String>) -> Vec<String> {
     for _ in 0..ALLOWED_RETRIES {
-        let (stdout, _stderr) = run("../swap-cli", args.clone()).unwrap();
+        let (stdout, stderr) = run("../swap-cli", args.clone()).unwrap();
+        debug!("{:?}", stderr);
         let offers: Vec<String> = cli_output_to_node_info(stdout)
             .offers
             .iter()