refactor: remove global accesses from handler paths (#656 PR C)#666
Merged
refactor: remove global accesses from handler paths (#656 PR C)#666
Conversation
## Context Issue #656 tracks cleanup tasks after phase-5 DI migration (#639). This PR implements **PR C**: remove global accesses from handler paths. ## Changes Replaced direct global function calls with `ctx` accessors in handlers: ### Settings access - `Settings::get_mostro()` → `ctx.settings().mostro` - `Settings::get_ln()` → `ctx.settings().lightning` (where applicable) ### Nostr client access - `get_nostr_client()?` → `ctx.nostr_client()` - Removed fallible Result handling (ctx always has valid client) ### Database pool access - `pool` parameters in internal functions → `ctx: &AppContext` - Extract pool internally: `let pool = ctx.pool();` ## Files Modified (8 total) **Handlers:** - `src/app/admin_cancel.rs` - Settings + nostr_client - `src/app/admin_settle.rs` - Settings + nostr_client - `src/app/admin_take_dispute.rs` - Settings + nostr_client - `src/app/cancel.rs` - Propagate ctx to internal helpers - `src/app/dispute.rs` - Settings + nostr_client + close_dispute_after_user_resolution - `src/app/order.rs` - Settings (calculate_and_check_quote) - `src/app/orders.rs` - Settings - `src/app/release.rs` - nostr_client ## Breaking Changes - `close_dispute_after_user_resolution()` signature changed: - Before: `(pool, order, status, keys, context)` - After: `(ctx, order, status, keys, context)` ## What Remains Global The following still use globals (tracked for future PRs): **In `src/app/release.rs`:** - `check_failure_retries()` - uses `get_db_pool()`, `Settings::get_ln()` - `do_payment()` - uses `get_db_pool()` - `retry_failed_payments()` - uses `get_db_pool()` **Reason:** These are called from `src/scheduler.rs` which doesn't have `AppContext`. Migrating scheduler requires a larger refactor. **In `src/app/context.rs`:** - `AppContext::from_globals()` - by design (bridges old → new architecture) ## Diff Stats - 8 files changed - +59 / -91 lines - **Net: -32 lines** ## Validation ✅ `cargo fmt` ✅ `cargo clippy --all-targets --all-features -- -D warnings` ✅ `cargo test --bin mostrod` (189 passed, 0 failed) ## Debt Grep Checks ```bash # Remaining globals in src/app (excluding context.rs): $ grep -R "Settings::get_" src/app --include="*.rs" | grep -v context.rs src/app/release.rs:39: let ln_settings = Settings::get_ln(); $ grep -R "get_db_pool\|get_nostr_client" src/app --include="*.rs" | grep -v context.rs src/app/release.rs:36: let pool = get_db_pool(); src/app/release.rs:537: let pool = get_db_pool(); src/app/release.rs:622: let pool = get_db_pool(); ``` All remaining globals are in scheduler-called functions (documented above). ## Related - Parent cleanup issue: #656 - PR A (legacy wrappers): #663 ✅ merged - PR B (dispatcher pool): #665 ✅ merged - Original DI migration: #639 ## Next Steps - PR D (optional): Migrate scheduler to use AppContext - PR E (optional): Finalize AppContext::from_globals()
Member
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughThis PR systematically refactors dependency access across multiple handler modules, replacing global Settings and Nostr client getters with context-based accessors. Functions now accept AppContext to retrieve configurations, database pools, and clients instead of using static global access patterns. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
mostronatorcoder bot
pushed a commit
that referenced
this pull request
Mar 17, 2026
## Context Issue #656 tracks cleanup tasks after phase-5 DI migration (#639). This PR implements **PR D**: migrate scheduler to use AppContext. ## Changes ### scheduler.rs - `start_scheduler()` now receives `AppContext` as parameter - All job functions updated to receive and use `ctx`: - `job_expire_pending_older_orders(ctx)` - `job_update_rate_events(ctx)` - `job_cancel_orders(ctx)` - `job_retry_failed_payments(ctx)` - `job_process_dev_fee_payment(ctx)` - `job_info_event_send(ctx)` - `job_relay_list(ctx)` - Removed all `get_db_pool()` calls → use `ctx.pool()` - Removed all `get_nostr_client()` calls → use `ctx.nostr_client()` - Removed all `Settings::get_*()` calls → use `ctx.settings()` ### main.rs - Build `AppContext` before starting scheduler - Pass `ctx` to `start_scheduler()` ### release.rs - `do_payment()` now receives `&AppContext` - `check_failure_retries()` now receives `&AppContext` - `payment_success()` now receives `&AppContext` - `get_child_order()` now receives `&AppContext` - `create_order_event()` now receives `&AppContext` - `order_for_equal()` and `order_for_greater()` now receive `&AppContext` - Removed unused `use crate::config` ### admin_settle.rs - Updated `do_payment()` call to pass `ctx` ## Breaking Changes ### Public API changes: - `start_scheduler()`: `() -> (ctx: AppContext)` - `do_payment()`: `(order, request_id) -> (ctx, order, request_id)` - `check_failure_retries()`: `(order, request_id) -> (ctx, order, request_id)` - `get_child_order()`: `(order, keys) -> (ctx, order, keys)` ## Global Access Elimination After this PR, the following globals are **no longer used anywhere** in handler/scheduler paths: - `get_db_pool()` ❌ - `get_nostr_client()` ❌ (except initial setup in main.rs) - `Settings::get_mostro()` ❌ - `Settings::get_ln()` ❌ The only remaining global access is: - `AppContext::from_globals()` in main.rs (by design - bridges initialization) ## Diff Stats - 4 files changed - +89 / -84 lines - **Net: +5 lines** (mostly signature changes) ## Validation ✅ `cargo fmt` ✅ `cargo clippy --all-targets --all-features -- -D warnings` ✅ `cargo test --bin mostrod` (189 passed, 0 failed) ## Related - Parent cleanup issue: #656 - PR A (legacy wrappers): #663 ✅ merged - PR B (dispatcher pool): #665 ✅ merged - PR C (handler paths): #666 ✅ merged - Original DI migration: #639
This was referenced Mar 17, 2026
grunch
pushed a commit
that referenced
this pull request
Mar 18, 2026
## Context Issue #656 tracks cleanup tasks after phase-5 DI migration (#639). This PR implements **PR D**: migrate scheduler to use AppContext. ## Changes ### scheduler.rs - `start_scheduler()` now receives `AppContext` as parameter - All job functions updated to receive and use `ctx`: - `job_expire_pending_older_orders(ctx)` - `job_update_rate_events(ctx)` - `job_cancel_orders(ctx)` - `job_retry_failed_payments(ctx)` - `job_process_dev_fee_payment(ctx)` - `job_info_event_send(ctx)` - `job_relay_list(ctx)` - Removed all `get_db_pool()` calls → use `ctx.pool()` - Removed all `get_nostr_client()` calls → use `ctx.nostr_client()` - Removed all `Settings::get_*()` calls → use `ctx.settings()` ### main.rs - Build `AppContext` before starting scheduler - Pass `ctx` to `start_scheduler()` ### release.rs - `do_payment()` now receives `&AppContext` - `check_failure_retries()` now receives `&AppContext` - `payment_success()` now receives `&AppContext` - `get_child_order()` now receives `&AppContext` - `create_order_event()` now receives `&AppContext` - `order_for_equal()` and `order_for_greater()` now receive `&AppContext` - Removed unused `use crate::config` ### admin_settle.rs - Updated `do_payment()` call to pass `ctx` ## Breaking Changes ### Public API changes: - `start_scheduler()`: `() -> (ctx: AppContext)` - `do_payment()`: `(order, request_id) -> (ctx, order, request_id)` - `check_failure_retries()`: `(order, request_id) -> (ctx, order, request_id)` - `get_child_order()`: `(order, keys) -> (ctx, order, keys)` ## Global Access Elimination After this PR, the following globals are **no longer used anywhere** in handler/scheduler paths: - `get_db_pool()` ❌ - `get_nostr_client()` ❌ (except initial setup in main.rs) - `Settings::get_mostro()` ❌ - `Settings::get_ln()` ❌ The only remaining global access is: - `AppContext::from_globals()` in main.rs (by design - bridges initialization) ## Diff Stats - 4 files changed - +89 / -84 lines - **Net: +5 lines** (mostly signature changes) ## Validation ✅ `cargo fmt` ✅ `cargo clippy --all-targets --all-features -- -D warnings` ✅ `cargo test --bin mostrod` (189 passed, 0 failed) ## Related - Parent cleanup issue: #656 - PR A (legacy wrappers): #663 ✅ merged - PR B (dispatcher pool): #665 ✅ merged - PR C (handler paths): #666 ✅ merged - Original DI migration: #639 Co-authored-by: MostronatorCoder[bot] <182182091+MostronatorCoder[bot]@users.noreply.github.com>
mostronatorcoder bot
pushed a commit
that referenced
this pull request
Mar 18, 2026
## Context Issue #656 tracks cleanup tasks after phase-5 DI migration (#639). This PR implements **PR E**: add Mostro's signing keys to AppContext. ## Problem `get_keys()` was called 10+ times across the codebase, re-parsing the nsec on every call. This was inefficient and spread error handling across multiple call sites. ## Solution Add `keys: Keys` field to `AppContext`: - Parse nsec once at startup in `from_globals()` - Early error detection for invalid nsec - Access via `ctx.keys()` instead of `get_keys()?` ## Changes ### AppContext (src/app/context.rs) - Added `keys: Keys` field to struct - Updated `new()` to accept `keys` parameter - Updated `from_globals()` to parse keys at construction - Added `keys(&self) -> &Keys` accessor - Updated `TestContextBuilder` with `with_keys()` method ### Scheduler (src/scheduler.rs) - Replaced all `get_keys()?` calls with `ctx.keys().clone()` - Updated jobs: flush_messages_queue, relay_list, info_event_send, cancel_orders, expire_pending_older_orders - Removed `get_keys` import ### Handlers - `release.rs`: Use `ctx.keys().clone()` in `do_payment()` - `admin_take_dispute.rs`: Pass `keys` to `pubkey_event_can_solve()` - `admin_cancel.rs`, `admin_settle.rs`: Pass admin pubkey to `is_dispute_taken_by_admin()` ### Database (src/db.rs) - `is_dispute_taken_by_admin()`: Now takes `admin_pubkey: &str` parameter instead of calling `get_keys()` internally ### Flow (src/flow.rs) - `hold_invoice_paid()`: Now takes `my_keys: &Keys` parameter ### RPC Service (src/rpc/service.rs) - Updated all `AppContext::new()` calls to include `self.keys.clone()` ## What Still Uses `get_keys()` The following still call `get_keys()` (documented for future cleanup): - `src/util.rs`: `publish_dev_fee_audit_event()` - called from dev_fee flow which doesn't have ctx - `src/util.rs`: `invoice_subscribe()` - invoice subscription flow - `src/main.rs`: Initial key loading at startup (by design) ## Diff Stats - 10 files changed - +88 / -59 lines - **Net: +29 lines** (mostly accessor and parameter additions) ## Validation ✅ `cargo fmt` ✅ `cargo clippy --all-targets --all-features -- -D warnings` ✅ `cargo test --bin mostrod` (189 passed, 0 failed) ## Related - Parent cleanup issue: #656 - PR A (legacy wrappers): #663 ✅ merged - PR B (dispatcher pool): #665 ✅ merged - PR C (handler paths): #666 ✅ merged - PR D (scheduler): #667 ✅ merged - PR F (remove from_globals): 📋 planned
grunch
pushed a commit
that referenced
this pull request
Mar 18, 2026
## Context Issue #656 tracks cleanup tasks after phase-5 DI migration (#639). This PR implements **PR E**: add Mostro's signing keys to AppContext. ## Problem `get_keys()` was called 10+ times across the codebase, re-parsing the nsec on every call. This was inefficient and spread error handling across multiple call sites. ## Solution Add `keys: Keys` field to `AppContext`: - Parse nsec once at startup in `from_globals()` - Early error detection for invalid nsec - Access via `ctx.keys()` instead of `get_keys()?` ## Changes ### AppContext (src/app/context.rs) - Added `keys: Keys` field to struct - Updated `new()` to accept `keys` parameter - Updated `from_globals()` to parse keys at construction - Added `keys(&self) -> &Keys` accessor - Updated `TestContextBuilder` with `with_keys()` method ### Scheduler (src/scheduler.rs) - Replaced all `get_keys()?` calls with `ctx.keys().clone()` - Updated jobs: flush_messages_queue, relay_list, info_event_send, cancel_orders, expire_pending_older_orders - Removed `get_keys` import ### Handlers - `release.rs`: Use `ctx.keys().clone()` in `do_payment()` - `admin_take_dispute.rs`: Pass `keys` to `pubkey_event_can_solve()` - `admin_cancel.rs`, `admin_settle.rs`: Pass admin pubkey to `is_dispute_taken_by_admin()` ### Database (src/db.rs) - `is_dispute_taken_by_admin()`: Now takes `admin_pubkey: &str` parameter instead of calling `get_keys()` internally ### Flow (src/flow.rs) - `hold_invoice_paid()`: Now takes `my_keys: &Keys` parameter ### RPC Service (src/rpc/service.rs) - Updated all `AppContext::new()` calls to include `self.keys.clone()` ## What Still Uses `get_keys()` The following still call `get_keys()` (documented for future cleanup): - `src/util.rs`: `publish_dev_fee_audit_event()` - called from dev_fee flow which doesn't have ctx - `src/util.rs`: `invoice_subscribe()` - invoice subscription flow - `src/main.rs`: Initial key loading at startup (by design) ## Diff Stats - 10 files changed - +88 / -59 lines - **Net: +29 lines** (mostly accessor and parameter additions) ## Validation ✅ `cargo fmt` ✅ `cargo clippy --all-targets --all-features -- -D warnings` ✅ `cargo test --bin mostrod` (189 passed, 0 failed) ## Related - Parent cleanup issue: #656 - PR A (legacy wrappers): #663 ✅ merged - PR B (dispatcher pool): #665 ✅ merged - PR C (handler paths): #666 ✅ merged - PR D (scheduler): #667 ✅ merged - PR F (remove from_globals): 📋 planned Co-authored-by: MostronatorCoder[bot] <182182091+MostronatorCoder[bot]@users.noreply.github.com>
mostronatorcoder bot
pushed a commit
that referenced
this pull request
Mar 18, 2026
…656 PR F) ## Context Issue #656 tracks cleanup tasks after phase-5 DI migration (#639). This PR implements **PR F**: remove the `from_globals()` bridge and construct `AppContext` explicitly at bootstrap. ## Problem `AppContext::from_globals()` was a transitional bridge between the old global-based architecture and the new DI pattern. Now that all handlers and scheduler use `AppContext`, we can eliminate this bridge. ## Solution Construct `AppContext` explicitly in `main.rs` using values already available at that point: - `get_db_pool()` — database pool - `client` — Nostr client - `MOSTRO_CONFIG` — settings - `MESSAGE_QUEUES.queue_order_msg` — message queue - `mostro_keys` — signing keys ## Changes ### main.rs - Construct `AppContext::new()` explicitly with all dependencies - Pass `ctx` to `run()` instead of `my_keys` and `client` - Import `MESSAGE_QUEUES` and `MOSTRO_CONFIG` ### app.rs - `run()` signature changed: `(my_keys, client, ln_client)` → `(ctx, ln_client)` - Extract `my_keys`, `client`, and `pow` from `ctx` at function start - Remove `AppContext::from_globals()` call from event loop - Remove unused `Settings` import ### context.rs - **Removed `from_globals()` method entirely** - Removed unused `MESSAGE_QUEUES` import ## Breaking Changes `run()` function signature changed: - **Before:** `run(my_keys: Keys, client: &Client, ln_client: &mut LndConnector)` - **After:** `run(ctx: AppContext, ln_client: &mut LndConnector)` ## Benefits 1. **No more bridge code** — cleaner architecture 2. **Single construction point** — `AppContext` built once in main 3. **Explicit dependencies** — all deps visible at construction 4. **Testability** — easier to mock in tests ## Diff Stats - 3 files changed - +26 / -61 lines - **Net: -35 lines** 🧹 ## Validation ✅ `cargo fmt` ✅ `cargo clippy --all-targets --all-features -- -D warnings` ✅ `cargo test --bin mostrod` (189 passed, 0 failed) ## DI Migration Complete! 🎉 With this PR, the dependency injection migration from #639 is **complete**: | PR | Description | Status | |----|-------------|--------| | PR A | Remove legacy wrappers | ✅ #663 | | PR B | Simplify dispatcher | ✅ #665 | | PR C | Remove global accesses | ✅ #666 | | PR D | Migrate scheduler | ✅ #667 | | PR E | Add keys to AppContext | ✅ #670 | | **PR F** | Remove from_globals() | ✅ This PR | ## Related - Parent cleanup issue: #656 - Suggested by @codaMW in #667 review
grunch
pushed a commit
that referenced
this pull request
Mar 18, 2026
…656 PR F) (#671) ## Context Issue #656 tracks cleanup tasks after phase-5 DI migration (#639). This PR implements **PR F**: remove the `from_globals()` bridge and construct `AppContext` explicitly at bootstrap. ## Problem `AppContext::from_globals()` was a transitional bridge between the old global-based architecture and the new DI pattern. Now that all handlers and scheduler use `AppContext`, we can eliminate this bridge. ## Solution Construct `AppContext` explicitly in `main.rs` using values already available at that point: - `get_db_pool()` — database pool - `client` — Nostr client - `MOSTRO_CONFIG` — settings - `MESSAGE_QUEUES.queue_order_msg` — message queue - `mostro_keys` — signing keys ## Changes ### main.rs - Construct `AppContext::new()` explicitly with all dependencies - Pass `ctx` to `run()` instead of `my_keys` and `client` - Import `MESSAGE_QUEUES` and `MOSTRO_CONFIG` ### app.rs - `run()` signature changed: `(my_keys, client, ln_client)` → `(ctx, ln_client)` - Extract `my_keys`, `client`, and `pow` from `ctx` at function start - Remove `AppContext::from_globals()` call from event loop - Remove unused `Settings` import ### context.rs - **Removed `from_globals()` method entirely** - Removed unused `MESSAGE_QUEUES` import ## Breaking Changes `run()` function signature changed: - **Before:** `run(my_keys: Keys, client: &Client, ln_client: &mut LndConnector)` - **After:** `run(ctx: AppContext, ln_client: &mut LndConnector)` ## Benefits 1. **No more bridge code** — cleaner architecture 2. **Single construction point** — `AppContext` built once in main 3. **Explicit dependencies** — all deps visible at construction 4. **Testability** — easier to mock in tests ## Diff Stats - 3 files changed - +26 / -61 lines - **Net: -35 lines** 🧹 ## Validation ✅ `cargo fmt` ✅ `cargo clippy --all-targets --all-features -- -D warnings` ✅ `cargo test --bin mostrod` (189 passed, 0 failed) ## DI Migration Complete! 🎉 With this PR, the dependency injection migration from #639 is **complete**: | PR | Description | Status | |----|-------------|--------| | PR A | Remove legacy wrappers | ✅ #663 | | PR B | Simplify dispatcher | ✅ #665 | | PR C | Remove global accesses | ✅ #666 | | PR D | Migrate scheduler | ✅ #667 | | PR E | Add keys to AppContext | ✅ #670 | | **PR F** | Remove from_globals() | ✅ This PR | ## Related - Parent cleanup issue: #656 - Suggested by @codaMW in #667 review Co-authored-by: MostronatorCoder[bot] <182182091+MostronatorCoder[bot]@users.noreply.github.com>
mostronatorcoder bot
added a commit
that referenced
this pull request
Mar 18, 2026
…656 PR F) (#671) ## Context Issue #656 tracks cleanup tasks after phase-5 DI migration (#639). This PR implements **PR F**: remove the `from_globals()` bridge and construct `AppContext` explicitly at bootstrap. ## Problem `AppContext::from_globals()` was a transitional bridge between the old global-based architecture and the new DI pattern. Now that all handlers and scheduler use `AppContext`, we can eliminate this bridge. ## Solution Construct `AppContext` explicitly in `main.rs` using values already available at that point: - `get_db_pool()` — database pool - `client` — Nostr client - `MOSTRO_CONFIG` — settings - `MESSAGE_QUEUES.queue_order_msg` — message queue - `mostro_keys` — signing keys ## Changes ### main.rs - Construct `AppContext::new()` explicitly with all dependencies - Pass `ctx` to `run()` instead of `my_keys` and `client` - Import `MESSAGE_QUEUES` and `MOSTRO_CONFIG` ### app.rs - `run()` signature changed: `(my_keys, client, ln_client)` → `(ctx, ln_client)` - Extract `my_keys`, `client`, and `pow` from `ctx` at function start - Remove `AppContext::from_globals()` call from event loop - Remove unused `Settings` import ### context.rs - **Removed `from_globals()` method entirely** - Removed unused `MESSAGE_QUEUES` import ## Breaking Changes `run()` function signature changed: - **Before:** `run(my_keys: Keys, client: &Client, ln_client: &mut LndConnector)` - **After:** `run(ctx: AppContext, ln_client: &mut LndConnector)` ## Benefits 1. **No more bridge code** — cleaner architecture 2. **Single construction point** — `AppContext` built once in main 3. **Explicit dependencies** — all deps visible at construction 4. **Testability** — easier to mock in tests ## Diff Stats - 3 files changed - +26 / -61 lines - **Net: -35 lines** 🧹 ## Validation ✅ `cargo fmt` ✅ `cargo clippy --all-targets --all-features -- -D warnings` ✅ `cargo test --bin mostrod` (189 passed, 0 failed) ## DI Migration Complete! 🎉 With this PR, the dependency injection migration from #639 is **complete**: | PR | Description | Status | |----|-------------|--------| | PR A | Remove legacy wrappers | ✅ #663 | | PR B | Simplify dispatcher | ✅ #665 | | PR C | Remove global accesses | ✅ #666 | | PR D | Migrate scheduler | ✅ #667 | | PR E | Add keys to AppContext | ✅ #670 | | **PR F** | Remove from_globals() | ✅ This PR | ## Related - Parent cleanup issue: #656 - Suggested by @codaMW in #667 review Co-authored-by: MostronatorCoder[bot] <182182091+MostronatorCoder[bot]@users.noreply.github.com>
grunch
pushed a commit
that referenced
this pull request
Mar 18, 2026
…656 PR F) (#671) (#672) ## Context Issue #656 tracks cleanup tasks after phase-5 DI migration (#639). This PR implements **PR F**: remove the `from_globals()` bridge and construct `AppContext` explicitly at bootstrap. ## Problem `AppContext::from_globals()` was a transitional bridge between the old global-based architecture and the new DI pattern. Now that all handlers and scheduler use `AppContext`, we can eliminate this bridge. ## Solution Construct `AppContext` explicitly in `main.rs` using values already available at that point: - `get_db_pool()` — database pool - `client` — Nostr client - `MOSTRO_CONFIG` — settings - `MESSAGE_QUEUES.queue_order_msg` — message queue - `mostro_keys` — signing keys ## Changes ### main.rs - Construct `AppContext::new()` explicitly with all dependencies - Pass `ctx` to `run()` instead of `my_keys` and `client` - Import `MESSAGE_QUEUES` and `MOSTRO_CONFIG` ### app.rs - `run()` signature changed: `(my_keys, client, ln_client)` → `(ctx, ln_client)` - Extract `my_keys`, `client`, and `pow` from `ctx` at function start - Remove `AppContext::from_globals()` call from event loop - Remove unused `Settings` import ### context.rs - **Removed `from_globals()` method entirely** - Removed unused `MESSAGE_QUEUES` import ## Breaking Changes `run()` function signature changed: - **Before:** `run(my_keys: Keys, client: &Client, ln_client: &mut LndConnector)` - **After:** `run(ctx: AppContext, ln_client: &mut LndConnector)` ## Benefits 1. **No more bridge code** — cleaner architecture 2. **Single construction point** — `AppContext` built once in main 3. **Explicit dependencies** — all deps visible at construction 4. **Testability** — easier to mock in tests ## Diff Stats - 3 files changed - +26 / -61 lines - **Net: -35 lines** 🧹 ## Validation ✅ `cargo fmt` ✅ `cargo clippy --all-targets --all-features -- -D warnings` ✅ `cargo test --bin mostrod` (189 passed, 0 failed) ## DI Migration Complete! 🎉 With this PR, the dependency injection migration from #639 is **complete**: | PR | Description | Status | |----|-------------|--------| | PR A | Remove legacy wrappers | ✅ #663 | | PR B | Simplify dispatcher | ✅ #665 | | PR C | Remove global accesses | ✅ #666 | | PR D | Migrate scheduler | ✅ #667 | | PR E | Add keys to AppContext | ✅ #670 | | **PR F** | Remove from_globals() | ✅ This PR | ## Related - Parent cleanup issue: #656 - Suggested by @codaMW in #667 review Co-authored-by: mostronatorcoder[bot] <263173566+mostronatorcoder[bot]@users.noreply.github.com> Co-authored-by: MostronatorCoder[bot] <182182091+MostronatorCoder[bot]@users.noreply.github.com>
mostronatorcoder bot
pushed a commit
that referenced
this pull request
Mar 18, 2026
## Context After completing the DI migration (PRs A-F in #656), the documentation needed updates to reflect the new architecture. ## Changes ### ARCHITECTURE.md - Added new section: **Dependency Injection (AppContext)** - Documents AppContext fields and accessors - Shows construction pattern and testing usage - Updated startup sequence diagram to show AppContext construction - Updated module description to mention `src/app/context.rs` - Changed `run(keys, client, ln)` to `run(ctx, ln)` in diagram ### STARTUP_AND_CONFIG.md - Updated startup steps 8-10: - Step 8: Build AppContext with all dependencies - Step 9: `start_scheduler(ctx)` now receives AppContext - Step 10: `run(ctx, ln_client)` receives AppContext ### DEV_FEE.md - Updated code example to show new scheduler pattern: - `job_process_dev_fee_payment(ctx: AppContext)` - `ctx.pool()` instead of `get_db_pool()` ## Related - Parent cleanup issue: #656 - DI migration PRs: #663, #665, #666, #667, #670, #672
grunch
pushed a commit
that referenced
this pull request
Mar 18, 2026
## Context After completing the DI migration (PRs A-F in #656), the documentation needed updates to reflect the new architecture. ## Changes ### ARCHITECTURE.md - Added new section: **Dependency Injection (AppContext)** - Documents AppContext fields and accessors - Shows construction pattern and testing usage - Updated startup sequence diagram to show AppContext construction - Updated module description to mention `src/app/context.rs` - Changed `run(keys, client, ln)` to `run(ctx, ln)` in diagram ### STARTUP_AND_CONFIG.md - Updated startup steps 8-10: - Step 8: Build AppContext with all dependencies - Step 9: `start_scheduler(ctx)` now receives AppContext - Step 10: `run(ctx, ln_client)` receives AppContext ### DEV_FEE.md - Updated code example to show new scheduler pattern: - `job_process_dev_fee_payment(ctx: AppContext)` - `ctx.pool()` instead of `get_db_pool()` ## Related - Parent cleanup issue: #656 - DI migration PRs: #663, #665, #666, #667, #670, #672 Co-authored-by: MostronatorCoder[bot] <182182091+MostronatorCoder[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Issue #656 tracks cleanup tasks after phase-5 DI migration (#639).
This PR implements PR C: remove global accesses from handler paths.
Changes
Replaced direct global function calls with
ctxaccessors in handlers:Settings access
Settings::get_mostro()→ctx.settings().mostroSettings::get_ln()→ctx.settings().lightning(where applicable)Nostr client access
get_nostr_client()?→ctx.nostr_client()Database pool access
poolparameters in internal functions →ctx: &AppContextlet pool = ctx.pool();Files Modified (8 total)
admin_cancel.rsadmin_settle.rsadmin_take_dispute.rscancel.rsdispute.rsorder.rsorders.rsrelease.rsBreaking Changes
close_dispute_after_user_resolution()signature changed:(pool, order, status, keys, context)(ctx, order, status, keys, context)What Remains Global
The following still use globals (tracked for future PRs):
In
src/app/release.rs:check_failure_retries()- usesget_db_pool(),Settings::get_ln()do_payment()- usesget_db_pool()retry_failed_payments()- usesget_db_pool()Reason: Called from
src/scheduler.rswhich doesn't haveAppContext. Migrating scheduler requires a larger refactor.Diff Stats
Checklist (#656)
get_nostr_client()from handler paths ✅Settings::get_*()from handler paths ✅Validation
✅
cargo fmt✅
cargo clippy --all-targets --all-features -- -D warnings✅
cargo test --bin mostrod(189 passed, 0 failed)Related
Summary by CodeRabbit