Conversation
497aa6d to
85ffb8b
Compare
1a2fd1f to
b535f81
Compare
b535f81 to
84528dd
Compare
60280ad to
ee94ebb
Compare
|
Rebasing, I am on it now. |
ee94ebb to
d30cce9
Compare
8f12b14 to
b160bcc
Compare
0ac5cdb to
63705bd
Compare
Define the WalletTrait in cdk-common with default method implementations for mint, melt, send, receive, and balance operations across Bolt11, Bolt12, and custom payment methods. Implement the trait for the concrete Wallet type in cdk, moving public API methods from inherent impls into the trait so that consumers can program against the trait abstraction. Add cdk-ffi/src/wallet_trait.rs to expose WalletTrait methods through the FFI layer, and update all CLI commands, examples, and integration tests to import WalletTrait where needed.
…unused imports Refactor the WalletTrait implementation so each method delegates to an inherent method on Wallet rather than containing inline logic. This makes the trait a thin forwarding layer for the compiler contract while the real implementations live as regular methods on Wallet. Remove the now-unnecessary `use WalletTrait` import from 42 caller files across CLI, FFI, examples, and integration tests, since callers invoke inherent methods directly. Also remove section header comments (// === ... ===) added during the refactor.
63705bd to
660cc55
Compare
thesimplekid
left a comment
There was a problem hiding this comment.
I think this is pretty close just rename and few things to make it more clear that the Trait is more for internal use and not the primary API for the wallet and the Wallet struct should still be used directly, we should also add doc comments to this effect.
| }; | ||
| pub use builder::WalletBuilder; | ||
| pub use cdk_common::wallet as types; | ||
| pub use cdk_common::wallet::{ReceiveOptions, SendMemo, SendOptions, Wallet as WalletTrait}; |
There was a problem hiding this comment.
| pub use cdk_common::wallet::{ReceiveOptions, SendMemo, SendOptions, Wallet as WalletTrait}; | |
| pub use cdk_common::wallet::{ReceiveOptions, SendMemo, SendOptions}; |
I don't think we should re-export and promote the trait to be used publicity. The trait should be used internally by use to keep the ffi inline but public use should not be encouraged.
| /// Data for a prepared send operation (owned, no lifetime) | ||
| #[derive(Debug, Clone)] | ||
| pub struct PreparedSendData { | ||
| /// Operation ID | ||
| pub operation_id: Uuid, | ||
| /// Amount to send | ||
| pub amount: Amount, | ||
| /// Send options | ||
| pub options: SendOptions, | ||
| /// Proofs that need swapping | ||
| pub proofs_to_swap: Proofs, | ||
| /// Proofs to send directly | ||
| pub proofs_to_send: Proofs, | ||
| /// Fee for swap | ||
| pub swap_fee: Amount, | ||
| /// Fee for send | ||
| pub send_fee: Amount, | ||
| } | ||
|
|
||
| /// Data for a prepared melt operation (owned, no lifetime) | ||
| #[derive(Debug, Clone)] | ||
| pub struct PreparedMeltData { | ||
| /// Operation ID | ||
| pub operation_id: Uuid, | ||
| /// Melt quote | ||
| pub quote: MeltQuote, | ||
| /// Proofs to melt | ||
| pub proofs: Proofs, | ||
| /// Proofs that need swapping | ||
| pub proofs_to_swap: Proofs, | ||
| /// Swap fee | ||
| pub swap_fee: Amount, | ||
| /// Input fee | ||
| pub input_fee: Amount, | ||
| /// Input fee without swap | ||
| pub input_fee_without_swap: Amount, | ||
| /// Metadata | ||
| pub metadata: HashMap<String, String>, | ||
| } |
There was a problem hiding this comment.
Having these owned types is required for the trait and ffi but it has the downside of adding extra clones since we cannot borrow. This is one reason I suggest we dont encourage the public use of the trait and the Wallet should be used directly.
| async fn prepare_melt( | ||
| &self, | ||
| quote_id: &str, | ||
| metadata: HashMap<String, String>, | ||
| ) -> Result<Self::PreparedMelt, Self::Error>; | ||
|
|
||
| /// Prepare a melt operation with specific proofs | ||
| async fn prepare_melt_proofs( |
There was a problem hiding this comment.
| async fn prepare_melt( | |
| &self, | |
| quote_id: &str, | |
| metadata: HashMap<String, String>, | |
| ) -> Result<Self::PreparedMelt, Self::Error>; | |
| /// Prepare a melt operation with specific proofs | |
| async fn prepare_melt_proofs( | |
| async fn prepare_melt_data( | |
| &self, | |
| quote_id: &str, | |
| metadata: HashMap<String, String>, | |
| ) -> Result<Self::PreparedMelt, Self::Error>; | |
| /// Prepare a melt operation with specific proofs | |
| async fn prepare_melt_proofs_data( |
We should name this fns so its clear they use the ones with the owned type and not the borrowed like the rust api can.
| ) -> Result<Self::Amount, Self::Error>; | ||
|
|
||
| /// Prepare a send transaction | ||
| async fn prepare_send( |
There was a problem hiding this comment.
| async fn prepare_send( | |
| async fn prepare_send_data( |
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:
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
just final-checkbefore committing