Managment rpc #543

2025-01-19T20:47:35Z

thesimplekid commented

2025-01-19 20:47:35 +00:00

(Migrated from github.com)

Description

closes #456

Notes to the reviewers

A mint management rpc can be started by enabling it in the mintd config. There is a gprpc client cli that can be run by running cargo r --bin cdk-mint-cli to update the mint settings.

If the rpc server is enabled the mint will only read settings from the config file the first time after that the rpc must be used and changes to the mint info in the config file will not be used.

Suggested CHANGELOG Updates

CHANGED

ADDED

REMOVED

FIXED

Checklist

I followed the code style guidelines
I ran just final-check before committing

### Description  ----- closes #456 ### Notes to the reviewers A mint management rpc can be started by enabling it in the mintd config. There is a gprpc client cli that can be run by running `cargo r --bin cdk-mint-cli` to update the mint settings. If the rpc server is enabled the mint will only read settings from the config file the first time after that the rpc must be used and changes to the mint info in the config file will not be used.  ----- ### Suggested [CHANGELOG](https://github.com/cashubtc/cdk/blob/main/CHANGELOG.md) Updates  #### CHANGED #### ADDED #### REMOVED #### FIXED ---- ### Checklist * [x] I followed the [code style guidelines](https://github.com/cashubtc/cdk/blob/main/CODE_STYLE.md) * [x] I ran `just final-check` before committing

crodas (Migrated from github.com) reviewed

2025-01-19 20:47:35 +00:00

thesimplekid (Migrated from github.com) reviewed

2025-01-21 23:39:40 +00:00

.github/workflows/ci.yml

add mint_rpc_cli lib as well as the bin

add `mint_rpc_cli` lib as well as the bin

crates/cdk-mint-rpc/Cargo.toml

					
				@ -0,0 +1,41 @@

				[package]

We don't want to enforce such an old version of tonic, prost and tonic-build so we should be able to support the range to enable support of msrv and newer rust version.

We don't want to enforce such an old version of `tonic`, `prost` and `tonic-build` so we should be able to support the range to enable support of msrv and newer rust version.

thesimplekid commented

2025-01-28 09:39:00 +00:00

(Migrated from github.com)

cc @crodas @davidcaseria

ok300 (Migrated from github.com) reviewed

2025-01-28 12:02:02 +00:00

crates/cdk-mintd/src/main.rs

					
				@ -358,12 +360,51 @@ async fn main() -> anyhow::Result<()> {

				    }

Would it make sense to place it behind a feature flag, like the swagger UI?

thesimplekid (Migrated from github.com) reviewed

2025-01-28 23:46:43 +00:00

crates/cdk-mintd/src/main.rs

					
				@ -358,12 +360,51 @@ async fn main() -> anyhow::Result<()> {

				    }

resolved in 4ce1337a9397a6774b39a43fa779ee9d4bd8b3c2

ok300 commented

2025-02-03 16:28:28 +00:00

(Migrated from github.com)

Fixes #542

ok300 (Migrated from github.com) reviewed

2025-02-03 17:56:31 +00:00

crates/cashu/src/nuts/nut04.rs

				`@ -240,4 +240,16 @@ impl Settings {`

        self.methods
            .iter()
            .position(|settings| &settings.method == method && &settings.unit == unit)
            .map(|index| self.methods.remove(index))

A more compact way to achieve the same.

```suggestion self.methods .iter() .position(|settings| &settings.method == method && &settings.unit == unit) .map(|index| self.methods.remove(index)) ``` A more compact way to achieve the same.

crates/cashu/src/nuts/nut05.rs

				`@ -399,6 +399,18 @@ impl Settings {`

        self.methods
            .iter()
            .position(|settings| settings.method.eq(method) && settings.unit.eq(unit))
            .map(|index| self.methods.remove(index))

Same as in nut04, a more compact way.

```suggestion self.methods .iter() .position(|settings| settings.method.eq(method) && settings.unit.eq(unit)) .map(|index| self.methods.remove(index)) ``` Same as in `nut04`, a more compact way.

crates/cdk-mint-rpc/Cargo.toml

					
				@ -0,0 +1,41 @@

				[package]

A description tag would be useful.

A `description` tag would be useful.

All other CDK crates have a "unified", single version. Since there are so many inter-dependencies, maybe it's good to stick with that model and use the CDK version here (0.6.0)?

crates/cdk-mint-rpc/src/bin/mint_rpc_cli.rs

					
				@ -0,0 +1,196 @@

				use std::path::PathBuf;

unwrap() -> ?

`unwrap()` -> `?`

crates/cdk-mint-rpc/src/bin/mint_rpc_cli.rs

					
				@ -0,0 +92,4 @@

				    tracing::debug!("Using work dir: {}", work_dir.display());

				    let channel = if work_dir.join("tls").is_dir() {

				        // TLS directory exists, configure TLS

        tracing::info!("Found TLS directory, configuring TLS");

A log statement would be more helpful IMO.

```suggestion tracing::info!("Found TLS directory, configuring TLS"); ``` A log statement would be more helpful IMO.

crates/cdk-mint-rpc/src/bin/mint_rpc_cli.rs

					
				@ -0,0 +107,4 @@

				            .connect()

				            .await?

				    } else {

				        // No TLS directory, skip TLS configuration

        tracing::warn!("No TLS directory found, skipping TLS configuration");

```suggestion tracing::warn!("No TLS directory found, skipping TLS configuration"); ```

crates/cdk-mint-rpc/src/lib.rs

					
				@ -0,0 +2,4 @@

				pub mod mint_rpc_cli;

				pub use proto::*;

pub mod mint_rpc_cli;
pub mod proto;
pub use proto::*;

NIT: alphabetic sorting

```suggestion pub mod mint_rpc_cli; pub mod proto; pub use proto::*; ``` NIT: alphabetic sorting

crates/cdk-mint-rpc/src/proto/cdk-mint-rpc.proto

					
				@ -0,0 +16,4 @@

				    rpc UpdateNut04(UpdateNut04Request) returns (UpdateResponse) {}

				    rpc UpdateNut05(UpdateNut05Request) returns (UpdateResponse) {}

				    rpc UpdateQuoteTtl(UpdateQuoteTtlRequest) returns (UpdateResponse) {}

				    rpc UpdateNut04Quote(UpdateNut04QuoteRequest) returns (UpdateNut04QuoteRequest) {}

Is this for manual updating of quote states? If so, isn't this dangerous to allow (i.e. could interfere with the automatic quote state mgmt)?

crates/cdk-mint-rpc/src/proto/cdk-mint-rpc.proto

					
				@ -0,0 +28,4 @@

				    string info = 2;

				}

				message GetInfoResponse {

Maybe this should include the current settings, at least for nut04/05? Alternatively, maybe UpdateNut04/05 should return the current settings?

There are methods to update them, but there is no way to get them. Since update will merge the settings, it would be useful to have a way to see the current ones.

Maybe this should include the current settings, at least for `nut04/05`? Alternatively, maybe `UpdateNut04/05` should return the current settings? There are methods to update them, but there is no way to get them. Since `update` will merge the settings, it would be useful to have a way to see the current ones.

crates/cdk-mint-rpc/src/proto/server.rs

					
				@ -0,0 +1,586 @@

				use std::net::SocketAddr;

                tracing::warn!("No valid TLS configuration found, starting insecure server");

IMO warn is better for the insecure branch.

```suggestion tracing::warn!("No valid TLS configuration found, starting insecure server"); ``` IMO `warn` is better for the insecure branch.

crates/cdk-mintd/Cargo.toml

					
				@ -7,4 +7,3 @@

				homepage = "https://github.com/cashubtc/cdk"

				repository = "https://github.com/cashubtc/cdk.git"

				rust-version = "1.63.0"                            # MSRV

				description = "CDK mint binary"

Removed by mistake?

crates/cdk-mintd/Cargo.toml

					
				@ -11,1 +10,4 @@

				[features]

				default = ["management-rpc"]

				swagger = ["cdk-axum/swagger", "dep:utoipa", "dep:utoipa-swagger-ui"]

Maybe it's better to have this disabled by default?

Keeps the attack surface smaller for the default setup. Mint operators can still enable it if they need it.

Maybe it's better to have this disabled by default? Keeps the attack surface smaller for the default setup. Mint operators can still enable it if they need it.

crates/cdk-mintd/src/main.rs

					
				@ -358,12 +360,51 @@ async fn main() -> anyhow::Result<()> {

				    }

        tracing::info!("RPC not enabled, using mint info from config.");

```suggestion tracing::info!("RPC not enabled, using mint info from config."); ```

thesimplekid (Migrated from github.com) reviewed

2025-02-03 19:34:53 +00:00

crates/cdk-mintd/Cargo.toml

					
				@ -7,4 +7,3 @@

				homepage = "https://github.com/cashubtc/cdk"

				repository = "https://github.com/cashubtc/cdk.git"

				rust-version = "1.63.0"                            # MSRV

				description = "CDK mint binary"

mintd doesn't have an msrv or at least not 1.63 as it pulls in lnd that had a higer msrv and redb

thesimplekid (Migrated from github.com) reviewed

2025-02-03 19:38:59 +00:00

crates/cdk-mintd/Cargo.toml

					
				@ -11,1 +10,4 @@

				[features]

				default = ["management-rpc"]

				swagger = ["cdk-axum/swagger", "dep:utoipa", "dep:utoipa-swagger-ui"]

I think its better to have in the default binary as I think most operator will want it, though it can be disabled at run time if they do not want to so there there is little more expose there from the excess code. The rpc is not started by default at runtime that does default to not start.

Related if you are worried about a smaller attack surface via excess cdk code mintd should not be used as it brings in both dbs, and all the ln backends and then chooses these at runtime. I don't think this really increase the attack surface as its code that is not run, but does increase the binary.

I think its better to have in the default binary as I think most operator will want it, though it can be disabled at run time if they do not want to so there there is little more expose there from the excess code. The rpc is not started by default at runtime that does default to not start. Related if you are worried about a smaller attack surface via excess cdk code mintd should not be used as it brings in both dbs, and all the ln backends and then chooses these at runtime. I don't think this really increase the attack surface as its code that is not run, but does increase the binary.

ok300 (Migrated from github.com) reviewed

2025-02-05 15:22:05 +00:00

crates/cdk-mint-rpc/build.rs

					
				@ -0,0 +1,5 @@

				fn main() -> Result<(), Box<dyn std::error::Error>> {

				    println!("cargo:rerun-if-changed=src/proto/cdk-mint-rpc.proto");

				    tonic_build::compile_protos("src/proto/cdk-mint-rpc.proto")?;

    println!("cargo:rerun-if-changed=src/proto/cdk-mint-rpc.proto");
    tonic_build::compile_protos("src/proto/cdk-mint-rpc.proto")?;

Helps avoid unnecessary recompilations. With this, cargo will only recompile if the proto changed.

```suggestion println!("cargo:rerun-if-changed=src/proto/cdk-mint-rpc.proto"); tonic_build::compile_protos("src/proto/cdk-mint-rpc.proto")?; ``` Helps avoid unnecessary recompilations. With this, `cargo` will only recompile if the proto changed.

ok300 (Migrated from github.com) reviewed

2025-02-05 16:42:40 +00:00

crates/cdk-mint-rpc/Cargo.toml

					
				@ -0,0 +1,41 @@

				[package]

description = "CDK mintd mint managment RPC client and server"

```suggestion description = "CDK mintd mint managment RPC client and server" ```

thesimplekid (Migrated from github.com) reviewed

2025-02-06 09:54:01 +00:00

crates/cdk-mint-rpc/build.rs

					
				@ -0,0 +1,5 @@

				fn main() -> Result<(), Box<dyn std::error::Error>> {

				    println!("cargo:rerun-if-changed=src/proto/cdk-mint-rpc.proto");

				    tonic_build::compile_protos("src/proto/cdk-mint-rpc.proto")?;

Nice thank you

thesimplekid (Migrated from github.com) reviewed

2025-02-06 15:45:53 +00:00

crates/cdk-mint-rpc/src/proto/cdk-mint-rpc.proto

					
				@ -0,0 +28,4 @@

				    string info = 2;

				}

				message GetInfoResponse {

They are shows by curling the info endpoint. But yes more convenient to have here as well. we can add it.

Sign in to join this conversation.

No reviewers

crodas

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

cashubtc/cdk!543

				`@ -358,12 +360,51 @@ async fn main() -> anyhow::Result<()> {`
				`}`

				`@ -0,0 +1,41 @@`
				`[package]`

				`@ -0,0 +1,41 @@`
				`[package]`

				`@ -0,0 +1,196 @@`
				`use std::path::PathBuf;`

				`@ -0,0 +1,586 @@`
				`use std::net::SocketAddr;`