-
Notifications
You must be signed in to change notification settings - Fork 109
feat: custom axum router #1251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: custom axum router #1251
Conversation
| enum OutgoingPaymentOptionsType { | ||
| OUTGOING_BOLT11 = 0; | ||
| OUTGOING_BOLT12 = 1; | ||
| OUTGOING_PAYMENT_OPTIONS_TYPE_CUSTOM = 2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was prefixed to avoid collisions.
https://buf.build/docs/best-practices/style-guide/#enums
https://buf.build/docs/lint/rules/#enum_value_prefix
We might want to prefix all other enums as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was prefixed to avoid collisions. https://buf.build/docs/best-practices/style-guide/#enums https://buf.build/docs/lint/rules/#enum_value_prefix
We might want to prefix all other enums as well.
50a77aa was necessary for this.
- Replace `custom_payment_methods` struct with dynamic retrieval from payment processors. - Update payment-related logic to support dynamic custom methods. - Simplify configuration by removing redundant custom payment method fields. - Adjust examples and tests accordingly.
- Replace `PaymentProcessorSettings` with `SettingsResponse` for strongly typed configuration. - Update all `get_settings` methods to return `SettingsResponse` instead of JSON. - Adjust payment-related logic and proto definitions to utilize the new structure. - Simplify serialization and removal of redundant type conversions.
thesimplekid
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work. Initial comments
crates/cashu/src/nuts/nut_custom.rs
Outdated
| /// Optional description | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub description: Option<String>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think this is bolt11 specific and we shouldnt have it always defined if a payment type wants it they can put it on data.
| /// Custom payment method request | ||
| Custom { | ||
| /// Payment method name (e.g., "paypal", "venmo") | ||
| method: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our PaymentMethod enum has a custom variant should we use that?
| payment_quote.fee, | ||
| unix_time() + melt_ttl, | ||
| payment_quote.request_lookup_id.clone(), | ||
| None, // Custom methods don't use options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should they maybe there should be a custom variant on that?
|
Love this PR! My questions / requests from the call:
|
- Remove `bolt12_router` and related endpoint declarations. - Integrate BOLT12 handling directly into custom payment method logic. - Simplify `create_mint_router` by removing `include_bolt12` parameter. - Update routing and handlers for unified dynamic custom method processing. - Cleanup redundant code and references.
- Add `QuoteState` to `nut_custom` for enhanced quote handling. - Extend dynamic payment method handling with support for `bolt11`, `bolt12`, and custom methods. - Simplify `check_mint_quote_paid` by removing method-specific conditions. - Ensure consistent integration of custom payment logic across mint and melt operations. - Update NUT04 and NUT05 configurations to process new methods directly.
…stom payment support
This commit completes the wallet-side payment method generalization migration:
**Phase 1: Wallet Issue (Mint) Refactoring**
- Rename issue_bolt11.rs and issue_bolt12.rs to bolt11.rs and bolt12.rs
- Create unified router in issue/mod.rs with mint_quote_unified() and mint_unified()
- Add custom.rs with custom payment method support for minting
- Routes based on PaymentMethod enum (Known vs Custom variants)
**Phase 2: Wallet Melt (Payment) Refactoring**
- Rename melt_bolt11.rs and melt_bolt12.rs to bolt11.rs and bolt12.rs
- Create unified router in melt/mod.rs with melt_quote_unified()
- Add custom.rs with custom payment method support for melting
- **CRITICAL FIX**: Remove UnsupportedPaymentMethod error for Custom payment methods
in melt_proofs dispatcher (lines 200-210 in bolt11.rs)
**Phase 3: HTTP Client Updates**
- Add MintQuoteCustomRequest and MintQuoteCustomResponse imports
- Add MeltQuoteCustomRequest import
- Implement post_mint_custom_quote() in MintConnector trait and HttpClient
- Implement post_melt_custom_quote() in MintConnector trait and HttpClient
- Custom methods use /v1/mint/quote/{method} and /v1/melt/quote/{method} routes
**Architecture Changes:**
- Wallet now supports both Known (Bolt11, Bolt12) and Custom payment methods
- Private bolt11/bolt12 modules handle method-specific logic
- Public unified routers dispatch based on PaymentMethod variant
- Custom payment methods can be processed through the same flows as Bolt11/Bolt12
This implementation follows the migration_plan.md specification and removes the
critical blocker that prevented custom payment methods from working in the wallet.
Add post_mint_custom_quote() and post_melt_custom_quote() stub implementations to DirectMintConnection test mock to satisfy the MintConnector trait requirements. These methods return UnsupportedPaymentMethod error as custom payment methods are not yet implemented in the test infrastructure.
# Conflicts: # crates/cdk-sqlite/src/wallet/mod.rs # crates/cdk/src/wallet/melt/bolt11.rs # crates/cdk/src/wallet/melt/mod.rs
# Conflicts: # crates/cdk-common/src/mint.rs # crates/cdk/src/mint/ln.rs
thesimplekid
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work. We seem to be doing alot of matching on str when it would be better if we could use the enums I think for methods.
crates/cashu/src/nuts/nut00/mod.rs
Outdated
| impl Default for PaymentMethod { | ||
| fn default() -> Self { | ||
| Self::Known(KnownMethod::Bolt11) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have a default for this? I think maybe not.
crates/cdk-mintd/src/lib.rs
Outdated
| add_endpoint(ws_protected_endpoint, &auth_settings.websocket_auth); | ||
| } | ||
|
|
||
| // Custom payment method endpoints will be added dynamically after the mint is built |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this correct and in the right place? Does it refer to auth endpoints?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it does refer to the protected_endpoints in auth mint. These also have to be dynamically generated based on the provided payment processor methods here.
I'll update the comment to use the correct function name.
| message SettingsResponse { | ||
| string inner = 1; | ||
| string unit = 1; | ||
| Bolt11Settings bolt11 = 2; | ||
| Bolt12Settings bolt12 = 3; | ||
| map<string, string> custom = 4; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should maybe start versioning the proto file.
thesimplekid
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM to me. One small nit but other then that nice work.
| // Handle bolt11 methods | ||
| if method.is_bolt11() { | ||
| if let Some(ref bolt11_settings) = settings.bolt11 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A match would be better here then the if, else if else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to update the matching route paths to account for the custom path otherwise they wont be projected by auth.
Have you tried custom paths with auth?
diff --git a/crates/cashu/src/nuts/auth/nut21.rs b/crates/cashu/src/nuts/auth/nut21.rs
index 15557bdd..579d4b78 100644
--- a/crates/cashu/src/nuts/auth/nut21.rs
+++ b/crates/cashu/src/nuts/auth/nut21.rs
@@ -387,6 +387,16 @@ mod tests {
assert!(matches!(result.unwrap_err(), Error::InvalidRegex(_)));
}
+ #[test]
+ // #[ignore = "regex matching does not include custom payment methods"]
+ fn test_matching_route_paths_custom_payment_methods() {
+ let paths = matching_route_paths("^/v1/mint/.*").unwrap();
+
+ assert!(paths.contains(&RoutePath::Mint("bolt11".to_string())));
+ assert!(paths.contains(&RoutePath::Mint("bolt12".to_string())));
+ assert!(paths.contains(&RoutePath::Mint("paypal".to_string())));
+ }
+
#[test]
fn test_route_path_to_string() {
// Test that to_string() returns the correct path strings
Description
Add support for custom payment methods and custom Axum routes. Custom payment methods can be provided by gRPC payment processors using the SettingsResponse proto message.
Router Framework: Added CustomRouter and CustomHandlers in cdk-axum to enable pluggable payment method handlers with support for custom routes and business logic
Custom Payment Methods: Introduced NutCustom type definitions in the Cashu spec to formally define custom payment method schemas and request/response handling
Typed Settings Response: Migrated the settings endpoint to use a strongly-typed SettingsResponse for better type safety and validation across the mint
Auth & Payment Refactoring: Refactored authentication routes and custom payment method handling across all backend implementations (CLN, LDK, LNbits, fake-wallet) to work with the new router pattern
Rename
NutCustomtoCustomPaymentMethodUpdate mintd to NOT load MintInfo from storage on start
Notes to the reviewers
Suggested CHANGELOG Updates
CHANGED
ADDED
REMOVED
FIXED
Checklist
just final-checkbefore committing