Introduce ProofsWithState for atomic proof state management #1488
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
crodas
force-pushed
the
feature/mint-acquired-proofs
branch
2 times, most recently
from
6a59579 to
a22bdeb
Compare
thesimplekid
reviewed
Jan 3, 2026
crodas
force-pushed
the
feature/mint-acquired-proofs
branch
3 times, most recently
from
d1e8279 to
e5f358d
Compare
crodas
force-pushed
the
feature/mint-acquired-proofs
branch
2 times, most recently
from
9dca039 to
0d761ef
Compare
Split the large monolithic mod.rs file into separate files, each containing a database trait and its corresponding transaction trait: - proofs.rs: MintProofsTransaction + MintProofsDatabase - keys.rs: MintKeyDatabaseTransaction + MintKeysDatabase - quotes.rs: MintQuotesTransaction + MintQuotesDatabase - signatures.rs: MintSignatureTransaction + MintSignaturesDatabase - keyvalue.rs: KVStoreTransaction + KVStoreDatabase + KVStore - saga.rs: SagaTransaction + SagaDatabase - completed_operations.rs: CompletedOperationsTransaction + CompletedOperationsDatabase The mod.rs now only contains struct definitions, migrations, and the main MintDatabase/MintTransaction/DbTransactionFinalizer impls.
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
Add get_proofs_fails_when_some_not_found test that verifies the database returns an error when not all requested proofs exist, rather than silently returning a partial result. Fix get_proofs implementation to check that the number of rows returned matches the number of requested Y values, returning ProofNotFound if any proofs are missing.
crodas
force-pushed
the
feature/mint-acquired-proofs
branch
from
bbff771 to
e520c90
Compare
crodas
force-pushed
the
feature/mint-acquired-proofs
branch
from
e520c90 to
b07bede
Compare
thesimplekid
reviewed
Jan 5, 2026
Comment on lines
+353
to
+355
| if ys.is_empty() { | ||
| return Ok(ProofsWithState::new(vec![], State::Unspent).into()); | ||
| } |
Collaborator
There was a problem hiding this comment.
This seems a bit weird setting this to unspent, should we just error on an empty query?
Comment on lines
+100
to
+114
| /// Transitions all proofs to a new state. | ||
| /// | ||
| /// Validates that the state transition is allowed before applying it. | ||
| /// Returns the previous state on success. | ||
| /// | ||
| /// # Errors | ||
| /// | ||
| /// Returns [`Error::UnexpectedProofState`] if the transition from the current | ||
| /// state to the new state is not permitted. | ||
| pub fn set_new_state(&mut self, new_state: State) -> Result<State, Error> { | ||
| check_state_transition(self.state, new_state).map_err(|_| Error::UnexpectedProofState)?; | ||
| let old_state = self.state; | ||
| self.state = new_state; | ||
| Ok(old_state) | ||
| } |
Collaborator
There was a problem hiding this comment.
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
Benefits:
Notes to the reviewers
Suggested CHANGELOG Updates
CHANGED
ADDED
REMOVED
FIXED
Checklist
just final-checkbefore committing