Simplify keyset management to three core functions by crodas · Pull Request #2096 · cashubtc/cdk

crodas · 2026-06-13T19:33:39Z

Description

Replace 6+ overlapping keyset functions with a clean, minimal API:

keysets(): Returns Vec<KeySet> (full keysets with keys included). In automatic mode, refreshes from the mint with a graceful fallback to cached data on network failure. In manual mode (set_keysets(Some(...))), returns injected keysets directly without network access.
active_keyset(): Returns the active KeySet with the lowest input fee.
keyset(id): Returns a single KeySet by ID.
set_keysets(Option<Vec<KeySet>>): Enables manual keyset mode, bypassing the
metadata cache entirely. Pass None to return to automatic mode.

By returning full KeySet objects (which include .keys), the separate load_keyset_keys(id) function is no longer needed; callers access .keys directly from the keyset.

Removed from Wallet and WalletTrait:

load_keyset_keys()
refresh_keysets()
fetch_active_keyset()
get_active_keyset()
get_mint_keysets(KeysetFilter)
load_mint_keysets()

Also moves metadata_cache_ttl into MintMetadataCache, removing boilerplate TTL passing on every cache load call.

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)

codecov · 2026-06-13T20:56:56Z

Codecov Report

❌ Patch coverage is 87.66816% with 55 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.67%. Comparing base (41504a2) to head (e1a8854).

Files with missing lines	Patch %	Lines
crates/cdk/src/wallet/test_utils.rs	40.74%	16 Missing ⚠️
crates/cdk-ffi/src/wallet.rs	0.00%	13 Missing ⚠️
crates/cdk-ffi/src/types/wallet.rs	0.00%	12 Missing ⚠️
crates/cdk/src/wallet/mint_metadata_cache.rs	85.29%	5 Missing ⚠️
crates/cdk-ffi/src/wallet_trait.rs	0.00%	3 Missing ⚠️
crates/cdk/src/wallet/builder.rs	60.00%	2 Missing ⚠️
crates/cdk/src/wallet/issue/saga/resume.rs	0.00%	2 Missing ⚠️
crates/cdk-integration-tests/src/lib.rs	0.00%	1 Missing ⚠️
crates/cdk/src/wallet/melt/mod.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2096      +/-   ##
==========================================
+ Coverage   71.59%   71.67%   +0.07%     
==========================================
  Files         356      356              
  Lines       74106    74361     +255     
==========================================
+ Hits        53059    53301     +242     
- Misses      21047    21060      +13

☔ 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-06-16T11:46:52Z

Description

Replace 6+ overlapping keyset functions with a clean, minimal API:

* `keysets()`: Returns `Vec<KeySet>` (full keysets with keys included).  In automatic mode, refreshes from the mint with a graceful fallback to cached data on network failure. In manual mode (`set_keysets(Some(...))`), returns injected keysets directly without network access.

* `active_keyset()`: Returns the active `KeySet` with the lowest input fee.

* `keyset(id)`: Returns a single `KeySet` by ID.

* `set_keysets(Option<Vec<KeySet>>)`: Enables manual keyset mode, bypassing the
  metadata cache entirely. Pass `None` to return to automatic mode.

By returning full KeySet objects (which include .keys), the separate load_keyset_keys(id) function is no longer needed; callers access .keys directly from the keyset.

Removed from Wallet and WalletTrait:

* load_keyset_keys()

* refresh_keysets()

* fetch_active_keyset()

* get_active_keyset()

* get_mint_keysets(KeysetFilter)

* load_mint_keysets()

I think I would change keysets() to accept an online parameter this way for the SendOffline we can make sure it does not send a request to the mint. I would also make keysets() respect the ttl so if the data it has is newer then ttl it does not send a network request, this would address the hot path issue.

I would also keep refresh_keysets() this would refresh the keysets regardless of cache ttl and could be used by wallet at start or via some action where the wallet wants to force a refresh but cdk would not use it internally we would use the keysets().

What is the use case of the set_keysets() I am not sure why a wallet would want to set this manually?

thesimplekid · 2026-06-16T11:53:49Z

Or better we could avoid the refresh_keysets fn by making the keysets() accept an enum:

enum KeysetLoadPolicy {
      CacheOnly,
      CacheThenNetwork,
      Refresh,
  }

With the default CacheThenNetwork, Cache used for the offline send. Cache here also includes the wallets local DB.

thesimplekid

Can we add a test for how a wallet handle a receive that includes proofs from inactive keysets? I have seen issues with this and hope this PR fixes it or at least does not regress after #2114

thesimplekid · 2026-06-19T08:07:31Z

+            KeysetLoadPolicy::CacheThenNetwork => {
+                self.metadata_cache
+                    .load(&self.localstore, &self.client)
+                    .await?


Replace 6+ overlapping keyset functions with a clean, minimal API: - `keysets()`: Returns `Vec<KeySet>` (full keysets with keys included). In automatic mode, refreshes from the mint with graceful fallback to cached data on network failure. In manual mode (`set_keysets(Some(...))`), returns injected keysets directly without network access. - `active_keyset()`: Returns the active `KeySet` with the lowest input fee. - `keyset(id)`: Returns a single `KeySet` by ID. - `set_keysets(Option<Vec<KeySet>>)`: Enables manual keyset mode, bypassing the metadata cache entirely. Pass `None` to return to automatic mode. By returning full `KeySet` objects (which include `.keys`), the separate `load_keyset_keys(id)` function is no longer needed — callers access `.keys` directly from the keyset. Removed from Wallet and WalletTrait: - load_keyset_keys() - refresh_keysets() - fetch_active_keyset() - get_active_keyset() - get_mint_keysets(KeysetFilter) - load_mint_keysets() Also moves `metadata_cache_ttl` into `MintMetadataCache`, removing boilerplate TTL passing on every cache load call.

Introduces CacheOnly, CacheThenNetwork (default), and Refresh variants so callers can control whether keysets are loaded from cache, cache+network, or always refreshed. Adds load_cached() to MintMetadataCache for offline cache+DB access. Updates trait, FFI, and all callers.

When a mint rotates keysets, the wallet's cached active keyset becomes stale. Operations that build outputs with the old keyset get rejected with InactiveKeyset. Add retry_on_inactive_keyset helper that refreshes the keyset cache and retries once, applied to mint, swap, and receive.

Eliminate the dual-mode keyset code path (manual vs cache-based). Tests now go through the real metadata cache via MockMintConnector, which is upgraded to support multiple keysets.

Verify the full receive preparation path works when a token contains proofs from an inactive keyset: token decoding, keyset lookup, fee calculation, and active keyset selection all succeed.

Only retry when the active keyset actually changes after refresh. If unchanged, return InactiveKeyset immediately. Add tests for retry-on-change, no-retry-when-unchanged, and no-retry-on-success.

Offline sends (OfflineExact/OfflineTolerance) now use KeysetLoadPolicy::CacheOnly so they never hit the network. Load all keysets once and derive fees, active IDs locally. Add tests for offline-without-cache, offline-with-cache, and online-from-network scenarios.

When the in-memory cache TTL expired, load() would fall back to the DB and return stale data instead of fetching fresh data from the mint. Fixed by only using the DB fallback when the cache is empty (cold start), not when it's stale. Also updated token_proofs doc comment.

load_from_db was setting updated_at to Instant::now(), making the TTL think DB-loaded data was freshly fetched. This delayed mint refreshes after cold starts. Now updated_at keeps its default value so the next load() correctly treats DB data as stale and fetches from the mint.

crodas self-assigned this Jun 13, 2026

github-project-automation Bot added this to CDK Jun 13, 2026

github-project-automation Bot moved this to Backlog in CDK Jun 13, 2026

crodas force-pushed the feature/simpler-wallet-key-management branch 3 times, most recently from 1d288d7 to a7ee108 Compare June 13, 2026 20:25

crodas force-pushed the feature/simpler-wallet-key-management branch 5 times, most recently from 918427f to ea9d710 Compare June 14, 2026 22:26

thesimplekid added this to the 0.18.0 milestone Jun 16, 2026

thesimplekid reviewed Jun 16, 2026

View reviewed changes

Comment thread crates/cdk/src/wallet/send/saga/mod.rs Outdated

Comment thread crates/cdk/src/wallet/keysets.rs Outdated

Comment thread crates/cdk/src/wallet/melt/mod.rs Outdated

crodas force-pushed the feature/simpler-wallet-key-management branch 2 times, most recently from c4b2401 to 55dd96c Compare June 18, 2026 21:22

thesimplekid requested changes Jun 19, 2026

View reviewed changes

github-project-automation Bot moved this from Backlog to In progress in CDK Jun 19, 2026

crodas added 4 commits June 19, 2026 18:53

Remove set_keysets and manual_keysets from Wallet

da381eb

Eliminate the dual-mode keyset code path (manual vs cache-based). Tests now go through the real metadata cache via MockMintConnector, which is upgraded to support multiple keysets.

crodas force-pushed the feature/simpler-wallet-key-management branch from 55dd96c to d6796c2 Compare June 19, 2026 21:53

crodas added 5 commits June 19, 2026 19:08

Add test for receive with inactive keyset proofs

c1dab27

Verify the full receive preparation path works when a token contains proofs from an inactive keyset: token decoding, keyset lookup, fee calculation, and active keyset selection all succeed.

Check for keyset rotation before retrying on InactiveKeyset

9b87dc5

Only retry when the active keyset actually changes after refresh. If unchanged, return InactiveKeyset immediately. Add tests for retry-on-change, no-retry-when-unchanged, and no-retry-on-success.

crodas force-pushed the feature/simpler-wallet-key-management branch from d6796c2 to e1a8854 Compare June 20, 2026 02:09

crodas marked this pull request as ready for review June 23, 2026 00:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify keyset management to three core functions#2096

Simplify keyset management to three core functions#2096
crodas wants to merge 9 commits into
cashubtc:mainfrom
crodas:feature/simpler-wallet-key-management

crodas commented Jun 13, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Jun 13, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thesimplekid commented Jun 16, 2026

Description

Uh oh!

thesimplekid commented Jun 16, 2026

Uh oh!

thesimplekid left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thesimplekid Jun 19, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

crodas commented Jun 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Notes to the reviewers

Suggested CHANGELOG Updates

CHANGED

ADDED

REMOVED

FIXED

Checklist

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thesimplekid commented Jun 16, 2026

Description

Uh oh!

thesimplekid commented Jun 16, 2026

Uh oh!

thesimplekid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thesimplekid Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

crodas commented Jun 13, 2026 •

edited

Loading

codecov Bot commented Jun 13, 2026 •

edited

Loading