Conversation
- flakebox install for flake CIs - custom, informational flake self-check - declare lint shell
The fmcd API now provides: - Automatic invoice monitoring - Using fedimint's native subscribe_ln_receive - Real-time streaming updates - SSE endpoints for live invoice status - Bulk status queries - Efficient multi-invoice management - Full fedimint 0.8 alignment - Native behavior matching the underlying client feat: simplify fmcd api surface
Reorganize fmcd from monolithic binary into single crate that publishes both library and binary, following FMCD_LIBBIN.md design. Changes: - Move business logic to src/core/ (multimint, operations, services) - Move API handlers to src/api/ (REST routes, WebSocket handlers) - Create FmcdCore as central business logic interface - Add library entry point at src/lib.rs, binary at src/bin/main.rs - Simplify AppState to delegate to FmcdCore, removing duplication - Add 'api' feature flag to make web dependencies optional - Refactor handlers as thin wrappers around core methods This enables fmcd to be used as a Rust library while preserving all existing API functionality. Core business logic is now separate from HTTP concerns, improving testability and maintainability.
…ulation - Fix zero balance bug by using client.get_balance() instead of only counting mint notes - Remove duplicate implementations between FmcdCore and API layers - Implement PaymentInfoResolver trait pattern for LNURL handling without coupling core to web protocols - Move all business logic (get_info, join_federation, create_invoice, pay_invoice) to FmcdCore - Make API layer a thin wrapper that delegates to FmcdCore methods - Add proper error handling and context propagation throughout
There was a problem hiding this comment.
Pull request overview
This PR migrates fmcd to the Fedimint 0.9.x stack and updates the surrounding wiring (Nix/toolchain, API/event shapes, and tests) so the workspace builds cleanly and linting passes.
Changes:
- Bump Fedimint crates to
0.9.0and update client/service code to match new APIs (e.g., balance retrieval, client builder/open/join). - Update event models (e.g.,
PaymentSucceededfields) and propagate changes through webhook payloads, metrics, logging, and tests. - Refresh Nix flake inputs/toolchain and RocksDB wiring to improve build reproducibility.
Reviewed changes
Copilot reviewed 28 out of 31 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/config_tests.rs | Refactors config setup in tests to struct-update initialization. |
| tests/auth_tests.rs | Adjusts authenticated message tampering test logic. |
| src/webhooks/tests/notifier_tests.rs | Updates webhook event payload tests for new event fields/redaction expectations. |
| src/webhooks/notifier.rs | Exposes internal helpers to crate scope for testing. |
| src/state_tests.rs | Updates tests to use new accessor style. |
| src/state.rs | Updates multimint() accessor implementation (currently introduces a type issue). |
| src/observability/tests/correlation_tests.rs | Refactors middleware tests to use an Axum router-based approach. |
| src/observability/test_integration.rs | Aligns observability integration tests with middleware exports. |
| src/observability/mod.rs | Re-exports request_id_middleware for wider crate use. |
| src/observability/logging.rs | Tweaks log cleanup logic and removes custom appender/permission enforcement code. |
| src/metrics/collectors.rs | Simplifies collector struct and refactors event matching. |
| src/events/tests/event_bus_tests.rs | Updates test events and improves failure behavior. |
| src/events/handlers/metrics.rs | Simplifies handler struct and updates event handling for new fields. |
| src/events/handlers/logging.rs | Updates logging for new event fields and adjusts tests. |
| src/error/tests.rs | Adds clippy allow to address module naming lint. |
| src/database/instrumented.rs | Exposes get_key_prefix to crate and relocates tests. |
| src/core/services/payment_lifecycle.rs | Updates operation matching and balance retrieval to new APIs. |
| src/core/services/deposit_monitor.rs | Removes unused outpoint plumbing and refactors iteration. |
| src/core/services/balance_monitor.rs | Improves error handling and uses abs_diff for change computation. |
| src/core/operations/tests/payment_tests.rs | Updates payment tracker API usage in tests. |
| src/core/multimint/tests.rs | Adds clippy allow for module naming lint. |
| src/core/multimint/mod.rs | Updates DB/secret storage calls to match new signatures. |
| src/core/multimint/client.rs | Updates Fedimint client builder/open/join flows for 0.9 APIs. |
| src/core/mod.rs | Refactors error handling with inspect_err and updates balance calls/field usage. |
| src/bin/main.rs | Installs rustls crypto provider, cleans env var checks, and removes unused metrics setup. |
| src/api/rest/ln/status.rs | Updates span recording call site. |
| src/api/resolvers/payment.rs | Cleans up unused anyhow imports. |
| flake.nix | Updates nixpkgs/Fedimint inputs, introduces fenix pinning, and adjusts RocksDB/Snappy linking env. |
| flake.lock | Locks updated flake inputs to new revisions/hashes. |
| Cargo.toml | Bumps Fedimint crate versions from 0.8.0 to 0.9.0. |
Comments suppressed due to low confidence (1)
src/events/handlers/logging.rs:496
test_logging_handler_debug_eventsonly asserts the disabled case now, even thoughLoggingEventHandlerstill hasinclude_debug_eventsbranches. Add a positive assertion forLoggingEventHandler::new(true)(and clear captured logs between assertions) so regressions in debug-event logging are caught.
#[tokio::test]
#[traced_test]
async fn test_logging_handler_debug_events() {
let handler_without_debug = LoggingEventHandler::new(false);
let event = FmcdEvent::FederationBalanceUpdated {
federation_id: "test_federation_id".to_string(),
balance_msat: 5000,
correlation_id: Some("test_correlation".to_string()),
timestamp: Utc::now(),
};
// Handler without debug should not log this event
let result = handler_without_debug.handle(event).await;
assert!(result.is_ok());
assert!(!logs_contain("Federation balance updated"));
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async fn ok_handler() -> StatusCode { | ||
| StatusCode::OK | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_request_id_middleware_with_correlation_id() { | ||
| let mut request = Request::builder() | ||
| let app = Router::new() | ||
| .route("/test", get(ok_handler)) | ||
| .layer(from_fn(request_id_middleware)); | ||
|
|
There was a problem hiding this comment.
These tests no longer assert that request_id_middleware actually injects a RequestContext into request extensions (the middleware still does this). Consider updating ok_handler (or adding a dedicated handler) to extract Extension<RequestContext> and fail the request if it’s missing, so the middleware contract remains covered.
| let path = entry.path(); | ||
|
|
||
| // Only process .log files | ||
| if path.is_file() && path.extension().map_or(false, |ext| ext == "log") { | ||
| if path.is_file() && path.extension().is_some_and(|ext| ext == "log") { | ||
| let metadata = entry.metadata().ok()?; | ||
| let modified = metadata.modified().ok()?; |
There was a problem hiding this comment.
With the SecureFileAppender / permission-setting logic removed, init_logging no longer enforces LoggingConfig.file_permissions on newly created log files. This can create world-readable logs depending on the process umask. Either reintroduce setting permissions for created log files (best-effort) or remove file_permissions from LoggingConfig to avoid implying it’s enforced.
| /// Metrics collector that implements the EventHandler trait | ||
| pub struct MetricsCollector { | ||
| start_time: Instant, | ||
| } | ||
| pub struct MetricsCollector {} | ||
|
|
||
| impl MetricsCollector { | ||
| pub fn new() -> Self { | ||
| Self { | ||
| start_time: Instant::now(), | ||
| } | ||
| Self {} | ||
| } |
There was a problem hiding this comment.
pub struct MetricsCollector {} is an empty braced struct; cargo clippy -- --deny warnings commonly flags this (use a unit struct instead). Consider changing this to a unit-like struct (pub struct MetricsCollector;) and adjusting new() accordingly to avoid clippy failures and reduce noise.
| /// Event handler that collects metrics from events for Prometheus export | ||
| pub struct MetricsEventHandler { | ||
| service_name: String, | ||
| } | ||
| pub struct MetricsEventHandler {} | ||
|
|
||
| impl MetricsEventHandler { | ||
| pub fn new(service_name: impl Into<String>) -> Self { | ||
| Self { | ||
| service_name: service_name.into(), | ||
| } | ||
| pub fn new(_service_name: impl Into<String>) -> Self { | ||
| Self {} | ||
| } |
There was a problem hiding this comment.
pub struct MetricsEventHandler {} is an empty braced struct; with cargo clippy -- --deny warnings this may be flagged (prefer a unit struct). Consider changing it to pub struct MetricsEventHandler; and returning Self from new() to keep clippy clean.
| // Convenience accessors for backward compatibility | ||
| pub fn multimint(&self) -> &MultiMint { | ||
| &*self.core.multimint | ||
| &self.core.multimint |
There was a problem hiding this comment.
AppState::multimint() now returns &self.core.multimint, but FmcdCore::multimint is an Arc<MultiMint> (see src/core/mod.rs). This will not type-check against the &MultiMint return type. Deref the Arc (e.g., self.core.multimint.as_ref() / &*self.core.multimint) or change the return type to &Arc<MultiMint> if that’s what callers need.
| &self.core.multimint | |
| self.core.multimint.as_ref() |
Summary
Verification