CLI Host, port, and token can be set at runtime #62

litch · 2022-04-25T12:53:52Z

Ok, this is my re-attempt at making the CLI work again

% cargo run --bin senseicli -- --node 0090a039-f1dd-4daa-a7e1-1446f9c9bbae --token 8d55f4eaba224b78788c866
a3be09f8c7dc0c2def0a57e9a55e658ac95689b17 nodeinfo

InfoResponse { node_info: Some(Info { node_pubkey: "02ccd784d90a263c60c50b248d0234e4ecf52a221438b2c525765072af4fb20de5", num_channels: 1, num_usable_channels: 1, num_peers: 1 }) }

It feels pretty verbose to put all that on the CLI, but this actually gets the CLI back to a working state. I'm seeing unrelated errors it looks with listchannels returning an empty set, but the other node can see my channel, and the nodeinfo command shows an active channel (as above). But this gets it to working

* Host, port, and token can be set at runtime * Default host has the protocol specified (http)

johncantrell97 · 2022-04-25T14:00:21Z

Getting closer! You shouldn't need (or require) the admin access token for accessing the individual node apis as those are completely authenticated by the macaroon.

Happy to merge this since it's completely broken in main at the moment but I still think the better path forward is to probably write out the 'admin access token' to filesystem in DATA_DIR/NEWORK/access.token or some fixed path based on data directory and network.

It can try to read it from that path if it's not provided as a cli arg.

Once the database work I'm doing lands I don't think there will even be these node directories anymore since there'll be a single database for all the nodes. Will probably make this simpler so maybe we don't take the cli too far until that lands.

litch · 2022-04-25T14:04:15Z

Yes! I was looking at segmenting off the AdminClient api calls (init, start, create_node, list_nodes, to “want” separate data).

litch · 2022-04-27T13:00:04Z

Ok, I've done a bit of restructuring, and thinking, and going to do a bit more restructuring -

[ ] - The ListNodes command should list the nodes
[ ] - Admin token is written to the caller filesystem

litch · 2022-04-30T23:38:08Z

This turned into a bit of a mess. Sorry. I may re-do it once I have a better feel for what I'm actually trying to accomplish. I've discovered several error states I could put my system into, and trying to help them recover. Trying to add some instrumentation and such to help with debugging. Not sure that it's all totally appropriate in a "repair the CLI" bit of work, and almost certainly not worth doing if I'm just trying to get a few methods looking better. So probably worth making a decision about "smallest mergable/usable" piece of work and go from there.

litch · 2022-04-30T23:38:57Z

src/cli.rs

 use crate::sensei::{
    CloseChannelRequest, ConnectPeerRequest, CreateAdminRequest, CreateInvoiceRequest,
    CreateNodeRequest, GetUnusedAddressRequest, InfoRequest, KeysendRequest, ListChannelsRequest,
    ListNodesRequest, ListPaymentsRequest, ListPeersRequest, OpenChannelRequest, PayInvoiceRequest,
-    SignMessageRequest, StartAdminRequest, StartNodeRequest,
+    SignMessageRequest, StartAdminRequest, StartNodeRequest, PaginationRequest


This is needed (I think) in order to make the CLI actually be able to send paginated requests (which allows, the list of nodes to be more than 0 items long).

litch · 2022-04-30T23:39:23Z

src/cli.rs

@@ -35,14 +39,6 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
        .version("1.0")
        .author("John Cantrell <[email protected]>")
        .about("Control your sensei node from a cli")
-        .arg(


I removed all the CLI knowing about the node data directory.

litch · 2022-04-30T23:39:43Z

src/cli.rs

@@ -51,6 +47,37 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
                .help("Sets the node to issue commands to")
                .takes_value(true),
        )
+        .arg(
+            Arg::new("token")


The token is now passed in on the CLI, along with host and port.

litch · 2022-04-30T23:40:06Z

src/cli.rs

@@ -83,6 +110,13 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
                        .required(true)
                        .index(2)
                        .help("alias to use for this lightning node"),
+                )
+                .arg(
+                    Arg::new("start")


This allows the create child node to pass the start value as well.

litch · 2022-04-30T23:41:23Z

src/cli.rs

-            .connect()
-            .await?;
+        // This is only needed for a couple of queries.  Disabling pagination effectively by large get
+        let pagination = PaginationRequest {


I generally prefer pull request documentation/explanation for potentially surprising things, rather than comments, since comments tend to be lies waiting to happen, but put the explanatory comment there just in case. LMK if you'd like it out.

litch · 2022-04-30T23:42:16Z

src/cli.rs

+
+                let message = response.into_inner();
+
+                let mut config = NodeConfig {


After we get the node created back from senseid, save that data to the local (to the CLI) filesystem for future use. I'm not exactly sure the best place to put this data_dir and would appreciate some input.

litch · 2022-04-30T23:42:56Z

src/cli.rs

+    }
+}
+
+// I could not figure out how to improt this from the database/admin


It may be easy to import this Role from the database::admin but for whatever reason I just couldn't make it happen.

litch · 2022-04-30T23:43:28Z

src/database/admin.rs

@@ -360,10 +361,21 @@ impl AdminDatabase {
            ":pubkey": node.pubkey,
            ":status": node.status
        })?;
-
+        dbg!("Wrote node to the database");


I'll remove some of these dbg!s. I went a little crazy with them trying to understand why the code was failing.

litch · 2022-04-30T23:46:42Z

src/database/admin.rs

        Ok(self.connection.last_insert_rowid())
    }

+    pub fn clear_pending_node(&mut self) -> Result<(), Error> {


So this is an error state I could pretty reliably put my system into.

Basically it goes

Create node in the sensei sqlite DB (empty string pubkey)

Create the actual lightning node (gets a pubkey)

Update the sensei sqlite record with the pubkey

If there was ever a problem creating the node in the lightning node or updating the sqlite record, then there is an existing empty-string-for-a-pubkey "node" that's basically WIP for the system, but a new node can never be created because it can't write any more "new node with empty string pubkey". So I made this to be able to "clean up after itself."

I actually think that's probably a wrong approach, and we should have a different table for like, "nodes to create" and then a "nodes created" and it's not just, "update the node record in place with a pubkey", but that's a bit larger scope.

litch · 2022-04-30T23:46:54Z

src/database/admin.rs

+        statement.execute(named_params! {
+            ":blank": "",
+        })?;
+        dbg!("I deleted the node with no pubkey because it sucks!");


Ooops, will pull this.

litch · 2022-04-30T23:47:06Z

src/database/admin.rs

@@ -376,7 +388,7 @@ impl AdminDatabase {
            ":pubkey": node.pubkey,
            ":status": node.status
        })?;
-
+        dbg!("Ok now I just wrote a node update.  It probably has a pubkey");


TODO remove

litch · 2022-04-30T23:47:58Z

src/grpc/admin.rs

@@ -8,6 +8,7 @@
 // licenses.

 use std::sync::Arc;
+use tracing::{instrument, info, error, debug};


I don't know how you feel about tracing/logging, but it seems like we need to get some kind of structured something in here. This library feels pretty fantastic for doing it. We can pull it back out for something better, but it felt good to me to use.

litch · 2022-04-30T23:49:09Z

src/services/admin.rs

+            node.id = database.create_node(node.clone())
+                .map_err(|err| {
+                    error!("Unable to create node in Sensei Database");
+                    database.clear_pending_node();


This is a "Cleaning up after myself" I mentioned before. I got pretty good at putting my instance in this broken state.

litch · 2022-04-30T23:50:57Z

src/services/admin.rs


-        let lightning_node = self.get_node(node.clone(), passphrase).await?;
+        let lightning_node = self.get_node(node.clone(), passphrase).await.unwrap();


I have the explicit unwrap here because it felt like a panic at this stage, if it failed, is pretty acceptable. Not sure - the previous behavior was, "Fail silently" and this seems better but not necessarily optimal.

litch · 2022-04-30T23:51:04Z

src/services/admin.rs

        node.pubkey = lightning_node.node_info()?.node_pubkey;
-
+        dbg!("I have a pubkey even {:?}", node.pubkey.clone());


Todo remove

* Host, port, and token can be set at runtime * Default host has the protocol specified (http)

…on local disk

recoverable

litch added 4 commits April 22, 2022 14:01

CLI takes an argument to set the endpoint correctly if in dev mode

8ebf64a

Health endpoint is added for sanity checking

b02006a

Extraneous space is removed

946e65b

Host, port, and token can be set at runtime (#1)

a01a890

* Host, port, and token can be set at runtime * Default host has the protocol specified (http)

litch mentioned this pull request Apr 25, 2022

Add --dev argument to the CLI #58

Closed

litch commented Apr 30, 2022

View reviewed changes

litch force-pushed the cli-args branch from ef62fff to a6aa2f9 Compare May 6, 2022 12:05

litch added 5 commits May 7, 2022 13:03

Merge branch 'L2-Technology:main' into main

c0c0d6b

Host, port, and token can be set at runtime (#1)

b8d9db6

* Host, port, and token can be set at runtime * Default host has the protocol specified (http)

The macaroon is passed in on the command line, rather than looked up …

f32f8d9

…on local disk

Large pagination request provided so that list results work

142d147

Listpeers request does not use pagination

0d8a601

litch added 8 commits May 7, 2022 13:10

Instrumentation is added/improved, a failure state of node creation is

04a14b2

recoverable

Removed extraneous debug statements :

b9cb720

Passphrase can be passed in as an arg

5ed9390

Extract method - thanks @futurepaul

ba22139

The response message is parsed and output as correct json

a6d6686

Invoice creation is returned as JSON

8e5771c

Addresses can be fetched, passphrase get/set is renamed

54b4628

A number of changes are reverted

d8e1a33

litch force-pushed the cli-args branch from 4565fd0 to d8e1a33 Compare May 7, 2022 18:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLI Host, port, and token can be set at runtime #62

CLI Host, port, and token can be set at runtime #62

litch commented Apr 25, 2022

johncantrell97 commented Apr 25, 2022

litch commented Apr 25, 2022

litch commented Apr 27, 2022 •

edited

Loading

litch commented Apr 30, 2022

litch Apr 30, 2022

litch Apr 30, 2022

litch Apr 30, 2022

litch Apr 30, 2022

litch Apr 30, 2022

litch Apr 30, 2022

litch Apr 30, 2022

litch Apr 30, 2022

litch Apr 30, 2022

litch Apr 30, 2022

litch Apr 30, 2022

litch Apr 30, 2022

litch Apr 30, 2022

litch Apr 30, 2022

litch Apr 30, 2022


		let message = response.into_inner();

		let mut config = NodeConfig {


		let lightning_node = self.get_node(node.clone(), passphrase).await?;
		let lightning_node = self.get_node(node.clone(), passphrase).await.unwrap();

		node.pubkey = lightning_node.node_info()?.node_pubkey;

		dbg!("I have a pubkey even {:?}", node.pubkey.clone());

CLI Host, port, and token can be set at runtime #62

Are you sure you want to change the base?

CLI Host, port, and token can be set at runtime #62

Conversation

litch commented Apr 25, 2022

johncantrell97 commented Apr 25, 2022

litch commented Apr 25, 2022

litch commented Apr 27, 2022 • edited Loading

litch commented Apr 30, 2022

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

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

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

litch commented Apr 27, 2022 •

edited

Loading