Clear and Blind Auth #510

2024-12-22T03:05:29Z

thesimplekid commented

2024-12-22 03:05:29 +00:00

(Migrated from github.com)

Description

closes #69

Notes to the reviewers

Suggested CHANGELOG Updates

CHANGED

ADDED

REMOVED

FIXED

Checklist

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

### Description  ----- closes #69 ### Notes to the reviewers  ----- ### 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

ok300 (Migrated from github.com) reviewed

2025-01-30 10:13:22 +00:00

ok300 (Migrated from github.com) reviewed

2025-02-21 14:30:41 +00:00

crates/cashu/src/nuts/nut21/mod.rs

    /// Protected endpoints

```suggestion /// Protected endpoints ```

crates/cdk-mintd/src/main.rs

Does this need to be a separate DB?

It makes DB backup and restore more complicated (2 files to backup or restore, on restore the correct pair of files has to be used).

Does this need to be a separate DB? It makes DB backup and restore more complicated (2 files to backup or restore, on restore the correct pair of files has to be used).

crates/cdk/src/mint/auth/mod.rs

					
				@ -0,0 +1,413 @@

				use cdk_common::{CurrencyUnit, MintKeySet};

Perhaps is_protected is better?

Perhaps `is_protected` is better?

crates/cdk/src/wallet/mod.rs

nutxx can be renamed to nut21

`nutxx` can be renamed to `nut21`

nutxx1 can be renamed to nut22

`nutxx1` can be renamed to `nut22`

thesimplekid (Migrated from github.com) reviewed

2025-02-21 17:50:12 +00:00

crates/cdk-mintd/src/main.rs

It makes sense to have two separate databases since, while there is large overlap between them, they serve different purposes. For example, the auth database may grow faster as keysets are likely to be rotated more frequently. This separation also helps with separation of concerns between the auth stuff and ecash stuff.

I don't think an extra file adds too much complexity. The auth database is also less critical to back up since if it's lost, the mint can simply create new auth keysets, require everyone to reauth, and generate new tokens.

It makes sense to have two separate databases since, while there is large overlap between them, they serve different purposes. For example, the auth database may grow faster as keysets are likely to be rotated more frequently. This separation also helps with separation of concerns between the auth stuff and ecash stuff. I don't think an extra file adds too much complexity. The auth database is also less critical to back up since if it's lost, the mint can simply create new auth keysets, require everyone to reauth, and generate new tokens.

thesimplekid (Migrated from github.com) reviewed

2025-02-26 11:49:48 +00:00

crates/cashu/src/nuts/nut21/mod.rs

21

thesimplekid (Migrated from github.com) reviewed

2025-02-26 11:49:54 +00:00

crates/cashu/src/nuts/nut21/mod.rs

21

thesimplekid commented

2025-02-27 16:07:39 +00:00

(Migrated from github.com)

Require dleq proofs https://github.com/cashubtc/nuts/pull/222

- [x] Require dleq proofs https://github.com/cashubtc/nuts/pull/222

thesimplekid (Migrated from github.com) reviewed

2025-03-10 16:45:07 +00:00

crates/cdk/src/wallet/auth/auth_wallet.rs

					
				@ -0,0 +289,4 @@

				    }

				    /// Auth for request

				    #[instrument(skip(self))]

Depending on final wording of spec we should error here or allow proofs without a dleq proof.

ok300 (Migrated from github.com) reviewed

2025-03-13 16:43:16 +00:00

ok300 (Migrated from github.com) left a comment

Found a few NITs, but generally looks good.

crates/cashu/src/nuts/auth/nut22.rs

					
				@ -0,0 +1,369 @@

				//! 22 Blind Auth

        match self {
            Self::ClearAuth(cat) => cat.fmt(f),
            Self::BlindAuth(bat) => bat.fmt(f),
        }

Simpler way to write it.

```suggestion match self { Self::ClearAuth(cat) => cat.fmt(f), Self::BlindAuth(bat) => bat.fmt(f), } ``` Simpler way to write it.

        // Check prefix and extract the base64 encoded part in one step
        let encoded = s.strip_prefix("authA").ok_or(Error::InvalidPrefix)?;

```suggestion // Check prefix and extract the base64 encoded part in one step let encoded = s.strip_prefix("authA").ok_or(Error::InvalidPrefix)?; ```

crates/cashu/src/nuts/nut02.rs

    #[serde(default)]

#[serde(default)] falls back to Default, for which the value is 0 (equivalent to default_input_fee_ppk).

So I think this completely covers

the custom deserializer deserialize_input_fee_ppk
the function providing the default value default_input_fee_ppk

so they can both be removed too.

```suggestion #[serde(default)] ``` `#[serde(default)]` falls back to `Default`, for which the value is `0` (equivalent to `default_input_fee_ppk`). So I think this completely covers - the custom deserializer `deserialize_input_fee_ppk` - the function providing the default value `default_input_fee_ppk` so they can both be removed too.

crates/cdk-cli/src/sub_commands/cat_device_login.rs

					
				@ -0,0 +1,196 @@

				use std::path::Path;

        .expect("Nut21 defined")

```suggestion .expect("Nut21 defined") ```

crates/cdk-cli/src/sub_commands/cat_login.rs

					
				@ -0,0 +1,140 @@

				use std::path::Path;

        .expect("Nut21 defined")

```suggestion .expect("Nut21 defined") ```

crates/cdk-integration-tests/tests/fake_auth.rs

					
				@ -0,0 +1,866 @@

				use std::env;

Looks like some old unused test code.

```suggestion ``` Looks like some old unused test code.

crates/cdk/examples/auth_wallet.rs

					
				@ -0,0 +1,150 @@

				use std::sync::Arc;

        .expect("Nut21 defined")

```suggestion .expect("Nut21 defined") ```

findingsov (Migrated from github.com) reviewed

2025-03-13 21:06:16 +00:00

findingsov (Migrated from github.com) reviewed

2025-03-13 21:07:35 +00:00

thesimplekid (Migrated from github.com) reviewed

2025-03-14 09:23:03 +00:00

crates/cashu/src/nuts/nut02.rs

I thought this too but doing this fails the test on line 502, which is how nutshell responds with auth keysets.

ok300 (Migrated from github.com) reviewed

2025-03-14 11:41:02 +00:00

crates/cashu/src/nuts/nut02.rs

I see, you're right.

ok300 (Migrated from github.com) approved these changes

2025-03-14 11:41:38 +00:00

thesimplekid (Migrated from github.com) reviewed

2025-03-21 13:19:27 +00:00

crates/cdk-common/src/error.rs

					
				@ -510,0 +574,4 @@

				    /// Clear Auth Required

				    ClearAuthRequired,

				    /// Clear Auth Failed

				    ClearAuthFailed,

Use the correct error codes from spec

Sign in to join this conversation.

No reviewers

ok300

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!510

				`@ -0,0 +1,413 @@`
				`use cdk_common::{CurrencyUnit, MintKeySet};`

Rows
Columns