Signatory: share keysets across instances via periodic reload by crodas · Pull Request #2273 · cashubtc/cdk

crodas · 2026-07-29T00:52:23Z

Description

This depends on #2270

Let multiple signatories (or embedded mints) run active/active on one keys database without putting it on the signing path. The derivation index is allocated in the database under a per-unit lock and a version token gates reloads, so peers stay consistent; keys are served from an ArcSwap so a reload never blocks signing. The reload is opt-in, off by default.

See docs/adr/0004-signatory-multi-instance-sharing.md.

Notes to the reviewers

Suggested CHANGELOG Updates

CHANGED

ADDED

REMOVED

FIXED

Checklist

I followed the code style guidelines
I ran just quick-check before committing
If the Wallet API was modified (added/removed/changed), I have reflected those changes in the FFI bindings (crates/cdk-ffi)

crodas · 2026-07-29T00:52:35Z

@cdk-bot review

codecov · 2026-07-29T12:21:25Z

Codecov Report

❌ Patch coverage is 97.45917% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.82%. Comparing base (c051ff9) to head (cc02d2d).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/cdk-signatory/src/db_signatory.rs	98.40%	6 Missing ⚠️
crates/cdk/src/mint/builder.rs	73.33%	4 Missing ⚠️
crates/cdk-signatory/src/common.rs	93.10%	2 Missing ⚠️
crates/cdk-sql-common/src/mint/keys.rs	96.92%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2273      +/-   ##
==========================================
+ Coverage   75.64%   75.82%   +0.17%     
==========================================
  Files         369      369              
  Lines       89142    89594     +452     
==========================================
+ Hits        67431    67931     +500     
+ Misses      21711    21663      -48

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

thesimplekid · 2026-07-30T11:30:56Z

Things for last pr that should be resolved here:

thesimplekid · 2026-07-30T11:47:36Z

@crodas if you could rebase now that #2270 is merged.

thesimplekid · 2026-07-30T15:38:55Z

@cdk-bot review

cdk-bot

Verified findings approved for disclosure:

Sub-millisecond refresh interval panics the refresh task with division by zero (low) - see inline comment
rotate_keyset now returns Err after the rotation has already committed, leaving memory stale with no self-heal in the default config (low) - see inline comment
Unanchored locations included in summary:
- crates/cdk-signatory/src/db_signatory.rs:452
next_derivation_index ignores NULL derivation_path_index rows, so legacy keysets can cause index reuse and a silent non-rotation (low) - see inline comment

crodas · 2026-07-30T17:50:01Z

@cdk-bot review

crodas · 2026-07-31T13:26:30Z

@cdk-bot review

cdk-bot

Verified findings approved for disclosure:

Boot can deadlock across instances: init_keysets takes per-unit advisory locks in nondeterministic HashMap order (medium) - see inline comment
Additional locations included in summary:
- crates/cdk-sql-common/src/mint/keys.rs:197

crodas · 2026-07-31T14:12:28Z

@cdk-bot review

thesimplekid

One comment otherwise I think this is good

crodas · 2026-08-03T20:33:58Z

@cdk-bot review

crodas · 2026-08-04T14:09:27Z

@cdk-bot review

cdk-bot

Verified findings approved for disclosure:

ADR-0004 references non-existent KeysDatabase::keysets_version / keyset_version table; code implements keysets_epoch / keyset_epoch (low) - see inline comment

crodas · 2026-08-05T23:15:34Z

@cdk-bot review

cdk-bot

Verified findings approved for disclosure:

Boot-time reactivation unconditionally bumps keyset_epoch even when nothing changed, causing unnecessary fleet-wide reloads (low) - Every boot of every instance in steady state bumps the persisted keyset_epoch via the unconditional add_keyset_info/set_active_keyset writes in init_keysets. Refresh-enabled peers observe the epoch mismatch and perform a full reload + snapshot republish even though no keyset was added or (de)activated. In a fleet with frequent restarts, each boot fans out redundant reloads across all peers.
Unanchored locations included in summary:
- crates/cdk-sql-common/src/mint/keys.rs:110

crodas · 2026-08-10T17:42:27Z

@cdk-bot review

cdk-bot

Verified findings approved for disclosure:

Cargo.lock.msrv not regenerated for new dependency edges — MSRV CI build will fail (high) - see inline comment
Additional locations included in summary:
- crates/cdk-integration-tests/Cargo.toml:72

crodas · 2026-08-10T18:21:54Z

@cdk-bot review

crodas · 2026-08-10T19:11:30Z

@thesimplekid I simplified a lot, and all reading and writing from the database happens in a transaction, serialized by the database. That is OK to simplify complexity since all operations are happening off-memory and waiting for a database

crodas · 2026-08-10T22:40:48Z

@cdk-bot review

Coordinate keyset reads and rotations across processes with a global database lock and persisted epoch. Allocate derivation indexes transactionally and reload immutable in-memory snapshots on an opt-in interval. Add SQL migrations, builder and CLI configuration, concurrency regression coverage, and an ADR for the active/active design. BREAKING CHANGE: MintKeysDatabase removes autocommit keyset reads and requires epoch and transaction-scoped keyset operations.

Run the cross-instance rotation integration test in the Postgres CI job so it cannot silently pass through its missing-database guard. Update ADR wording to match the final refresh API and database-backed rotation behavior.

crodas requested a review from thesimplekid July 29, 2026 00:52

crodas self-assigned this Jul 29, 2026

github-project-automation Bot added this to CDK Jul 29, 2026

github-project-automation Bot moved this to Backlog in CDK Jul 29, 2026

crodas force-pushed the feat/share-signatories branch 2 times, most recently from 31499c1 to 6abbf4b Compare July 29, 2026 11:43

crodas mentioned this pull request Jul 29, 2026

Signatory: serve keys from memory #2270

Merged

3 tasks

crodas force-pushed the feat/share-signatories branch from 6abbf4b to f81c207 Compare July 30, 2026 11:57

cdk-bot reviewed Jul 30, 2026

View reviewed changes

Comment thread crates/cdk-signatory/src/db_signatory.rs

Comment thread crates/cdk-sql-common/src/mint/keys.rs Outdated

thesimplekid reviewed Jul 30, 2026

View reviewed changes

Comment thread crates/cdk-signatory/Cargo.toml Outdated

Comment thread crates/cdk-signatory/src/db_signatory.rs Outdated

Comment thread crates/cdk-signatory/src/db_signatory.rs

Comment thread crates/cdk-signatory/src/db_signatory.rs

Comment thread crates/cdk/src/mint/builder.rs Outdated

cdk-bot reviewed Jul 31, 2026

View reviewed changes

Comment thread crates/cdk-signatory/src/common.rs Outdated

crodas force-pushed the feat/share-signatories branch from ff2aff2 to afe25c0 Compare July 31, 2026 15:06

crodas requested a review from thesimplekid July 31, 2026 16:37

thesimplekid reviewed Aug 3, 2026

View reviewed changes

Comment thread crates/cdk-sql-common/src/mint/migrations/postgres/20260728000000_add_keyset_version.sql Outdated

crodas force-pushed the feat/share-signatories branch from afe25c0 to 19adae0 Compare August 3, 2026 16:35

cdk-bot reviewed Aug 4, 2026

View reviewed changes

Comment thread docs/adr/0004-signatory-multi-instance-sharing.md Outdated

crodas force-pushed the feat/share-signatories branch from 19adae0 to bac4a9b Compare August 5, 2026 23:15

cdk-bot reviewed Aug 5, 2026

View reviewed changes

Comment thread crates/cdk-signatory/src/common.rs

thesimplekid added this to the 0.17.4 milestone Aug 10, 2026

thesimplekid modified the milestones: 0.17.4, 0.18.0 Aug 10, 2026

crodas mentioned this pull request Aug 10, 2026

Move refresh keyset to the config #2309

Open

thesimplekid reviewed Aug 10, 2026

View reviewed changes

Comment thread crates/cdk-signatory/src/db_signatory.rs

thesimplekid reviewed Aug 10, 2026

View reviewed changes

Comment thread crates/cdk-signatory/src/db_signatory.rs

crodas force-pushed the feat/share-signatories branch from d85e1ef to 52b0552 Compare August 10, 2026 17:39

cdk-bot reviewed Aug 10, 2026

View reviewed changes

Comment thread crates/cdk-signatory/Cargo.toml

crodas requested a review from thesimplekid August 10, 2026 19:12

crodas force-pushed the feat/share-signatories branch 2 times, most recently from 1f2d76e to aac24d8 Compare August 10, 2026 22:40

thesimplekid force-pushed the feat/share-signatories branch 2 times, most recently from 05cab90 to 4c97d56 Compare August 11, 2026 09:08

thesimplekid approved these changes Aug 11, 2026

View reviewed changes

crodas and others added 2 commits August 11, 2026 11:45

ci(signatory): test shared-keyset rotation on Postgres

cc02d2d

Run the cross-instance rotation integration test in the Postgres CI job so it cannot silently pass through its missing-database guard. Update ADR wording to match the final refresh API and database-backed rotation behavior.

thesimplekid force-pushed the feat/share-signatories branch from 4c97d56 to cc02d2d Compare August 11, 2026 11:46

thesimplekid merged commit cc02d2d into cashubtc:main Aug 11, 2026
4 checks passed

github-project-automation Bot moved this from Backlog to Done in CDK Aug 11, 2026

crodas mentioned this pull request Aug 12, 2026

Add advisory locking mechanism in the mintdb trait #2332

Closed

Uh oh!

Conversation

crodas commented Jul 29, 2026

Description

Notes to the reviewers

Suggested CHANGELOG Updates

CHANGED

ADDED

REMOVED

FIXED

Checklist

Uh oh!

crodas commented Jul 29, 2026

Uh oh!

codecov Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

thesimplekid commented Jul 30, 2026

Uh oh!

thesimplekid commented Jul 30, 2026

Uh oh!

thesimplekid commented Jul 30, 2026

Uh oh!

cdk-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

crodas commented Jul 30, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

crodas commented Jul 31, 2026

Uh oh!

cdk-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

crodas commented Jul 31, 2026

Uh oh!

thesimplekid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

crodas commented Aug 3, 2026

Uh oh!

crodas commented Aug 4, 2026

Uh oh!

cdk-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

crodas commented Aug 5, 2026

Uh oh!

cdk-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

crodas commented Aug 10, 2026

Uh oh!

cdk-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

crodas commented Aug 10, 2026

Uh oh!

crodas commented Aug 10, 2026

Uh oh!

crodas commented Aug 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

codecov Bot commented Jul 29, 2026 •

edited

Loading