Introduce ProofsWithState for atomic proof state management #1488

2026-01-03T03:44:11Z

crodas commented

2026-01-03 03:44:11 +00:00

(Migrated from github.com)

Description

This PR depends on #1490

Replace individual proof state operations with a unified ProofsWithState type that enforces the invariant that all proofs in a set share the same state. This shifts responsibility for state consistency to the database layer and simplifies state transition logic in the saga implementations.

Changes to ProofsTransaction trait:

add_proofs now returns Acquired
update_proofs_states(ys, state) -> update_proofs(&mut Acquired)
get_proofs_states(ys) -> get_proofs(ys) returning Acquired

Benefits:

State transitions validated in memory before persisting
Eliminates scattered state checking in saga code
Database layer guarantees proof state consistency
Cleaner API with state encapsulated in the type

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 This PR depends on #1490 Replace individual proof state operations with a unified ProofsWithState type that enforces the invariant that all proofs in a set share the same state. This shifts responsibility for state consistency to the database layer and simplifies state transition logic in the saga implementations. Changes to ProofsTransaction trait: - add_proofs now returns Acquired<ProofsWithState> - update_proofs_states(ys, state) -> update_proofs(&mut Acquired<ProofsWithState>) - get_proofs_states(ys) -> get_proofs(ys) returning Acquired<ProofsWithState> Benefits: - State transitions validated in memory before persisting - Eliminates scattered state checking in saga code - Database layer guarantees proof state consistency - Cleaner API with state encapsulated in the type ----- ### 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

thesimplekid (Migrated from github.com) reviewed

2026-01-03 13:58:15 +00:00

crates/cdk-sql-common/src/mint/mod.rs

We should enforce that the number of got proofs is == the number requested. I think this is safer then forcing the application to check this.

crates/cdk/src/mint/swap/swap_saga/mod.rs

We should explicitly roll back the tx before returning the error. As is done on 214, other wise it leads to race conditions in tests.

crates/cdk/src/mint/swap/swap_saga/mod.rs

Can this check be removed now? Since we have the lock on the proofs from the add and then we check the state transition in the set_new_state?

Can this check be removed now? Since we have the lock on the proofs from the add and then we check the state transition in the `set_new_state`?

crates/cdk/src/mint/swap/swap_saga/mod.rs

					
				@ -463,2 +422,2 @@

				        )

				        .await?;

				        if let Err(err) = tx

				            .add_completed_operation(

I think we need to call compensate on the error here since we would expect the error here and not in the tx.update_proofs like we used to.

I think we need to call compensate on the error here since we would expect the error here and not in the `tx.update_proofs` like we used to.

Should have compensate here as well or input proofs are stuck in pending.

crodas (Migrated from github.com) reviewed

2026-01-03 14:31:05 +00:00

crates/cdk-sql-common/src/mint/mod.rs

Yes, I'm doing this right now.

crodas (Migrated from github.com) reviewed

2026-01-04 20:58:33 +00:00

crates/cdk-sql-common/src/mint/mod.rs

Fixed in e5f358df02af5a4d1f9938a8069e8cfd50c7b23a

crodas (Migrated from github.com) reviewed

2026-01-04 21:23:42 +00:00

crodas (Migrated from github.com) reviewed

2026-01-04 21:26:30 +00:00

crates/cdk/src/mint/swap/swap_saga/mod.rs

					
				@ -463,2 +422,2 @@

				        )

				        .await?;

				        if let Err(err) = tx

				            .add_completed_operation(

Fixed in 0d761ef

crodas (Migrated from github.com) reviewed

2026-01-04 21:26:39 +00:00

crates/cdk/src/mint/swap/swap_saga/mod.rs

Fixed in 0d761ef

crodas (Migrated from github.com) reviewed

2026-01-04 21:26:46 +00:00

crodas (Migrated from github.com) reviewed

2026-01-04 21:27:04 +00:00

crates/cdk/src/mint/swap/swap_saga/mod.rs

					
				@ -463,2 +422,2 @@

				        )

				        .await?;

				        if let Err(err) = tx

				            .add_completed_operation(

Fixed in 0d761ef

crodas (Migrated from github.com) reviewed

2026-01-04 21:27:19 +00:00

crates/cdk/src/mint/swap/swap_saga/mod.rs

Fixed in 0d761ef

crodas (Migrated from github.com) reviewed

2026-01-04 21:28:31 +00:00

thesimplekid (Migrated from github.com) reviewed

2026-01-05 16:51:37 +00:00

crates/cdk-common/src/mint.rs

					
				@ -31,6 +32,67 @@ pub enum OperationKind {

				    Melt,

				}

This introduces a third state update pattern to the codebase. Currently we have:

MeltQuote (DB-first): State is updated in-memory after writing to DB, ensuring the two are always in sync.
MintQuote (change-tracking): Uses take_changes() to track and persist events. This is justified because MintQuote state is computed from payments/issuances rather than explicitly set.
ProofsWithState (memory-first): Updates in-memory first via set_new_state(), then requires a separate update_proofs() call.

Since proofs follow a state machine (Unspent → Pending → Spent) just like MeltQuote, it makes sense to use the same pattern:

  // Current
  proofs.set_new_state(State::Pending)?;
  tx.update_proofs(&mut proofs).await?;

  // Suggested (matches MeltQuote pattern)
  tx.update_proofs_state(&mut proofs, State::Pending).await?;

This keeps persistence patterns consistent and eliminates the possibility of in-memory/DB state divergence if update_proofs() fails after set_new_state() succeeds.

This introduces a third state update pattern to the codebase. Currently we have: 1. MeltQuote (DB-first): State is updated in-memory after writing to [DB](https://github.com/cashubtc/cdk/blob/dd196a88048532796e3f35821e7cdfc31c5a329a/crates/cdk-sql-common/src/mint/quotes.rs#L927), ensuring the two are always in sync. 2. MintQuote (change-tracking): Uses take_changes() to track and persist events. This is justified because MintQuote state is computed from payments/issuances rather than explicitly set. 3. ProofsWithState (memory-first): Updates in-memory first via set_new_state(), then requires a separate update_proofs() call. Since proofs follow a state machine (Unspent → Pending → Spent) just like MeltQuote, it makes sense to use the same pattern: ```rust // Current proofs.set_new_state(State::Pending)?; tx.update_proofs(&mut proofs).await?; // Suggested (matches MeltQuote pattern) tx.update_proofs_state(&mut proofs, State::Pending).await?; ``` This keeps persistence patterns consistent and eliminates the possibility of in-memory/DB state divergence if update_proofs() fails after set_new_state() succeeds.

crates/cdk-sql-common/src/mint/proofs.rs

					
				@ -451,2 +516,4 @@

				        Ok((proofs, states.into_iter().map(Some).collect()))

				    }

This seems a bit weird setting this to unspent, should we just error on an empty query?

crodas (Migrated from github.com) reviewed

2026-01-07 15:11:35 +00:00

crates/cdk-common/src/mint.rs

					
				@ -31,6 +32,67 @@ pub enum OperationKind {

				    Melt,

				}

@thesimplekid What about the changes in d8bd4fa17ac06d03f7496e5532178bb6227a7b8a ?

thesimplekid (Migrated from github.com) reviewed

2026-01-07 17:40:31 +00:00

crates/cdk-common/src/mint.rs

					
				@ -31,6 +32,67 @@ pub enum OperationKind {

				    Melt,

				}

There is still a difference here with how we handle melt quotes vs proofs. Melt quotes we check the transition within sql common(github.com/cashubtc/cdk@44fee4c4d1/crates/cdk-sql-common/src/mint/quotes.rs (L898)). Where the proofs we call check transition in the mint before calling update.

There is still a difference here with how we handle melt quotes vs proofs. Melt quotes we check the transition within sql common(https://github.com/cashubtc/cdk/blob/44fee4c4d1903b3d0911014fbff3b78293c9d3ea/crates/cdk-sql-common/src/mint/quotes.rs#L898). Where the proofs we call check transition in the mint before calling update.

crodas (Migrated from github.com) reviewed

2026-01-07 17:45:55 +00:00

crates/cdk-common/src/mint.rs

					
				@ -31,6 +32,67 @@ pub enum OperationKind {

				    Melt,

				}

Yeah, this is duplicated and will be removed in another PR. I think the consensus was to move all checks off the storage layer. This was missed, I'll fix it later

thesimplekid (Migrated from github.com) reviewed

2026-01-07 20:27:09 +00:00

crates/cdk/src/mint/swap/swap_saga/mod.rs

					
				@ -453,2 +413,4 @@

				            self.compensate_all().await?;

				            return Err(err);

				        }

I think we can do better then calling check state and then update. If we have a set_state fn on the Acquired<ProofsWithState> that takes a db tx. It can check its a valid transition and update the memory state and db state in one fn. Same pattern could be applied to the MeltQuote, and a similar one even to MintQuote.

I think we can do better then calling check state and then update. If we have a `set_state` fn on the `Acquired<ProofsWithState>` that takes a db tx. It can check its a valid transition and update the memory state and db state in one fn. Same pattern could be applied to the MeltQuote, and a similar one even to MintQuote.

thesimplekid (Migrated from github.com) reviewed

2026-01-07 20:28:07 +00:00

crates/cdk-common/src/mint.rs

					
				@ -31,6 +32,67 @@ pub enum OperationKind {

				    Melt,

				}

this is duplicated

As far as I can tell we only call it there and not in the mint so its not a duplicate.

I think the consensus was to move all checks off the storage layer.

Yes but we need to make sure they are not missed and the storage and memory are updated together (see review comment).

> this is duplicated As far as I can tell we only call it there and not in the mint so its not a duplicate. > I think the consensus was to move all checks off the storage layer. Yes but we need to make sure they are not missed and the storage and memory are updated together (see review comment).

thesimplekid (Migrated from github.com) reviewed

2026-01-08 20:32:15 +00:00

crates/cdk/src/mint/melt/melt_saga/mod.rs

Can't we remove this check as its done in the Mint::update_proofs_state? We should make sure that method returns the correct error.

crates/cdk/src/mint/proofs.rs

					
				@ -0,0 +1,267 @@

				use cdk_common::database::{Acquired, DynMintTransaction};

We should return the correct error based on the current state.

Ex from what we did before

 if matches!(original_state, State::Pending | State::Spent) {
            tx.rollback().await?;
            return Err(if original_state == State::Pending {
                Error::TokenPending
            } else {
                Error::TokenAlreadySpent
            });
        }

We should return the correct error based on the current state. Ex from what we did before ```rust if matches!(original_state, State::Pending | State::Spent) { tx.rollback().await?; return Err(if original_state == State::Pending { Error::TokenPending } else { Error::TokenAlreadySpent }); } ```

crates/cdk/src/mint/proofs.rs

					
				@ -0,0 +40,4 @@

				#[cfg(test)]

				mod tests {

				    use cdk_common::mint::Operation;

Nice adding tests.

crates/cdk/src/mint/swap/swap_saga/mod.rs

					
				@ -177,54 +177,30 @@ impl<'a> SwapSaga<'a, Initial> {

				        );

Can't we remove this check as its done in the Mint::update_proofs_state? We should make sure that method returns the correct error.

Can't we remove this check as its done in the `Mint::update_proofs_state`? We should make sure that method returns the correct error.

crodas (Migrated from github.com) reviewed

2026-01-09 12:39:56 +00:00

crates/cdk/src/mint/melt/melt_saga/mod.rs

Fixed in 36b0dc6e2d

Fixed in 36b0dc6e2d9644b0ab3ff8a5b48b50036e86797d

crodas (Migrated from github.com) reviewed

2026-01-09 12:40:05 +00:00

crates/cdk/src/mint/proofs.rs

					
				@ -0,0 +1,267 @@

				use cdk_common::database::{Acquired, DynMintTransaction};

Fixed in 36b0dc6e2d

Fixed in 36b0dc6e2d9644b0ab3ff8a5b48b50036e86797d

crodas (Migrated from github.com) reviewed

2026-01-09 12:40:10 +00:00

crates/cdk/src/mint/swap/swap_saga/mod.rs

					
				@ -177,54 +177,30 @@ impl<'a> SwapSaga<'a, Initial> {

				        );

Fixed in 36b0dc6e2d

Fixed in 36b0dc6e2d9644b0ab3ff8a5b48b50036e86797d

thesimplekid (Migrated from github.com) approved these changes

2026-01-09 13:45:23 +00:00

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

				`@ -0,0 +1,267 @@`
				`use cdk_common::database::{Acquired, DynMintTransaction};`

Rows
Columns