cashubtc/cdk
1
0
Fork 0
Code Issues 25 Pull requests 23 Projects Releases 43 Packages Wiki Activity Actions

WIP: feat: add wallet trait #1581

Draft
crodas wants to merge 1 commit from crodas/feature/wallet-trait into main
pull from: crodas/feature/wallet-trait
merge into: cashubtc:main
Conversation 6 Commits 1 Files changed 55 +1503 -394
crodas commented 2026-01-28 03:28:54 +00:00 (Migrated from github.com)
Copy link

Description

Introduce a modular trait system for wallet implementations that enables flexible composition of wallet capabilities. This allows different wallet implementations to share a common interface while supporting selective trait implementation.

The trait hierarchy consists of:

  • WalletTypes: Base trait defining associated types (Amount, Proofs, etc.)
  • WalletBalance: Balance query operations
  • WalletMintInfo: Mint information and keyset operations
  • WalletMint: Minting operations (payment -> tokens)
  • WalletMelt: Melting operations (tokens -> payment)
  • WalletReceive: Token receiving
  • WalletProofs: Proof state management
  • Wallet: Super-trait combining all capabilities with blanket impl

Implements all traits for both cdk::Wallet and cdk-ffi::Wallet, enabling generic code that works with any wallet implementation.

Notes to the reviewers

Suggested CHANGELOG Updates

CHANGED

ADDED

REMOVED

FIXED

Checklist

### Description Introduce a modular trait system for wallet implementations that enables flexible composition of wallet capabilities. This allows different wallet implementations to share a common interface while supporting selective trait implementation. The trait hierarchy consists of: - WalletTypes: Base trait defining associated types (Amount, Proofs, etc.) - WalletBalance: Balance query operations - WalletMintInfo: Mint information and keyset operations - WalletMint: Minting operations (payment -> tokens) - WalletMelt: Melting operations (tokens -> payment) - WalletReceive: Token receiving - WalletProofs: Proof state management - Wallet: Super-trait combining all capabilities with blanket impl Implements all traits for both cdk::Wallet and cdk-ffi::Wallet, enabling generic code that works with any wallet implementation. ----- ### Notes to the reviewers <!-- In this section you can include notes directed to the reviewers, like explaining why some parts of the PR were done in a specific way --> ----- ### Suggested [CHANGELOG](https://github.com/cashubtc/cdk/blob/main/CHANGELOG.md) Updates <!-- Please do not edit the actual changelog but note what you changed here. --> #### CHANGED #### ADDED #### REMOVED #### FIXED ---- ### Checklist * [ ] I followed the [code style guidelines](https://github.com/cashubtc/cdk/blob/main/CODE_STYLE.md) * [ ] I ran `just final-check` before committing
🎉 1
davidcaseria (Migrated from github.com) reviewed 2026-01-28 03:28:54 +00:00
thesimplekid (Migrated from github.com) reviewed 2026-01-28 17:42:17 +00:00
crates/cdk/src/wallet/traits.rs
thesimplekid (Migrated from github.com) commented 2026-01-28 17:42:17 +00:00
Copy link

Do we see a use case where we would want multiple traits to implement only part of the api? I don't think I see one and one trait would be simpler.

Do we see a use case where we would want multiple traits to implement only part of the api? I don't think I see one and one trait would be simpler.
crodas (Migrated from github.com) reviewed 2026-01-30 06:22:01 +00:00
crates/cdk/src/wallet/traits.rs
crodas (Migrated from github.com) commented 2026-01-30 06:22:01 +00:00
Copy link

Is it any better at 1a2fd1fcd960696fd5fb3beb0ac60921b9d4bf32?

Is it any better at 1a2fd1fcd960696fd5fb3beb0ac60921b9d4bf32?
thesimplekid (Migrated from github.com) reviewed 2026-01-30 11:41:04 +00:00
crates/cdk/src/wallet/traits.rs
thesimplekid (Migrated from github.com) commented 2026-01-30 11:41:04 +00:00
Copy link

i don't see how this change addresses the comment of 1 wallet trait vs the multiple it still has multiple. To be clear I not strongly against having multiple traits if there is a reason for it, just one trait would be simpler but if others see a use case for the multiple I'm okay with that.

i don't see how this change addresses the comment of 1 wallet trait vs the multiple it still has multiple. To be clear I not strongly against having multiple traits if there is a reason for it, just one trait would be simpler but if others see a use case for the multiple I'm okay with that.
thesimplekid commented 2026-02-24 12:24:00 +00:00 (Migrated from github.com)
Copy link

@crodas Now that we have released 0.15 I think we should turn our attention back to this and get it merged. Looks like it needs a rebase is there anything else that is outstanding with it?

cc @ye0man

@crodas Now that we have released 0.15 I think we should turn our attention back to this and get it merged. Looks like it needs a rebase is there anything else that is outstanding with it? cc @ye0man
crodas commented 2026-02-24 12:40:42 +00:00 (Migrated from github.com)
Copy link

Rebasing, I am on it now.

Rebasing, I am on it now.
This pull request is marked as a work in progress.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin crodas/feature/wallet-trait:crodas/feature/wallet-trait
git switch crodas/feature/wallet-trait

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff crodas/feature/wallet-trait
git switch crodas/feature/wallet-trait
git rebase main
git switch main
git merge --ff-only crodas/feature/wallet-trait
git switch crodas/feature/wallet-trait
git rebase main
git switch main
git merge --no-ff crodas/feature/wallet-trait
git switch main
git merge --squash crodas/feature/wallet-trait
git switch main
git merge --ff-only crodas/feature/wallet-trait
git switch main
git merge crodas/feature/wallet-trait
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
davidcaseria
Labels
Clear labels   
DB & Storage

   
Deployment

   
Error Handling & Logging

   
Maintenance

   
Payment Backend

   
backport

   
backport v0.13.x

   
backport v0.14.x

   
backport v0.15.x

   
bindings

   
blocked

   
bug

Something isn't working

  
cdk-sql

   
ci

   
cli

   
deps

   
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
enhancement

New feature or request

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
keep-open

   
ldk-node-ui

   
migrations

   
mint

   
mutation-testing

   
needs rebase

   
needs review

   
new nut

This PR is held pending the merge of a new or updated NUT

  
nut change

   
question

Further information is requested

  
ready

   
rust-version

   
rustfmt

   
stacked hold

PR is based on top of another PR, so should be merged in sequence. See comments for details.

  
stale

   
testing

   
wallet

   
weekly-report

   
wontfix

This will not be worked on

No labels
DB & Storage
Deployment
Error Handling & Logging
Maintenance
Payment Backend
backport
backport v0.13.x
backport v0.14.x
backport v0.15.x
bindings
blocked
bug
cdk-sql
ci
cli
deps
documentation
duplicate
enhancement
good first issue
help wanted
invalid
keep-open
ldk-node-ui
migrations
mint
mutation-testing
needs rebase
needs review
new nut
nut change
question
ready
rust-version
rustfmt
stacked hold
stale
testing
wallet
weekly-report
wontfix
Milestone
Clear milestone
No items
No milestone
0.16.0
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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!1581
No description provided.