fix(cmd): add network validation in coffee cmd inputs #274

Anyitechs · 2024-04-24T16:37:34Z

This PR validates the argument passed into the -n flag, and ensures only supported CLN networks are created in the /Users/User/.coffee directory, and closes #270 .

Implementation

The current implementation consists of:

An enum (ClnNetwork) with Mainnet, Testnet, Signet, and Regtest as variants
A ClnNetwork implementation (impl) with a from_str function that matches on the network (&str) argument and returns a ClnNetwork variant, and to_str function that matches on the ClnNetwork variant and returns the network String.
The network function validates the -n argument.

Current Behaviour

The validation works as expected, so passing in a random name as the network name will not create the directory but the command returns the same output as below regardless of the network name. Also, passing in a random network name and running coffee -n remote list for the first time ignores the invalid network name and creates a /Users/ifeanyichukwu/.coffee/bitcoin directory for example, but running the command again doesn't create a ../.coffee/bitcoin directory again.

ifeanyichukwu@IfeanyiukwusMBP coffee % coffee -n remote list
● Plugins installed
╭───────────────────────────────────────────╮
│ ●   Language   Name   Enabled   Exec path 
├───────────────────────────────────────────┤
╰───────────────────────────────────────────╯

How to test

cd into the coffee directory
Run make install
Run coffee -n remote list

netlify · 2024-04-24T16:37:51Z

✅ Deploy Preview for coffee-docs canceled.

Name	Link
🔨 Latest commit	`ddaa0dc`
🔍 Latest deploy log	https://app.netlify.com/sites/coffee-docs/deploys/663e32a72b1e1600078188cd

vincenzopalazzo · 2024-04-24T18:12:24Z

coffee_cmd/src/cmd.rs

@@ -1,5 +1,6 @@
 //! Coffee command line arguments definition.
 use clap::{Parser, Subcommand};
+use coffee_lib::{error, errors::CoffeeError};


Suggested change

use coffee_lib::{error, errors::CoffeeError};

use coffee_lib::error;

use coffee_lib::errors::CoffeeError;

vincenzopalazzo · 2024-04-24T18:12:47Z

coffee_cmd/src/cmd.rs

+        match network {
+            "mainnet" => Some(Self::Mainnet),
+            "testnet" => Some(Self::Testnet),
+            "signet" => Some(Self::Signet),
+            "regtest" => Some(Self::Regtest),
+            _ => None,


What about Liquid?

vincenzopalazzo · 2024-04-24T18:13:14Z

coffee_cmd/src/cmd.rs

+    fn to_str(network: Self) -> Option<String> {
+        match network {
+            Self::Mainnet => Some("mainnet".to_string()),
+            Self::Testnet => Some("testnet".to_string()),
+            Self::Signet => Some("signet".to_string()),
+            Self::Regtest => Some("regtest".to_string()),
+        }
+    }


implement the std::fmt::Display trait not use this an hack

vincenzopalazzo · 2024-04-24T18:13:43Z

coffee_cmd/src/cmd.rs

+    fn from_str(network: &str) -> Option<Self> {
+
+        match network {
+            "mainnet" => Some(Self::Mainnet),
+            "testnet" => Some(Self::Testnet),
+            "signet" => Some(Self::Signet),
+            "regtest" => Some(Self::Regtest),
+            _ => None,


implement the From<&str> trait, do not reivent rust

vincenzopalazzo · 2024-04-24T18:14:15Z

coffee_cmd/src/cmd.rs

+        let network = self.network.clone().ok_or_else(|| error!("Network is not set")).ok()?;
+        let validated_network = ClnNetwork::from_str(&network.to_lowercase()).ok_or_else(|| error!("Invalid network name")).ok()?;


I think you do not run make command, this code looks like unformatted.

Anyitechs · 2024-04-25T08:52:56Z

Thank you for the review. I'll work on the requested changes and update the PR.

vincenzopalazzo

Good code, some small minor changes

coffee_cmd/src/cmd.rs

Anyitechs · 2024-05-10T14:46:26Z

Good code, some small minor changes

Thank you for the review. I've addressed all the requested changes and updated the PR.

Let me know if there's anything I'm missing.

vincenzopalazzo

ACK ddaa0dc

vincenzopalazzo · 2024-05-10T15:41:46Z

coffee_cmd/src/cmd.rs

+use coffee_lib::error;
+use coffee_lib::errors::CoffeeError;
+use std::fmt::Display;


look at how we organize the import, we follow the following rules

std import external dependencies import coffee importa crate import

But this can be addressed in a followup PR

Thank you for pointing that out. I'll address that asap!

I've addressed this in #276.

Anyitechs mentioned this pull request Apr 24, 2024

Network Validation in Command Line Inputs #270

Closed

vincenzopalazzo requested changes Apr 24, 2024

View reviewed changes

Anyitechs force-pushed the fix/network-validation branch from ece6c69 to 71a76dc Compare May 9, 2024 22:11

Anyitechs marked this pull request as ready for review May 9, 2024 22:12

Anyitechs requested a review from vincenzopalazzo May 9, 2024 22:12

vincenzopalazzo requested changes May 10, 2024

View reviewed changes

coffee_cmd/src/cmd.rs Outdated Show resolved Hide resolved

coffee_cmd/src/cmd.rs Outdated Show resolved Hide resolved

coffee_cmd/src/cmd.rs Outdated Show resolved Hide resolved

coffee_cmd/src/cmd.rs Outdated Show resolved Hide resolved

fix(cmd): add network validation in coffee cmd inputs

ddaa0dc

Anyitechs force-pushed the fix/network-validation branch from 71a76dc to ddaa0dc Compare May 10, 2024 14:43

vincenzopalazzo approved these changes May 10, 2024

View reviewed changes

vincenzopalazzo merged commit fb966e7 into coffee-tools:master May 10, 2024
8 of 9 checks passed

Anyitechs mentioned this pull request May 14, 2024

chore(cmd): organize imports properly #276

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cmd): add network validation in coffee cmd inputs #274

fix(cmd): add network validation in coffee cmd inputs #274

Anyitechs commented Apr 24, 2024 •

edited

Loading

netlify bot commented Apr 24, 2024 •

edited

Loading

vincenzopalazzo Apr 24, 2024

vincenzopalazzo Apr 24, 2024

vincenzopalazzo Apr 24, 2024

vincenzopalazzo Apr 24, 2024

vincenzopalazzo Apr 24, 2024

Anyitechs commented Apr 25, 2024 •

edited

Loading

vincenzopalazzo left a comment

Anyitechs commented May 10, 2024

vincenzopalazzo left a comment

vincenzopalazzo May 10, 2024

Anyitechs May 10, 2024

Anyitechs May 14, 2024

	use coffee_lib::{error, errors::CoffeeError};
	use coffee_lib::error;
	use coffee_lib::errors::CoffeeError;

		let network = self.network.clone().ok_or_else(\|\| error!("Network is not set")).ok()?;
		let validated_network = ClnNetwork::from_str(&network.to_lowercase()).ok_or_else(\|\| error!("Invalid network name")).ok()?;

fix(cmd): add network validation in coffee cmd inputs #274

fix(cmd): add network validation in coffee cmd inputs #274

Conversation

Anyitechs commented Apr 24, 2024 • edited Loading

Implementation

Current Behaviour

How to test

netlify bot commented Apr 24, 2024 • edited Loading

✅ Deploy Preview for coffee-docs canceled.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anyitechs commented Apr 25, 2024 • edited Loading

vincenzopalazzo left a comment

Choose a reason for hiding this comment

Anyitechs commented May 10, 2024

vincenzopalazzo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anyitechs commented Apr 24, 2024 •

edited

Loading

netlify bot commented Apr 24, 2024 •

edited

Loading

Anyitechs commented Apr 25, 2024 •

edited

Loading