Skip to content

refactor: simplify dispatcher by removing pool parameter (#656 PR B) - #665

Merged
grunch merged 1 commit into
mainfrom
cleanup/simplify-dispatcher-656
Mar 17, 2026
Merged

refactor: simplify dispatcher by removing pool parameter (#656 PR B)#665
grunch merged 1 commit into
mainfrom
cleanup/simplify-dispatcher-656

Conversation

@mostronatorcoder

Copy link
Copy Markdown
Contributor

Context

Issue #656 tracks cleanup tasks after phase-5 DI migration (#639).

This PR implements PR B from the #656 checklist: simplify dispatcher and remove pool parameter.

Changes

After PR A (#663), all handlers use AppContext for dependencies. This PR completes the dispatcher simplification:

1. Remove pool from main event loop

  • Deleted: let pool = get_db_pool();
  • Removed: use crate::config::settings::get_db_pool;
  • Removed: use sqlx::{Pool, Sqlite};

2. Update check_trade_index signature

  • Before: check_trade_index(pool: &Pool<Sqlite>, ...)
  • After: check_trade_index(ctx: &AppContext, ...)
  • Internally extracts pool via ctx.pool()

3. Remove migration comments

  • Deleted references to "gradually migrate from using pool directly"
  • Removed outdated docstring parameters (pool, rate_list)

4. Update tests to use AppContext

  • check_trade_index_tests now use TestContextBuilder
  • Tests create AppContext instead of raw pool

Impact

Before:

let pool = get_db_pool();
let ctx = AppContext::from_globals()?;
check_trade_index(&pool, &event, &message).await?;

After:

let ctx = AppContext::from_globals()?;
check_trade_index(&ctx, &event, &message).await?;

Diff stats:

  • 1 file changed
  • +19 / -21 lines
  • Net: -2 lines

Checklist (#656)

  • Remove pool from handle_message_action path ✅
  • Route all handlers through ctx dependencies only ✅
  • Remove migration-only comments ✅
  • Update tests to use AppContext

Validation

cargo fmt
cargo clippy --all-targets --all-features -- -D warnings (0 warnings)
cargo test --bin mostrod (189 passed, 0 failed)

Debt grep checks

$ grep -R "get_db_pool" src/app.rs
# 0 matches ✅

$ grep -R "Pool<Sqlite>" src/app.rs
# 0 matches ✅

Related

Next Steps

After merge:

  • PR C: eliminate remaining global accesses in handlers
  • PR D (optional): finalize AppContext::from_globals()

## Context

Issue #656 tracks cleanup tasks after phase-5 DI migration (#639).
This PR implements **PR B**: simplify dispatcher and remove `pool` parameter.

## Changes

After PR A (#663), all handlers use `AppContext` for dependencies.
This PR completes the dispatcher simplification:

### 1. Remove `pool` from main event loop
- Deleted: `let pool = get_db_pool();`
- Removed: `use crate::config::settings::get_db_pool;`
- Removed: `use sqlx::{Pool, Sqlite};`

### 2. Update `check_trade_index` signature
- Before: `check_trade_index(pool: &Pool<Sqlite>, ...)`
- After: `check_trade_index(ctx: &AppContext, ...)`
- Internally extracts pool via `ctx.pool()`

### 3. Remove migration comments
- Deleted references to "gradually migrate from using pool directly"
- Removed outdated docstring parameters (`pool`, `rate_list`)

### 4. Update tests to use `AppContext`
- `check_trade_index_tests` now use `TestContextBuilder`
- Tests create `AppContext` instead of raw pool

## Impact

**Before:**
```rust
let pool = get_db_pool();
let ctx = AppContext::from_globals()?;
check_trade_index(&pool, &event, &message).await?;
```

**After:**
```rust
let ctx = AppContext::from_globals()?;
check_trade_index(&ctx, &event, &message).await?;
```

**Diff stats:**
- 1 file changed
- +19 / -21 lines
- **Net: -2 lines**

## Validation

✅ `cargo fmt`
✅ `cargo clippy --all-targets --all-features -- -D warnings` (0 warnings)
✅ `cargo test --bin mostrod` (189 passed, 0 failed)

## Debt grep checks

```bash
grep -R "get_db_pool" src/app.rs  # 0 matches ✅
grep -R "Pool<Sqlite>" src/app.rs  # 0 matches ✅
```

## Related

- Parent cleanup issue: #656
- PR A (legacy wrappers): #663
- Original DI migration: #639

@grunch grunch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK

@grunch
grunch merged commit 6f2a623 into main Mar 17, 2026
7 checks passed
@grunch
grunch deleted the cleanup/simplify-dispatcher-656 branch March 17, 2026 17:57
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 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()
grunch 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 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()

Co-authored-by: MostronatorCoder[bot] <182182091+MostronatorCoder[bot]@users.noreply.github.com>
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
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant