Add ArcSwap to update Mint configuration at runtime #503

2024-12-16T19:16:54Z

crodas commented

2024-12-16 19:16:54 +00:00

(Migrated from github.com)

Description

The main goal is to change settings without having multiple RwLock everywhere, instead having ArcSwap to update the configuration without having access to a mutable reference to the Mint.

This will allow the RPC Server, or any other medium to update the Mint without minimum contention.

This is a pre-requisite for #476

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 The main goal is to change settings without having multiple RwLock everywhere, instead having ArcSwap to update the configuration without having access to a mutable reference to the Mint. This will allow the RPC Server, or any other medium to update the Mint without minimum contention. This is a pre-requisite for #476 ----- ### 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

davidcaseria (Migrated from github.com) reviewed

2024-12-17 00:01:15 +00:00

crates/cdk/src/mint/config.rs

					
				@ -0,0 +1,128 @@

				//! Active mint configuration

Does this need to be pub?

Does this need to be `pub`?

crodas (Migrated from github.com) reviewed

2024-12-17 01:23:25 +00:00

crates/cdk/src/mint/config.rs

					
				@ -0,0 +1,128 @@

				//! Active mint configuration

The fact that it is accessed through get_config() (that I think I should rename to load()), I think it should be pub, at least pub(crate).

I'll think if that can be transparent, with something like Deref or something similar.

The fact that it is accessed through `get_config()` (that I think I should rename to `load()`), I think it should be pub, at least pub(crate). I'll think if that can be transparent, with something like Deref or something similar.

crodas (Migrated from github.com) reviewed

2024-12-17 04:34:21 +00:00

crates/cdk/src/mint/config.rs

					
				@ -0,0 +1,128 @@

				//! Active mint configuration

I think these new names are better (9272347), what do you think?

davidcaseria (Migrated from github.com) reviewed

2024-12-17 10:57:00 +00:00

crates/cdk/src/mint/config.rs

					
				@ -0,0 +1,128 @@

				//! Active mint configuration

That’s better. 👍

thesimplekid (Migrated from github.com) reviewed

2024-12-17 18:26:46 +00:00

crates/cdk/Cargo.toml

deps under the -Z minimal-version comment are for deps that are not a direct dep of ours, but whatever dependency of ours that pull them in does not properly define their minimal version. Since this is a direct dep of ours can we just move this above the comment.

crates/cdk/src/mint/mod.rs

					
				@ -40,20 +42,14 @@ pub use types::{MeltQuote, MintQuote};

				/// Cashu Mint

Think the quote_ttl should also be in the swapable config?

vnprc (Migrated from github.com) reviewed

2024-12-17 21:40:06 +00:00

crates/cdk/src/mint/config.rs

					
				@ -0,0 +25,4 @@

				/// Mint configuration

				///

				/// This struct is used to configure the mint, and it is wrapped inside a ArcSwap, so it can be

				/// updated at runtime without locking the shared config nor without requiriming a mutable reference

/// updated at runtime without locking the shared config nor without requiring a mutable reference

```suggestion /// updated at runtime without locking the shared config nor without requiring a mutable reference ```

crodas commented

2024-12-24 00:55:05 +00:00

(Migrated from github.com)

@thesimplekid this PR is ready for another round of reviews.

thesimplekid (Migrated from github.com) approved these changes

2024-12-25 14:53:21 +00:00

thesimplekid (Migrated from github.com) left a comment

ACK 712fa5176d

ACK 712fa5176dbe443ae89cd3ca7e59091cca32f157

Sign in to join this conversation.

No reviewers

thesimplekid

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

				`@ -40,20 +42,14 @@ pub use types::{MeltQuote, MintQuote};`
				`/// Cashu Mint`

Rows
Columns